keith-turner commented on code in PR #6156:
URL: https://github.com/apache/accumulo/pull/6156#discussion_r2879219068
##########
core/src/main/java/org/apache/accumulo/core/conf/ClientProperty.java:
##########
@@ -141,7 +141,25 @@ public enum ClientProperty {
"A list of span receiver classes to send trace spans"),
@Deprecated(since = "2.1.0", forRemoval = true)
TRACE_ZOOKEEPER_PATH("trace.zookeeper.path", Constants.ZTRACERS,
PropertyType.PATH,
- "The zookeeper node where tracers are registered", "2.0.0", false);
+ "The zookeeper node where tracers are registered", "2.0.0", false),
+
+ /*
+ * For use with OfflineTabletLocatorImpl
+ */
+ OFFLINE_LOCATOR_CACHE_DURATION("offline.locator.cache.duration", "10m",
PropertyType.TIMEDURATION,
+ "Amount of time for which offline extent information should be cached in
the client. The offline"
Review Comment:
This reads like things are always removed after the time period. Maybe it
should say that extents that are not used within this time period are removed.
Also maybe all these new properties should be marked as experimental until
we see how this shakes out in the 4.0 code? The 4.0 code already caches
"offline" tablets, however it does not do any sort of eviction. If the 4.0
code did do eviction it would probably be more general and apply to all tablet,
not just "offline" tablets so its property names would be more general.
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TabletLocator.java:
##########
@@ -135,24 +138,31 @@ static synchronized void enable() {
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);
- if (tl == null) {
- MetadataLocationObtainer mlo = new MetadataLocationObtainer();
-
- if (RootTable.ID.equals(tableId)) {
- tl = new RootTabletLocator(context.getTServerLockChecker());
- } else if (MetadataTable.ID.equals(tableId)) {
- tl = new TabletLocatorImpl(MetadataTable.ID, getLocator(context,
RootTable.ID), mlo,
- context.getTServerLockChecker());
- } else {
- tl = new TabletLocatorImpl(tableId, getLocator(context,
MetadataTable.ID), mlo,
- context.getTServerLockChecker());
+ TableState state = context.getTableState(tableId);
+ if (state == TableState.OFFLINE) {
Review Comment:
Could online locators be removed here like it remove offline in the else
below?
--
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]
