This is an automated email from the ASF dual-hosted git repository.
kturner pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/elasticity by this push:
new 063d12057a Moves reading root tablet metadata w/ zoocache out of Ample
(#4673)
063d12057a is described below
commit 063d12057adeaa98810f560eb663609b12302c99
Author: Keith Turner <[email protected]>
AuthorDate: Fri Jun 14 16:27:23 2024 -0400
Moves reading root tablet metadata w/ zoocache out of Ample (#4673)
After the changes in #4669 only a single location in the code was
reading tablet metadata w/ eventual consistency using Ample. This
single location was the root tablet location cache. Since this
was the only use of the code and its a highly nuanced use case,
moved the code out of Ample to the only place that was using it.
Removing the levels of indirection makes it easier to understand
the root tablet location cache. Also added some comments about
what the the root tablet location cache is doing and noticed and
uneeded cache clear it was doing and removed it.
---
.../core/clientImpl/RootClientTabletCache.java | 42 ++++++++++++++------
.../accumulo/core/metadata/schema/Ample.java | 33 +---------------
.../accumulo/core/metadata/schema/AmpleImpl.java | 5 +--
.../core/metadata/schema/RootTabletMetadata.java | 25 +++++++-----
.../core/metadata/schema/TabletsMetadata.java | 45 ++--------------------
.../core/clientImpl/RootClientTabletCacheTest.java | 14 +++++--
.../server/manager/state/ZooTabletStateStore.java | 3 +-
7 files changed, 63 insertions(+), 104 deletions(-)
diff --git
a/core/src/main/java/org/apache/accumulo/core/clientImpl/RootClientTabletCache.java
b/core/src/main/java/org/apache/accumulo/core/clientImpl/RootClientTabletCache.java
index 4e3af2a5d6..e3e959e536 100644
---
a/core/src/main/java/org/apache/accumulo/core/clientImpl/RootClientTabletCache.java
+++
b/core/src/main/java/org/apache/accumulo/core/clientImpl/RootClientTabletCache.java
@@ -19,9 +19,9 @@
package org.apache.accumulo.core.clientImpl;
import static
com.google.common.util.concurrent.Uninterruptibles.sleepUninterruptibly;
+import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;
-import static
org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.LOCATION;
import java.util.Collection;
import java.util.Collections;
@@ -30,15 +30,13 @@ import java.util.Map;
import java.util.Optional;
import java.util.function.BiConsumer;
-import org.apache.accumulo.core.Constants;
import org.apache.accumulo.core.client.admin.TabletAvailability;
import
org.apache.accumulo.core.clientImpl.ClientTabletCacheImpl.TabletServerLockChecker;
import org.apache.accumulo.core.data.Mutation;
import org.apache.accumulo.core.data.Range;
import org.apache.accumulo.core.dataImpl.KeyExtent;
-import org.apache.accumulo.core.fate.zookeeper.ZooCache;
import org.apache.accumulo.core.metadata.RootTable;
-import org.apache.accumulo.core.metadata.schema.Ample.ReadConsistency;
+import org.apache.accumulo.core.metadata.schema.RootTabletMetadata;
import org.apache.accumulo.core.metadata.schema.TabletMetadata.Location;
import org.apache.accumulo.core.metadata.schema.TabletMetadata.LocationType;
import org.apache.accumulo.core.util.OpTimer;
@@ -46,6 +44,20 @@ import org.apache.hadoop.io.Text;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+/**
+ * Provides ability to get the location of the root tablet from a zoocache.
This cache
+ * implementation does not actually do any caching on its own and soley relies
on zoocache. One nice
+ * feature of using zoo cache is that if the location changes in zookeeper, it
will eventually be
+ * updated by zookeeper watchers in zoocache. Therefore, the invalidation
functions are
+ * intentionally no-ops and rely on the zookeeper watcher to keep things up to
date.
+ *
+ * <p>
+ * This code is relying on the assumption that if two client objects are
created for the same
+ * accumulo instance in the same process that both will have the same
zoocache. This assumption
+ * means there is only a single zookeeper watch per process per accumulo
instance. This assumptions
+ * leads to efficiencies at the cluster level by reduce the total number of
zookeeper watches.
+ * </p>
+ */
public class RootClientTabletCache extends ClientTabletCache {
private final TabletServerLockChecker lockChecker;
@@ -87,20 +99,24 @@ public class RootClientTabletCache extends
ClientTabletCache {
}
@Override
- public void invalidateCache(KeyExtent failedExtent) {}
+ public void invalidateCache(KeyExtent failedExtent) {
+ // no-op see class level javadoc
+ }
@Override
- public void invalidateCache(Collection<KeyExtent> keySet) {}
+ public void invalidateCache(Collection<KeyExtent> keySet) {
+ // no-op see class level javadoc
+ }
@Override
public void invalidateCache(ClientContext context, String server) {
- ZooCache zooCache = context.getZooCache();
- String root = context.getZooKeeperRoot() + Constants.ZTSERVERS;
- zooCache.clear(root + "/" + server);
+ // no-op see class level javadoc
}
@Override
- public void invalidateCache() {}
+ public void invalidateCache() {
+ // no-op see class level javadoc
+ }
protected CachedTablet getRootTabletLocation(ClientContext context) {
Logger log = LoggerFactory.getLogger(this.getClass());
@@ -113,8 +129,10 @@ public class RootClientTabletCache extends
ClientTabletCache {
timer = new OpTimer().start();
}
- Location loc = context.getAmple()
- .readTablet(RootTable.EXTENT, ReadConsistency.EVENTUAL,
LOCATION).getLocation();
+ var zpath = RootTabletMetadata.zooPath(context);
+ var zooCache = context.getZooCache();
+ Location loc = new RootTabletMetadata(new String(zooCache.get(zpath),
UTF_8)).toTabletMetadata()
+ .getLocation();
if (timer != null) {
timer.stop();
diff --git
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java
index 8243ad50d4..888775aed5 100644
--- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java
+++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java
@@ -121,24 +121,6 @@ public interface Ample {
}
}
- /**
- * Controls how Accumulo metadata is read. Currently this only impacts
reading the root tablet
- * stored in Zookeeper. Reading data stored in the Accumulo metadata table
is always immediate
- * consistency.
- */
- public enum ReadConsistency {
- /**
- * Read data in a way that is slower, but should always yield the latest
data. In addition to
- * being slower, it's possible this read consistency can place higher load
on shared resource
- * which can negatively impact an entire cluster.
- */
- IMMEDIATE,
- /**
- * Read data in a way that may be faster but may yield out of date data.
- */
- EVENTUAL
- }
-
/**
* Enables status based processing of GcCandidates.
*/
@@ -157,26 +139,13 @@ public interface Ample {
INVALID
}
- /**
- * Read a single tablets metadata. No checking is done for prev row, so it
could differ. The
- * method will read the data using {@link ReadConsistency#IMMEDIATE}.
- *
- * @param extent Reads tablet metadata using the table id and end row from
this extent.
- * @param colsToFetch What tablets columns to fetch. If empty, then
everything is fetched.
- */
- default TabletMetadata readTablet(KeyExtent extent, ColumnType...
colsToFetch) {
- return readTablet(extent, ReadConsistency.IMMEDIATE, colsToFetch);
- }
-
/**
* Read a single tablets metadata. No checking is done for prev row, so it
could differ.
*
* @param extent Reads tablet metadata using the table id and end row from
this extent.
- * @param readConsistency Controls how the data is read.
* @param colsToFetch What tablets columns to fetch. If empty, then
everything is fetched.
*/
- TabletMetadata readTablet(KeyExtent extent, ReadConsistency readConsistency,
- ColumnType... colsToFetch);
+ TabletMetadata readTablet(KeyExtent extent, ColumnType... colsToFetch);
/**
* Entry point for reading multiple tablets' metadata. Generates a
TabletsMetadata builder object
diff --git
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/AmpleImpl.java
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/AmpleImpl.java
index cdad8708e6..7b01ea3989 100644
--- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/AmpleImpl.java
+++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/AmpleImpl.java
@@ -46,15 +46,12 @@ public class AmpleImpl implements Ample {
}
@Override
- public TabletMetadata readTablet(KeyExtent extent, ReadConsistency
readConsistency,
- ColumnType... colsToFetch) {
+ public TabletMetadata readTablet(KeyExtent extent, ColumnType...
colsToFetch) {
Options builder = newBuilder().forTablet(extent);
if (colsToFetch.length > 0) {
builder.fetch(colsToFetch);
}
- builder.readConsistency(readConsistency);
-
try (TabletsMetadata tablets = builder.build()) {
return tablets.stream().collect(onlyElement());
} catch (NoSuchElementException e) {
diff --git
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/RootTabletMetadata.java
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/RootTabletMetadata.java
index a89fded331..107e9f4004 100644
---
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/RootTabletMetadata.java
+++
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/RootTabletMetadata.java
@@ -61,18 +61,25 @@ public class RootTabletMetadata {
private static final int VERSION = 1;
+ public static String zooPath(ClientContext ctx) {
+ return ctx.getZooKeeperRoot() + RootTable.ZROOT_TABLET;
+ }
+
/**
* Reads the tablet metadata for the root tablet from zookeeper
*/
- public static RootTabletMetadata read(ClientContext ctx)
- throws InterruptedException, KeeperException {
- final String zpath = ctx.getZooKeeperRoot() + RootTable.ZROOT_TABLET;
- ZooReader zooReader = ctx.getZooReader();
- // attempt (see ZOOKEEPER-1675) to ensure the latest root table metadata
is read from
- // zookeeper
- zooReader.sync(zpath);
- byte[] bytes = zooReader.getData(zpath);
- return new RootTabletMetadata(new String(bytes, UTF_8));
+ public static RootTabletMetadata read(ClientContext ctx) {
+ try {
+ final String zpath = zooPath(ctx);
+ ZooReader zooReader = ctx.getZooReader();
+ // attempt (see ZOOKEEPER-1675) to ensure the latest root table metadata
is read from
+ // zookeeper
+ zooReader.sync(zpath);
+ byte[] bytes = zooReader.getData(zpath);
+ return new RootTabletMetadata(new String(bytes, UTF_8));
+ } catch (KeeperException | InterruptedException e) {
+ throw new IllegalStateException(e);
+ }
}
// This class is used to serialize and deserialize root tablet metadata
using GSon. Any changes to
diff --git
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java
index 2bb8082c4b..a3914ea0ed 100644
---
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java
+++
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java
@@ -19,7 +19,6 @@
package org.apache.accumulo.core.metadata.schema;
import static com.google.common.base.Preconditions.checkState;
-import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.toList;
import static
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN;
@@ -61,12 +60,9 @@ import org.apache.accumulo.core.clientImpl.ClientContext;
import org.apache.accumulo.core.data.Range;
import org.apache.accumulo.core.data.TableId;
import org.apache.accumulo.core.dataImpl.KeyExtent;
-import org.apache.accumulo.core.fate.zookeeper.ZooCache;
import org.apache.accumulo.core.iterators.user.WholeRowIterator;
import org.apache.accumulo.core.metadata.AccumuloTable;
-import org.apache.accumulo.core.metadata.RootTable;
import org.apache.accumulo.core.metadata.schema.Ample.DataLevel;
-import org.apache.accumulo.core.metadata.schema.Ample.ReadConsistency;
import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.BulkFileColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ClonedColumnFamily;
@@ -87,7 +83,6 @@ import
org.apache.accumulo.core.metadata.schema.filters.TabletMetadataFilter;
import org.apache.accumulo.core.security.Authorizations;
import org.apache.accumulo.core.util.ColumnFQ;
import org.apache.hadoop.io.Text;
-import org.apache.zookeeper.KeeperException;
import com.google.common.base.Preconditions;
import com.google.common.collect.Iterators;
@@ -113,7 +108,6 @@ public class TabletsMetadata implements
Iterable<TabletMetadata>, AutoCloseable
private boolean checkConsistency = false;
private boolean saveKeyValues;
private TableId tableId;
- private ReadConsistency readConsistency = ReadConsistency.IMMEDIATE;
private final AccumuloClient _client;
private final List<TabletMetadataFilter> tabletMetadataFilters = new
ArrayList<>();
private final Function<DataLevel,String> tableMapper;
@@ -153,7 +147,7 @@ public class TabletsMetadata implements
Iterable<TabletMetadata>, AutoCloseable
level, table);
if (level == DataLevel.ROOT) {
ClientContext ctx = ((ClientContext) _client);
- return new TabletsMetadata(getRootMetadata(ctx, readConsistency));
+ return new TabletsMetadata(getRootMetadata(ctx));
} else {
return buildNonRoot(_client);
}
@@ -176,8 +170,7 @@ public class TabletsMetadata implements
Iterable<TabletMetadata>, AutoCloseable
for (DataLevel level : groupedExtents.keySet()) {
if (level == DataLevel.ROOT) {
- iterables.add(() -> Iterators
- .singletonIterator(getRootMetadata((ClientContext) client,
readConsistency)));
+ iterables.add(() ->
Iterators.singletonIterator(getRootMetadata((ClientContext) client)));
} else {
try {
BatchScanner scanner =
@@ -496,12 +489,6 @@ public class TabletsMetadata implements
Iterable<TabletMetadata>, AutoCloseable
this.tabletMetadataFilters.add(filter);
return this;
}
-
- @Override
- public Options readConsistency(ReadConsistency readConsistency) {
- this.readConsistency = Objects.requireNonNull(readConsistency);
- return this;
- }
}
public interface Options {
@@ -522,12 +509,6 @@ public class TabletsMetadata implements
Iterable<TabletMetadata>, AutoCloseable
*/
Options saveKeyValues();
- /**
- * Controls how the data is read. If not, set then the default is
- * {@link ReadConsistency#IMMEDIATE}
- */
- Options readConsistency(ReadConsistency readConsistency);
-
/**
* Adds a filter to be applied while fetching the data. Filters are
applied in the order they
* are added. This method can be called multiple times to chain multiple
filters together. The
@@ -662,26 +643,8 @@ public class TabletsMetadata implements
Iterable<TabletMetadata>, AutoCloseable
return new Builder(client, tableMapper);
}
- private static TabletMetadata getRootMetadata(ClientContext ctx,
- ReadConsistency readConsistency) {
- String zkRoot = ctx.getZooKeeperRoot();
- switch (readConsistency) {
- case EVENTUAL:
- return getRootMetadata(zkRoot, ctx.getZooCache());
- case IMMEDIATE:
- try {
- return RootTabletMetadata.read(ctx).toTabletMetadata();
- } catch (InterruptedException | KeeperException e) {
- throw new IllegalStateException(e);
- }
- default:
- throw new IllegalArgumentException("Unknown consistency level " +
readConsistency);
- }
- }
-
- public static TabletMetadata getRootMetadata(String zkRoot, ZooCache zc) {
- byte[] jsonBytes = zc.get(zkRoot + RootTable.ZROOT_TABLET);
- return new RootTabletMetadata(new String(jsonBytes,
UTF_8)).toTabletMetadata();
+ private static TabletMetadata getRootMetadata(ClientContext ctx) {
+ return RootTabletMetadata.read(ctx).toTabletMetadata();
}
private final AutoCloseable closeable;
diff --git
a/core/src/test/java/org/apache/accumulo/core/clientImpl/RootClientTabletCacheTest.java
b/core/src/test/java/org/apache/accumulo/core/clientImpl/RootClientTabletCacheTest.java
index a50a314bd5..178b2e7ec4 100644
---
a/core/src/test/java/org/apache/accumulo/core/clientImpl/RootClientTabletCacheTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/clientImpl/RootClientTabletCacheTest.java
@@ -19,13 +19,16 @@
package org.apache.accumulo.core.clientImpl;
import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.createStrictMock;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.replay;
import static org.easymock.EasyMock.verify;
-import org.apache.accumulo.core.Constants;
+import java.util.List;
+
import
org.apache.accumulo.core.clientImpl.ClientTabletCacheImpl.TabletServerLockChecker;
import org.apache.accumulo.core.fate.zookeeper.ZooCache;
+import org.apache.accumulo.core.metadata.RootTable;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -39,7 +42,7 @@ public class RootClientTabletCacheTest {
public void setUp() {
context = createMock(ClientContext.class);
expect(context.getZooKeeperRoot()).andReturn("/accumulo/iid").anyTimes();
- zc = createMock(ZooCache.class);
+ zc = createStrictMock(ZooCache.class);
expect(context.getZooCache()).andReturn(zc).anyTimes();
replay(context);
lockChecker = createMock(TabletServerLockChecker.class);
@@ -47,10 +50,13 @@ public class RootClientTabletCacheTest {
}
@Test
- public void testInvalidateCache_Server() {
- zc.clear(context.getZooKeeperRoot() + Constants.ZTSERVERS + "/server");
+ public void testInvalidateCache_Noop() {
replay(zc);
+ // its not expected that any of the validate functions will interact w/
zoocache
rtl.invalidateCache(context, "server");
+ rtl.invalidateCache(RootTable.EXTENT);
+ rtl.invalidateCache();
+ rtl.invalidateCache(List.of(RootTable.EXTENT));
verify(zc);
}
}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/manager/state/ZooTabletStateStore.java
b/server/base/src/main/java/org/apache/accumulo/server/manager/state/ZooTabletStateStore.java
index 2b8dded1dd..110563f1e9 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/manager/state/ZooTabletStateStore.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/manager/state/ZooTabletStateStore.java
@@ -47,7 +47,6 @@ import org.apache.accumulo.core.util.time.SteadyTime;
import org.apache.accumulo.server.ServerContext;
import org.apache.accumulo.server.iterators.TabletIteratorEnvironment;
import org.apache.hadoop.fs.Path;
-import org.apache.zookeeper.KeeperException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -87,7 +86,7 @@ class ZooTabletStateStore extends AbstractTabletStateStore
implements TabletStat
Map.of(TabletManagementIterator.TABLET_GOAL_STATE_PARAMS_OPTION,
parameters.serialize()),
env);
tmi.seek(new Range(), null, true);
- } catch (KeeperException | InterruptedException | IOException e2) {
+ } catch (IOException e2) {
throw new IllegalStateException(
"Error setting up TabletManagementIterator for the root tablet", e2);
}
