jacklong319 commented on code in PR #10133:
URL: https://github.com/apache/paimon/pull/10133#discussion_r4256108386
##########
paimon-core/src/main/java/org/apache/paimon/operation/AbstractFileStoreWrite.java:
##########
@@ -703,12 +713,98 @@ private void checkNumBuckets(String partInfo, int
expected, int previous) {
}
}
- private ExecutorService compactExecutor() {
+ private ExecutorService compactExecutor(BinaryRow partition, int bucket) {
+ if (externalCompactExecutor) {
+ return lazyCompactExecutor;
+ }
+
+ switch (compactionTaskExecutorMode) {
+ case PER_BUCKET:
+ return perBucketCompactExecutors.computeIfAbsent(
+ new BucketCompactionExecutorKey(partition, bucket),
+ key ->
+ Executors.newSingleThreadExecutor(
+ new ExecutorThreadFactory(
+
Thread.currentThread().getName()
+ + "-compaction-bucket-"
+ + key.bucket)));
+ case FIXED_POOL:
+ return sharedCompactionExecutor(compactionTaskThreads,
"-compaction-pool");
+ case SINGLE:
+ default:
+ return sharedCompactionExecutor(1, "-compaction");
+ }
+ }
+
+ private void releaseCompactionExecutor(BinaryRow partition, int bucket) {
+ if (compactionTaskExecutorMode != CompactionTaskExecutorMode.PER_BUCKET
+ || externalCompactExecutor) {
Review Comment:
@JingsongLi Thanks for catching the late external-override leak.
Fix: releaseCompactionExecutor no longer bails out globally when
externalCompactExecutor is true. Cleanup is driven by actual per-bucket
ownership: if the bucket still has an entry in perBucketCompactExecutors, we
remove it, shutdownNow, and retire compact timers for that bucket. Buckets that
only ever used the shared external pool have no map entry, so release remains a
no-op and shared-worker timer semantics are unchanged.
Test:
CompactionTaskExecutorRoutingTest#testLateExternalOverrideReleasesOwnedPerBucketExecutor
— start an internal -1 worker, install withCompactExecutor(), release the old
bucket, assert zero retained internal executors and timers retired.
Could you re-run your executor-ownership regression when CI is green? Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
