FrankChen021 commented on code in PR #19603:
URL: https://github.com/apache/druid/pull/19603#discussion_r3442409940
##########
server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java:
##########
@@ -569,7 +570,6 @@ void forceOrWaitOngoingDatabasePoll()
}
// Verify if there was a on-demand poll completed while we were
waiting for the lock
if (latestDatabasePoll instanceof OnDemandDatabasePoll) {
- long checkStartTimeNanos =
TimeUnit.MILLISECONDS.toNanos(checkStartTime);
OnDemandDatabasePoll latestOnDemandPoll = (OnDemandDatabasePoll)
latestDatabasePoll;
if (latestOnDemandPoll.initiationTimeNanos > checkStartTimeNanos) {
Review Comment:
[P2] Check poll success before reusing a contended on-demand poll
Now that this nanoTime comparison can be true, a caller queued behind
another on-demand poll can return without checking whether that poll succeeded.
If the first poll throws, doOnDemandPoll completes pollCompletionFuture
exceptionally and releases the write lock; any caller whose checkStartTimeNanos
was before that poll started will hit this branch and return the stale
dataSourcesSnapshot instead of retrying or propagating the failure. The
existing catch says unsuccessful latest polls should trigger a new poll, so
this branch should verify pollCompletionFuture, for example with
Futures.getUnchecked(latestOnDemandPoll.pollCompletionFuture), before returning.
--
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]
