keith-turner commented on code in PR #5538:
URL: https://github.com/apache/accumulo/pull/5538#discussion_r2085509708
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImpl.java:
##########
@@ -690,25 +690,28 @@ private void lookupTablet(ClientContext context, Text
row, LockCheckerSession lc
metadataRow.append(row.getBytes(), 0, row.getLength());
CachedTablet ptl = parent.findTablet(context, metadataRow, false,
LocationNeed.REQUIRED);
- if (ptl != null) {
- // Only allow a single lookup at time per parent tablet. For example if
a tables tablets are
- // all stored in three metadata tablets, then that table could have up
to three concurrent
- // metadata lookups.
- Timer timer = Timer.startNew();
- try (var unused = lookupLocks.lock(ptl.getExtent())) {
- // See if entry was added to cache by another thread while we were
waiting on the lock
- var cached = findTabletInCache(row);
- if (cached != null && cached.getCreationTimer().startedAfter(timer)) {
Review Comment:
What made you suspect the timer here? I looked into the specifics of the
timer and found how its implemented here
https://github.com/openjdk/jdk/blob/b6b5ac1ef9042ed62a8358aa6943b8dc87dcf0ab/src/hotspot/os/posix/os_posix.cpp#L1474
. Looking at the docs for that system call it does mention that it will not
go backwards but also there is no gaurantee it wil go forward between two
calls. So its possible two threads got the exact same nanoTime causing the
entry to look stale, that is probably ok. We could make the time comparison
`>=` instead of `>`.
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
