Skip to content

Conversation

@duxiao1212
Copy link
Contributor

@duxiao1212 duxiao1212 commented Oct 31, 2025

Summary:

Reset executor before test teardown to ensure async work completes and releases
memory pool references before SharedArbitrator shutdown checks.
The executor thread pool processes async tasks could hold references to leaf memory pools.
Without explicitly resetting the executor, these tasks complete after TearDown(), keeping the pool hierarchy alive during shutdown assertions, causing " Unexpected alive participants" errors.

@netlify
Copy link

netlify bot commented Oct 31, 2025

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 21a0f00
🔍 Latest deploy log https://app.netlify.com/projects/meta-velox/deploys/690a3a0d1ca7a90008f9f744

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 31, 2025
@meta-codesync
Copy link

meta-codesync bot commented Oct 31, 2025

@duxiao1212 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D85685549.

duxiao1212 added a commit to duxiao1212/velox that referenced this pull request Oct 31, 2025
…bator#15344)

Summary:

X-link: prestodb/presto#26461

Reset executor before test teardown to ensure async work completes and releases 
memory pool references before SharedArbitrator shutdown checks.

The executor thread pool processes async tasks could hold references to leaf memory pools.
Without explicitly resetting the executor, these tasks complete after TearDown(), keeping the pool hierarchy alive during shutdown assertions, causing " Unexpected alive participants" errors.

Reviewed By: xiaoxmeng

Differential Revision: D85685549
@duxiao1212 duxiao1212 changed the title fix: Fix flaky test BroadcastTest.endToEndWithMultipleWriteNodes fix: Fix memory pool cleanup in exchange operator tests Oct 31, 2025
@duxiao1212 duxiao1212 changed the title fix: Fix memory pool cleanup in exchange operator tests fix(ci): Fix memory pool cleanup in exchange operator tests Oct 31, 2025
duxiao1212 added a commit to duxiao1212/velox that referenced this pull request Oct 31, 2025
…bator#15344)

Summary:


The executor thread pool processes async tasks could hold references to leaf memory pools.
Without explicitly resetting the executor, these tasks complete after TearDown(), keeping the pool hierarchy alive during shutdown assertions, causing " Unexpected alive participants" errors.


Reset executor before test teardown to ensure async work completes and releases
memory pool references before SharedArbitrator shutdown checks.
1: executor_.reset()  triggers the destructor
2: CPUThreadPoolExecutor's destructor  calls thread join internally
3: This blocks until all queued async work completes
4: Ensures pool references are released before TearDown()

Reviewed By: xiaoxmeng

Differential Revision: D85685549
…bator#15344)

Summary:


The executor thread pool processes async tasks could hold references to leaf memory pools.
Without explicitly resetting the executor, these tasks complete after TearDown(), keeping the pool hierarchy alive during shutdown assertions, causing " Unexpected alive participants" errors.


Reset executor before test teardown to ensure async work completes and releases
memory pool references before SharedArbitrator shutdown checks.
1: executor_.reset()  triggers the destructor
2: CPUThreadPoolExecutor's destructor  calls thread join internally
3: This blocks until all queued async work completes
4: Ensures pool references are released before TearDown()

Reviewed By: xiaoxmeng

Differential Revision: D85685549
@meta-codesync
Copy link

meta-codesync bot commented Nov 5, 2025

This pull request has been merged in 833482d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants