xiangfu0 commented on code in PR #19468:
URL: https://github.com/apache/pinot/pull/19468#discussion_r4088440545
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java:
##########
@@ -306,7 +306,11 @@ public synchronized void shutDown() {
return;
}
_logger.info("Shutting down table data manager");
- _shutDown = true;
+ // Close admission atomically with publishing and starting a consuming
segment. Segment construction and
+ // shutdown's blocking cleanup must remain outside this monitor.
+ synchronized (_segmentDataManagerMap) {
Review Comment:
Not deferred — this was closed in 22e1399, and 30033c3 hardens the remaining
edge:
- 22e1399: `addSegment(ImmutableSegment, ...)` now registers with the
shutdown drain barrier atomically with the `_shutDown` check under the map
monitor, and `shutDown()` waits (`arriveAndAwaitAdvance`) before draining. A
load that finishes after shutdown is rejected there and destroyed instead of
landing in the drained map, and `registerSegment` is only reachable through
that admission. Covered by the late-ONLINE-load and registration regressions in
`BaseTableDataManagerTest`.
- 30033c3: the download path mutates the segment data directory
(`moveSegment`: `deleteDirectory` + `moveDirectory`) before reaching that
admission, so a stale download of a deleted table could still replace a
recreated same-name table's directory. `moveSegment` now re-checks `_shutDown`
before the mutation; `testMoveSegmentRejectedAfterShutdown` fails with the
guard removed.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
