keith-turner commented on code in PR #5421:
URL: https://github.com/apache/accumulo/pull/5421#discussion_r2015052826
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java:
##########
@@ -1088,9 +1098,43 @@ public NamespaceMapping getNamespaces() {
return namespaces;
}
- public HashMap<TableId,ClientTabletCache> tabletCaches() {
+ public ClientTabletCache getTabletLocationCache(TableId tableId) {
ensureOpen();
- return tabletCaches;
+ return
tabletLocationCache.get(DataLevel.of(tableId)).computeIfAbsent(tableId,
+ (TableId key) -> {
+ var lockChecker = getTServerLockChecker();
+ if (AccumuloTable.ROOT.tableId().equals(tableId)) {
+ return new RootClientTabletCache(lockChecker);
+ }
+ var mlo = new MetadataCachedTabletObtainer();
+ if (AccumuloTable.METADATA.tableId().equals(tableId)) {
+ return new ClientTabletCacheImpl(AccumuloTable.METADATA.tableId(),
+ getTabletLocationCache(AccumuloTable.ROOT.tableId()), mlo,
lockChecker);
+ } else {
+ return new ClientTabletCacheImpl(tableId,
+ getTabletLocationCache(AccumuloTable.METADATA.tableId()), mlo,
lockChecker);
+ }
+ });
+ }
+
+ /**
+ * Clear the currently cached tablet locations. The use of ConcurrentHashMap
ensures this is
+ * thread-safe. However, since the ConcurrentHashMap iterator is weakly
consistent, it does not
+ * block new locations from being cached. If new locations are added while
this is executing, they
+ * may be immediately invalidated by this code. Multiple calls to this
method in different threads
+ * may cause some location caches to be invalidated multiple times. That is
okay, because cache
+ * invalidation is idempotent.
+ */
+ public void clearTabletLocationCache() {
+ tabletLocationCache.forEach((dataLevel, map) -> {
+ // use iter.remove() instead of calling clear() on the map, to prevent
clearing entries that
+ // may not have been invalidated
+ var iter = map.values().iterator();
+ while (iter.hasNext()) {
+ iter.next().invalidate();
+ iter.remove();
Review Comment:
The BatchWriter, BatchScanner, and ConditoinalWriter get a reference to a
tablet locator and then keep it. So then something like the following could
happen
1. Create a BatchWriter which gets a ref to a cache C1 for table TAB1.
2. clearTabletLocationCache() is called
3. Create a BatchWriter which gets a ref to a cache C2 for table TAB1
So now there two batch writers that reference two different cache instances
for the same table. Things probably should not hold on to references to the
cache. The code seems to do that because it wraps the cache w/ decorator
objects.
--
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]
