keith-turner commented on code in PR #5421:
URL: https://github.com/apache/accumulo/pull/5421#discussion_r2014666423
##########
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:
A lot of code will get a reference to a tablet location cache and hang on to
it. That could lead to multiple objects caching data for the same table. This
is not a new problem w/ this PR, seems like a preexisting problem with this
code as the existing code has this same problem.
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java:
##########
@@ -229,6 +233,12 @@ public ClientContext(ClientInfo info,
AccumuloConfiguration serverConf,
this.info = info;
this.hadoopConf = info.getHadoopConf();
+ var tabletCache = new
HashMap<DataLevel,ConcurrentHashMap<TableId,ClientTabletCache>>();
Review Comment:
Could use an EnumMap here
--
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]
