capistrant commented on code in PR #19671:
URL: https://github.com/apache/druid/pull/19671#discussion_r3561614190
##########
server/src/main/java/org/apache/druid/segment/loading/PartialSegmentMetadataCacheEntry.java:
##########
@@ -410,6 +410,56 @@ public void applyRule(String fingerprint, Set<String>
selectedBundleNames)
finally {
entryLock.unlock();
}
+
+ // Phase 3b: reconcile linkedBundles against the freshly-committed
selection. A concurrent registerBundle
+ // whose linkedBundles.put landed AFTER our Phase 1 snapshot but whose
Phase 1 selection-check ran BEFORE
+ // our Phase 3 commit would have observed the OLD selection and bailed
without installing a hold, while
+ // Phase 1's namesToAcquire also missed it. Rescan + install to close
that window. Loops in case a bundle
+ // arrives during reconciliation; termination is bounded by
|newSelection| because each iteration either installs
+ // at least one hold or exits (a bundle that arrives after this loop
terminates will see the committed
+ // newSelection in its own Phase 1 and install its hold via
registerBundle Phase 3).
+ while (true) {
Review Comment:
do we really need the loop? won't we clean up anything that arrived between
snapshot and commit here just fine and anything that happens now with a bundle
showing up will be registered just fine in registerBundle?
##########
server/src/main/java/org/apache/druid/segment/loading/PartialSegmentMetadataCacheEntry.java:
##########
@@ -410,6 +410,56 @@ public void applyRule(String fingerprint, Set<String>
selectedBundleNames)
finally {
entryLock.unlock();
}
+
+ // Phase 3b: reconcile linkedBundles against the freshly-committed
selection. A concurrent registerBundle
+ // whose linkedBundles.put landed AFTER our Phase 1 snapshot but whose
Phase 1 selection-check ran BEFORE
+ // our Phase 3 commit would have observed the OLD selection and bailed
without installing a hold, while
+ // Phase 1's namesToAcquire also missed it. Rescan + install to close
that window. Loops in case a bundle
+ // arrives during reconciliation; termination is bounded by
|newSelection| because each iteration either installs
+ // at least one hold or exits (a bundle that arrives after this loop
terminates will see the committed
+ // newSelection in its own Phase 1 and install its hold via
registerBundle Phase 3).
+ while (true) {
+ final List<String> reconcileNames;
+ entryLock.lock();
+ try {
+ reconcileNames = new ArrayList<>();
+ for (String name : ruleSelectedBundleNames) {
+ if (!ruleBundleHolds.containsKey(name) &&
findLinkedBundleByName(name) != null) {
+ reconcileNames.add(name);
+ }
+ }
+ }
+ finally {
+ entryLock.unlock();
+ }
+ if (reconcileNames.isEmpty()) {
Review Comment:
is there any unhinged situation were this condition is never met?
--
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]
