Skip to content

Conversation

@yjhjstz
Copy link
Member

@yjhjstz yjhjstz commented Jul 11, 2025

The legacy event_init() function is not thread-safe and can cause issues when gpfdist is run in multi-threaded environments. This patch updates gpfdist to use libevent 2.0+'s thread-safe APIs, specifically event_base along with event_assign() and evtimer_assign().

A new global gcb.event_base is introduced and used when compiled with libevent ≥ 2.0.1. This avoids the need for the deprecated and non-thread-safe event_set() / evtimer_set() APIs, and prepares gpfdist for better thread safety.

This change maintains backward compatibility with older libevent versions via conditional compilation.

Fixes #ISSUE_Number

What does this PR do?

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change (fix or feature with breaking changes)
  • Documentation update

Breaking Changes

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

Additional Context

CI Skip Instructions


@my-ship-it
Copy link
Contributor

Good start. Do we plan to enable multi-threads for libevent 2.0+ in current PR?

@my-ship-it my-ship-it self-requested a review July 24, 2025 06:41
@yjhjstz
Copy link
Member Author

yjhjstz commented Jul 24, 2025

Good start. Do we plan to enable multi-threads for libevent 2.0+ in current PR?

No, simple multi threads mvp show performance downgrade.

@yjhjstz yjhjstz force-pushed the opt_events branch 2 times, most recently from 056bfc3 to b756915 Compare July 28, 2025 23:10
@my-ship-it
Copy link
Contributor

Good start. Do we plan to enable multi-threads for libevent 2.0+ in current PR?

No, simple multi threads mvp show performance downgrade.

So do we still need the PR?

@yjhjstz
Copy link
Member Author

yjhjstz commented Jul 29, 2025

So do we still need the PR?

yes, using thread-safe APIs is a better choice. In case we found the right way.

@yjhjstz yjhjstz requested a review from gfphoenix78 July 29, 2025 01:43
Copy link
Contributor

@my-ship-it my-ship-it left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

yjhjstz added 2 commits July 30, 2025 22:22
…n gpfdist

The legacy event_init() function is not thread-safe and can cause issues
when gpfdist is run in multi-threaded environments. This patch updates
gpfdist to use libevent 2.0+'s thread-safe APIs, specifically
`event_base` along with `event_assign()` and `evtimer_assign()`.

A new global `gcb.event_base` is introduced and used when compiled with
libevent ≥ 2.0.1. This avoids the need for the deprecated and
non-thread-safe `event_set()` / `evtimer_set()` APIs, and prepares
gpfdist for better thread safety.
Copy link
Contributor

@gfphoenix78 gfphoenix78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM +1

@yjhjstz yjhjstz merged commit 26a2f9c into apache:main Jul 31, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants