noob-se7en commented on code in PR #19468:
URL: https://github.com/apache/pinot/pull/19468#discussion_r4087123309
##########
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java:
##########
@@ -355,7 +379,6 @@ private TableDataManager createTableDataManager(String
tableNameWithType) {
Preconditions.checkState(tableCreationTimeMs > tableDeleteTimeMs,
"Table: %s was recently deleted (deleted %dms ago) but the table
config was created before that (created "
Review Comment:
not from this PR, but this guard now also fires for tables that never had a
local manager so it will show up more: guava `checkState` only substitutes
`%s`, so the `%dms ago` placeholders render literally and the two durations get
appended in brackets at the end of the message.
##########
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:
the ONLINE path still has the window this closes for CONSUMING:
`addOnlineSegment` checks `_shutDown` before the slow download/load and
`registerSegment` publishes into the map without re-checking under this
monitor, so a segment that finishes loading after `releaseAndRemoveAllSegments`
lands in the drained map and is never destroyed. intentionally deferred? worth
a note in the description or a follow-up issue either way.
--
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]
