This is an automated email from the ASF dual-hosted git repository.
clintropolis pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 80d6991eadd fix: acquire cached unmount race (#19873)
80d6991eadd is described below
commit 80d6991eadd1daeaaa11f037e1f5ea0e0c76e186
Author: Clint Wylie <[email protected]>
AuthorDate: Tue Aug 4 13:33:19 2026 -0700
fix: acquire cached unmount race (#19873)
---
.../segment/loading/SegmentLocalCacheManager.java | 14 +++++++++-
.../loading/SegmentLocalCacheManagerTest.java | 31 ++++++++++++++++++++++
2 files changed, 44 insertions(+), 1 deletion(-)
diff --git
a/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java
b/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java
index 8ee13001f23..7764c62a153 100644
---
a/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java
+++
b/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java
@@ -1299,8 +1299,20 @@ public class SegmentLocalCacheManager implements
SegmentCacheManager
);
}
if (complete.isMounted()) {
+ // the entry is already mounted, so hand back its cached reference
provider. Read the volatile
+ // referenceProvider exactly once, inside the supplier, rather
than trusting the isMounted() check above and
+ // reading the field again when the supplier runs later: for a
static (non-virtual-storage) entry a
+ // concurrent drop (release() -> unmount()) can null it in
between. The reservation hold does not prevent
+ // this (it only guards weak entries against reclaim), so a weak
entry would stay mounted here, but a
+ // static entry can be unmounted out from under us. A dropped
entry is reported as absent (empty) rather
+ // than reloaded.
return new AcquireSegmentAction(
- () ->
Futures.immediateFuture(AcquireSegmentResult.cached(complete.referenceProvider)),
+ () -> {
+ final ReferenceCountedSegmentProvider provider =
complete.referenceProvider;
+ return Futures.immediateFuture(
+ provider != null ? AcquireSegmentResult.cached(provider)
: AcquireSegmentResult.empty()
+ );
+ },
hold
);
} else {
diff --git
a/server/src/test/java/org/apache/druid/segment/loading/SegmentLocalCacheManagerTest.java
b/server/src/test/java/org/apache/druid/segment/loading/SegmentLocalCacheManagerTest.java
index 458da17f219..d0ec702926b 100644
---
a/server/src/test/java/org/apache/druid/segment/loading/SegmentLocalCacheManagerTest.java
+++
b/server/src/test/java/org/apache/druid/segment/loading/SegmentLocalCacheManagerTest.java
@@ -918,6 +918,37 @@ public class SegmentLocalCacheManagerTest extends
InitializedNullHandlingTest
segmentActionAfterDrop.close();
}
+ @Test
+ public void
testAcquireExistingSegmentDroppedBeforeSupplierRunsReportsAbsent() throws
Exception
+ {
+ final DataSegment segmentToLoad =
makeTestDataSegment(segmentDeepStorageDir);
+ final File localSegmentFile = new File(segmentDeepStorageDir,
TEST_DATA_RELATIVE_PATH);
+ makeSegmentZip(
+ localSegmentFile,
+ new File(segmentDeepStorageDir.getCanonicalPath() + "/" +
TEST_DATA_RELATIVE_PATH + "/index.zip")
+ );
+
+ manager.load(segmentToLoad);
+ Assert.assertTrue("segment should be cached (static, mounted) after load",
manager.isSegmentCached(segmentToLoad));
+
+ // Take the already-loaded fast path, but do NOT invoke the supplier yet
(getSegmentFuture() is what runs it).
+ final AcquireSegmentAction action = manager.acquireSegment(segmentToLoad,
AcquireMode.FULL);
+
+ // Drop the segment: for a static entry release() unmounts it immediately,
nulling referenceProvider out from
+ // under the still-outstanding (no-op) hold.
+ manager.drop(segmentToLoad);
+
+ // Invoking the supplier now must not NPE; the segment is reported absent
(empty) instead.
+ final AcquireSegmentResult result = action.getSegmentFuture().get();
+ Assert.assertNotNull("reference provider must never be null",
result.getReferenceProvider());
+ Assert.assertFalse(
+ "a segment dropped before the supplier ran should be reported absent",
+ result.getReferenceProvider().acquireReference().isPresent()
+ );
+
+ action.close();
+ }
+
@Test
public void testVirtualStorageRejectsNonPositiveLoadThreads()
{
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
