keith-turner commented on code in PR #5282:
URL: https://github.com/apache/accumulo/pull/5282#discussion_r1929331849
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TabletLocator.java:
##########
@@ -81,62 +76,17 @@ public abstract List<Range> binRanges(ClientContext
context, List<Range> ranges,
*/
public abstract void invalidateCache(ClientContext context, String server);
- private static class LocatorKey {
- InstanceId instanceId;
- TableId tableId;
-
- LocatorKey(InstanceId instanceId, TableId table) {
- this.instanceId = instanceId;
- this.tableId = table;
- }
-
- @Override
- public int hashCode() {
- return instanceId.hashCode() + tableId.hashCode();
- }
-
- @Override
- public boolean equals(Object o) {
- if (o instanceof LocatorKey) {
- return equals((LocatorKey) o);
- }
- return false;
- }
-
- public boolean equals(LocatorKey lk) {
- return instanceId.equals(lk.instanceId) && tableId.equals(lk.tableId);
- }
-
- }
-
- private static final HashMap<LocatorKey,TabletLocator> locators = new
HashMap<>();
- private static boolean enabled = true;
-
- public static synchronized void clearLocators() {
+ public static synchronized void clearLocators(ClientContext context) {
+ final var locators = context.tabletLocators();
for (TabletLocator locator : locators.values()) {
locator.isValid = false;
}
locators.clear();
}
- static synchronized boolean isEnabled() {
- return enabled;
- }
-
- static synchronized void disable() {
- clearLocators();
- enabled = false;
- }
-
- static synchronized void enable() {
- enabled = true;
- }
-
public static synchronized TabletLocator getLocator(ClientContext context,
TableId tableId) {
- Preconditions.checkState(enabled, "The Accumulo singleton that that tracks
tablet locations is "
- + "disabled. This is likely caused by all AccumuloClients being closed
or garbage collected");
- LocatorKey key = new LocatorKey(context.getInstanceID(), tableId);
- TabletLocator tl = locators.get(key);
+ final var locators = context.tabletLocators();
Review Comment:
Could eventually transition away from this static synchronization and use
concurrent hash map for the table map and sync in its compute function. Could
also remove these static methods and transition this to the client context
which internalyl uses a concurrent hash map for sync.
Wonder if the code in the method were replaced w/ a single call to client
context if it would be easy to inline this method.
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java:
##########
@@ -340,7 +340,7 @@ private void
logBusyTablets(List<ComparablePair<Long,KeyExtent>> busyTablets,
this.security = context.getSecurityOperation();
watchCriticalScheduledTask(context.getScheduledExecutor().scheduleWithFixedDelay(
- TabletLocator::clearLocators, jitter(), jitter(),
TimeUnit.MILLISECONDS));
+ () -> TabletLocator.clearLocators(context), jitter(), jitter(),
TimeUnit.MILLISECONDS));
Review Comment:
This may be able to be removed, needs investigation. I think the reason this
code is here is that bulk v1 code would populate user tablet info in the
locator cache. This would periodically clear that in case user tables were
deleted to avoid memory build up. Now the tablet server should only have
accumulo tables in the tablet locator cache.
--
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]
