FrankChen021 commented on code in PR #20089:
URL: https://github.com/apache/druid/pull/20089#discussion_r3821779113
##########
server/src/main/java/org/apache/druid/client/BrokerServerView.java:
##########
@@ -351,26 +394,80 @@ private void serverRemovedSegment(DruidServerMetadata
server, DataSegment segmen
}
if (selector.isEmpty()) {
- VersionedIntervalTimeline<String, ServerSelector> timeline =
timelines.get(segment.getDataSource());
- selectors.remove(segmentId);
-
- final PartitionChunk<ServerSelector> removedPartition =
timeline.remove(
- segment.getInterval(), segment.getVersion(),
segment.getShardSpec().createChunk(selector)
- );
-
- if (removedPartition == null) {
- log.warn(
- "Asked to remove timeline entry[interval: %s, version: %s] that
doesn't exist",
- segment.getInterval(),
- segment.getVersion()
+ final long delayMillis =
segmentWatcherConfig.getSegmentDropDelayMillis();
+ if (delayMillis > 0) {
+ // Schedule a delayed removal to prevent the segment load/drop race
condition.
+ // When a segment is moved from one historical to another, the
broker may receive
+ // the drop callback from the old server before the load callback
from the new one.
+ // By delaying the timeline removal, we give the new server time to
announce the
+ // segment, preventing the segment from temporarily disappearing
from the timeline.
+ // See https://github.com/apache/druid/issues/18738
+ final ScheduledFuture<?> pendingRemoval =
delayedRemovalExecutor.schedule(
+ () -> {
+ pendingSegmentRemovals.remove(segmentId);
Review Comment:
[P1] Stale timer can remove a newer pending removal
An expired timer removes the segment ID without verifying that its future is
still current. If it loses the lock to a reload followed by another drop, the
newer timer is installed, then the older task can pass the selector checks and
remove the timeline entry before the newer delay expires, reintroducing the
load/drop race and causing partial query results. Guard removal with the
expected future or a generation token under the same lock.
--
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]
