keith-turner commented on code in PR #5538:
URL: https://github.com/apache/accumulo/pull/5538#discussion_r2085525925
##########
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:
Changing the `>` to a `>=` is iffy and probably not good so please ignore
that suggestion. One way we could avoid comparing the time or anything else is
to just look for change in the object reference. So maybe could do the
following?
```java
CachedTablet before = findTabletInCache(row);
try (var unused = lookupLocks.lock(ptl.getExtent())) {
CachedTablet after = findTabletInCache(row);
if(after != before){
// another thread probably did the lookup while we were waiting
on the lock, so let use that
return;
}
```
--
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
