cecemei commented on code in PR #18176:
URL: https://github.com/apache/druid/pull/18176#discussion_r2226768901
##########
server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java:
##########
@@ -65,35 +74,35 @@ public class SegmentLocalCacheManager implements
SegmentCacheManager
private final List<StorageLocation> locations;
- // This directoryWriteRemoveLock is used when creating or removing a
directory
- private final Object directoryWriteRemoveLock = new Object();
-
/**
* A map between segment and referenceCountingLocks.
*
* These locks should be acquired whenever getting or deleting files for a
segment.
* If different threads try to get or delete files simultaneously, one of
them creates a lock first using
- * {@link #createOrGetLock}. And then, all threads compete with each other
to get the lock.
- * Finally, the lock should be released using {@link #unlock}.
+ * {@link #lock(DataSegment)}. And then, all threads compete with each other
to get the lock.
+ * Finally, the lock should be released using {@link #unlock(DataSegment,
ReferenceCountingLock)}.
*
* An example usage is:
*
- * final ReferenceCountingLock lock = createOrGetLock(segment);
+ * final SegmentCacheEntry entry = new SegmentCacheEntry(segment);
+ * final ReferenceCountingLock lock = entry.lock();
Review Comment:
`entry.lock()` is not true? we could maybe use `entry.lock(segmentLocks)`.
also this lock map seems to be the guard for concurrent segment operation,
might be worth calling that out, since `SegmentCacheEntry` is not thread safe.
--
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]
