From 5369f535f3e6d3ecbc6df4aa8600da5c4cae398b Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Tue, 21 Feb 2023 12:27:58 +0000 Subject: [PATCH 01/12] wip --- .../org/apache/accumulo/core/Constants.java | 2 + .../server/metadata/TabletMetadataCache.java | 138 ++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java diff --git a/core/src/main/java/org/apache/accumulo/core/Constants.java b/core/src/main/java/org/apache/accumulo/core/Constants.java index 5d37166538e..075d61c9a7c 100644 --- a/core/src/main/java/org/apache/accumulo/core/Constants.java +++ b/core/src/main/java/org/apache/accumulo/core/Constants.java @@ -47,6 +47,8 @@ public class Constants { public static final String ZTABLE_COMPACT_CANCEL_ID = "/compact-cancel-id"; public static final String ZTABLE_NAMESPACE = "/namespace"; + public static final String ZTABLET_CACHE = "/tablet_cache"; + public static final String ZNAMESPACES = "/namespaces"; public static final String ZNAMESPACE_NAME = "/name"; diff --git a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java new file mode 100644 index 00000000000..1f072598c12 --- /dev/null +++ b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java @@ -0,0 +1,138 @@ +package org.apache.accumulo.server.metadata; + +import static java.nio.charset.StandardCharsets.UTF_8; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.accumulo.core.Constants; +import org.apache.accumulo.core.clientImpl.AcceptableThriftTableOperationException; +import org.apache.accumulo.core.data.InstanceId; +import org.apache.accumulo.core.dataImpl.KeyExtent; +import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.core.metadata.schema.TabletMetadata; +import org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType; +import org.apache.accumulo.server.ServerContext; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.WatchedEvent; +import org.apache.zookeeper.Watcher; + +public class TabletMetadataCache { + + private static final Long INITIAL_VALUE = Long.valueOf(0); + private final Map cache = new HashMap<>(); + private final ServerContext ctx; + private final InstanceId iid; + private final ZooReaderWriter zrw; + + public TabletMetadataCache(ServerContext ctx) { + this.ctx = ctx; + this.iid = this.ctx.getInstanceID(); + this.zrw = this.ctx.getZooReaderWriter(); + } + + public void put(final KeyExtent extent, final TabletMetadata metadata) + throws KeeperException, InterruptedException, AcceptableThriftTableOperationException { + TabletMetadata priorVal = cache.putIfAbsent(extent, metadata); + if (priorVal == null) { + final String path = getPath(extent); + this.zrw.mutateOrCreate(path, serialize(INITIAL_VALUE), (currVal) -> { + return serialize(deserialize(currVal) + 1); + }); + watch(path, extent); + } + } + + private void watch(final String path, final KeyExtent extent) { + + final Watcher watcher = new Watcher() { + @Override + public void process(WatchedEvent event) { + switch (event.getType()) { + case NodeChildrenChanged: + case ChildWatchRemoved: + case PersistentWatchRemoved: + case DataWatchRemoved: + case NodeCreated: + // I don't think we care about these cases, but we need + // to recreate this watcher + watch(path, extent); + break; + case None: + Event.KeeperState state = event.getState(); + switch (state) { + case AuthFailed: + case Closed: + case ConnectedReadOnly: + case Disconnected: + case Expired: + // remove all entries on connection issue + cache.clear(); + // case NoSyncConnected: + // case SaslAuthenticated: + // case SyncConnected: + // case Unknown: + default: + // don't care + break; + } + case NodeDataChanged: + case NodeDeleted: + remove(extent); + break; + default: + break; + } + } + }; + // Place the watcher + try { + this.zrw.exists(path, watcher); + } catch (KeeperException | InterruptedException e) { + // TODO: Handle this + } + } + + public TabletMetadata get(final KeyExtent extent) + throws AcceptableThriftTableOperationException, KeeperException, InterruptedException { + final String path = getPath(extent); + TabletMetadata val = cache.computeIfAbsent(extent, (k) -> { + return this.ctx.getAmple().readTablet(k, ColumnType.values()); + }); + this.zrw.mutateOrCreate(path, serialize(INITIAL_VALUE), (currVal) -> { + return serialize(deserialize(currVal) + 1); + }); + watch(path, extent); + return val; + } + + public void remove(KeyExtent extent) { + TabletMetadata priorVal = cache.remove(extent); + if (priorVal != null) { + try { + this.zrw.delete(getPath(extent)); + } catch (InterruptedException | KeeperException e) { + // TODO: handle this + } + } + } + + private String getPath(KeyExtent extent) { + return Constants.ZROOT + "/" + this.iid + Constants.ZTABLET_CACHE + "/" + extent.toString(); + } + + private Long deserialize(byte[] value) { + return Long.parseLong(new String(value, UTF_8)); + } + + private byte[] serialize(Long value) { + return value.toString().getBytes(UTF_8); + } + + public void tabletMetadataChanged(KeyExtent extent) + throws AcceptableThriftTableOperationException, KeeperException, InterruptedException { + this.zrw.mutateOrCreate(getPath(extent), serialize(INITIAL_VALUE), (currVal) -> { + return serialize(deserialize(currVal) + 1); + }); + } +} From d4fb9d4743aaba3c292de1324fa55dd9383f1cf3 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Tue, 21 Feb 2023 22:51:58 +0000 Subject: [PATCH 02/12] Updates to use CuratorCache, added tests --- .../core/metadata/schema/TabletMetadata.java | 2 +- pom.xml | 7 +- server/base/pom.xml | 4 + .../apache/accumulo/server/ServerContext.java | 22 ++ .../server/init/ZooKeeperInitializer.java | 2 + .../server/metadata/TabletMetadataCache.java | 267 ++++++++++++------ .../server/metadata/TabletMutatorBase.java | 4 +- .../server/metadata/TabletMutatorImpl.java | 1 + .../functional/TabletMetadataCacheIT.java | 235 +++++++++++++++ 9 files changed, 451 insertions(+), 93 deletions(-) create mode 100644 test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java index d1a83772cdd..c2b24f4bffb 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java @@ -520,7 +520,7 @@ private void setLocationOnce(String val, String qual, LocationType lt) { } @VisibleForTesting - static TabletMetadata create(String id, String prevEndRow, String endRow) { + public static TabletMetadata create(String id, String prevEndRow, String endRow) { TabletMetadata te = new TabletMetadata(); te.tableId = TableId.of(id); te.sawPrevEndRow = true; diff --git a/pom.xml b/pom.xml index d3ebe9291a9..90fd60d0d8e 100644 --- a/pom.xml +++ b/pom.xml @@ -119,7 +119,7 @@ 1.70 - 5.3.0 + 5.4.0 ${project.parent.basedir}/contrib/Eclipse-Accumulo-Codestyle.xml 2.15.0 @@ -452,6 +452,11 @@ curator-framework ${curator.version} + + org.apache.curator + curator-recipes + ${curator.version} + org.apache.curator curator-test diff --git a/server/base/pom.xml b/server/base/pom.xml index 5888ffa259d..81047009d00 100644 --- a/server/base/pom.xml +++ b/server/base/pom.xml @@ -84,6 +84,10 @@ org.apache.commons commons-lang3 + + org.apache.curator + curator-recipes + org.apache.hadoop hadoop-client-api diff --git a/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java b/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java index 90cb171b884..82ae5bdbace 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java +++ b/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java @@ -68,6 +68,7 @@ import org.apache.accumulo.server.fs.VolumeManager; import org.apache.accumulo.server.mem.LowMemoryDetector; import org.apache.accumulo.server.metadata.ServerAmpleImpl; +import org.apache.accumulo.server.metadata.TabletMetadataCache; import org.apache.accumulo.server.rpc.SaslServerConnectionParams; import org.apache.accumulo.server.rpc.ThriftServerType; import org.apache.accumulo.server.security.AuditedSecurityOperation; @@ -87,6 +88,7 @@ * and have access to the system files and configuration. */ public class ServerContext extends ClientContext { + private static final Logger log = LoggerFactory.getLogger(ServerContext.class); private final ServerInfo info; @@ -104,6 +106,9 @@ public class ServerContext extends ClientContext { private final Supplier securityOperation; private final Supplier cryptoFactorySupplier; private final Supplier lowMemoryDetector; + private final Supplier tabletMetadataCache; + + private volatile boolean tabletMetadataCacheInitialized = false; public ServerContext(SiteConfiguration siteConfig) { this(new ServerInfo(siteConfig)); @@ -130,6 +135,7 @@ private ServerContext(ServerInfo info) { memoize(() -> new AuditedSecurityOperation(this, SecurityOperation.getAuthorizor(this), SecurityOperation.getAuthenticator(this), SecurityOperation.getPermHandler(this))); lowMemoryDetector = memoize(() -> new LowMemoryDetector()); + tabletMetadataCache = memoize(() -> new TabletMetadataCache(this)); } /** @@ -460,4 +466,20 @@ public LowMemoryDetector getLowMemoryDetector() { return lowMemoryDetector.get(); } + public TabletMetadataCache getTabletMetadataCache() { + try { + return tabletMetadataCache.get(); + } finally { + tabletMetadataCacheInitialized = true; + } + } + + @Override + public synchronized void close() { + super.close(); + if (tabletMetadataCacheInitialized) { + this.tabletMetadataCache.get().close(); + } + } + } diff --git a/server/base/src/main/java/org/apache/accumulo/server/init/ZooKeeperInitializer.java b/server/base/src/main/java/org/apache/accumulo/server/init/ZooKeeperInitializer.java index a939d237c14..5e68d019ec5 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/init/ZooKeeperInitializer.java +++ b/server/base/src/main/java/org/apache/accumulo/server/init/ZooKeeperInitializer.java @@ -162,6 +162,8 @@ void initialize(final ServerContext context, final boolean clearInstanceName, ZooUtil.NodeExistsPolicy.FAIL); zoo.putPersistentData(zkInstanceRoot + Constants.ZSSERVERS, EMPTY_BYTE_ARRAY, ZooUtil.NodeExistsPolicy.FAIL); + zoo.putPersistentData(zkInstanceRoot + Constants.ZTABLET_CACHE, EMPTY_BYTE_ARRAY, + ZooUtil.NodeExistsPolicy.FAIL); } /** diff --git a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java index 1f072598c12..a94e969d1eb 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java +++ b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java @@ -1,138 +1,225 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ package org.apache.accumulo.server.metadata; import static java.nio.charset.StandardCharsets.UTF_8; -import java.util.HashMap; -import java.util.Map; +import java.io.Closeable; +import java.util.concurrent.CountDownLatch; import org.apache.accumulo.core.Constants; import org.apache.accumulo.core.clientImpl.AcceptableThriftTableOperationException; -import org.apache.accumulo.core.data.InstanceId; +import org.apache.accumulo.core.data.TableId; import org.apache.accumulo.core.dataImpl.KeyExtent; import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter; import org.apache.accumulo.core.metadata.schema.TabletMetadata; import org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType; import org.apache.accumulo.server.ServerContext; +import org.apache.curator.framework.CuratorFramework; +import org.apache.curator.framework.CuratorFrameworkFactory; +import org.apache.curator.framework.WatcherRemoveCuratorFramework; +import org.apache.curator.framework.recipes.cache.ChildData; +import org.apache.curator.framework.recipes.cache.CuratorCache; +import org.apache.curator.framework.recipes.cache.CuratorCacheBuilder; +import org.apache.curator.framework.recipes.cache.CuratorCacheListener; +import org.apache.curator.framework.recipes.cache.CuratorCacheStorage; +import org.apache.curator.retry.RetryNTimes; +import org.apache.hadoop.io.Text; import org.apache.zookeeper.KeeperException; -import org.apache.zookeeper.WatchedEvent; -import org.apache.zookeeper.Watcher; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; -public class TabletMetadataCache { +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.LoadingCache; +import com.github.benmanes.caffeine.cache.Scheduler; +import com.github.benmanes.caffeine.cache.stats.CacheStats; +/** + * Object that uses ZooKeeper for signaling Tablet metadata changes to keep a cache of Tablet + * metadata up to date. + * + */ +public class TabletMetadataCache implements Closeable { + + private static final Logger LOG = LoggerFactory.getLogger(TabletMetadataCache.class); private static final Long INITIAL_VALUE = Long.valueOf(0); - private final Map cache = new HashMap<>(); - private final ServerContext ctx; - private final InstanceId iid; + + private final String znodeBasePath; private final ZooReaderWriter zrw; + private final CuratorFramework cf; + private final WatcherRemoveCuratorFramework watcherWrapper; + private final CuratorCache tabletStateCache; + private final LoadingCache tabletMetadataCache; - public TabletMetadataCache(ServerContext ctx) { - this.ctx = ctx; - this.iid = this.ctx.getInstanceID(); - this.zrw = this.ctx.getZooReaderWriter(); - } + public TabletMetadataCache(final ServerContext ctx) { - public void put(final KeyExtent extent, final TabletMetadata metadata) - throws KeeperException, InterruptedException, AcceptableThriftTableOperationException { - TabletMetadata priorVal = cache.putIfAbsent(extent, metadata); - if (priorVal == null) { - final String path = getPath(extent); - this.zrw.mutateOrCreate(path, serialize(INITIAL_VALUE), (currVal) -> { - return serialize(deserialize(currVal) + 1); - }); - watch(path, extent); - } - } + this.zrw = ctx.getZooReaderWriter(); + + final String rootPath = Constants.ZROOT + "/" + ctx.getInstanceID() + Constants.ZTABLET_CACHE; + znodeBasePath = rootPath + "/"; + + // TODO: Likely want to set a max size and eviction parameters + tabletMetadataCache = + Caffeine.newBuilder().recordStats().scheduler(Scheduler.systemScheduler()).build((k) -> { + try { + TabletMetadata tm = ctx.getAmple().readTablet(k, ColumnType.values()); + LOG.info("Loading tablet metadata for extent: {}. Returned: {}", k, tm); + return tm; + } catch (Exception e) { + LOG.error("Error loading tablet metadata for extent: {}", k, e); + throw e; + } + }); - private void watch(final String path, final KeyExtent extent) { + CuratorFrameworkFactory.Builder curatorBuilder = CuratorFrameworkFactory.builder(); + curatorBuilder.connectString(ctx.getZooKeepers()); + curatorBuilder.sessionTimeoutMs(ctx.getZooKeepersSessionTimeOut()); + curatorBuilder.retryPolicy(new RetryNTimes(5, 2000)); + cf = curatorBuilder.build(); + cf.start(); + watcherWrapper = cf.newWatcherRemoveCuratorFramework(); - final Watcher watcher = new Watcher() { + CuratorCacheBuilder cacheBuilder = CuratorCache.builder(watcherWrapper, rootPath); + cacheBuilder.withStorage(CuratorCacheStorage.standard()); + this.tabletStateCache = cacheBuilder.build(); + CountDownLatch initializedLatch = new CountDownLatch(1); + CuratorCacheListener listener = new CuratorCacheListener() { @Override - public void process(WatchedEvent event) { - switch (event.getType()) { - case NodeChildrenChanged: - case ChildWatchRemoved: - case PersistentWatchRemoved: - case DataWatchRemoved: - case NodeCreated: - // I don't think we care about these cases, but we need - // to recreate this watcher - watch(path, extent); - break; - case None: - Event.KeeperState state = event.getState(); - switch (state) { - case AuthFailed: - case Closed: - case ConnectedReadOnly: - case Disconnected: - case Expired: - // remove all entries on connection issue - cache.clear(); - // case NoSyncConnected: - // case SaslAuthenticated: - // case SyncConnected: - // case Unknown: - default: - // don't care - break; - } - case NodeDataChanged: - case NodeDeleted: - remove(extent); + public void event(Type type, ChildData oldData, ChildData newData) { + LOG.info("Received event type: {}, old: {}, new: {}", type, oldData, newData); + String path = newData.getPath(); + switch (type) { + case NODE_CREATED: + // Do nothing, cache will be populated on clients first call to get() break; + case NODE_CHANGED: + case NODE_DELETED: default: + // Remove the tablet metadata cache entry on update or delete. + KeyExtent extent = getExtent(path); + LOG.info("Invalidating extent: {}", extent); + tabletMetadataCache.invalidate(extent); break; + } } + + @Override + public void initialized() { + initializedLatch.countDown(); + } }; - // Place the watcher + tabletStateCache.listenable() + .addListener(CuratorCacheListener.builder().forAll(listener).afterInitialized().build()); + tabletStateCache.start(); try { - this.zrw.exists(path, watcher); - } catch (KeeperException | InterruptedException e) { - // TODO: Handle this + initializedLatch.await(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException("Thread interrupted waiting for tabletStateCache to initialize", + e); } } - public TabletMetadata get(final KeyExtent extent) - throws AcceptableThriftTableOperationException, KeeperException, InterruptedException { + protected KeyExtent getExtent(String path) { + return deserializeKeyExtent(path.substring(znodeBasePath.length())); + } + + protected String getPath(KeyExtent extent) { + return znodeBasePath + serializeKeyExtent(extent); + } + + public TabletMetadata get(KeyExtent extent) { + return tabletMetadataCache.get(extent); + } + + protected int getTabletMetadataCacheSize() { + return tabletMetadataCache.asMap().size(); + } + + protected CacheStats getTabletMetadataCacheStats() { + return tabletMetadataCache.stats(); + } + + /** + * Updates the data of the znode for this extent which will trigger TabletMetadataCache watchers + * to reload the TabletMetadata for this extent. + * + * @param extent + */ + public void tabletMetadataChanged(KeyExtent extent) { final String path = getPath(extent); - TabletMetadata val = cache.computeIfAbsent(extent, (k) -> { - return this.ctx.getAmple().readTablet(k, ColumnType.values()); - }); - this.zrw.mutateOrCreate(path, serialize(INITIAL_VALUE), (currVal) -> { - return serialize(deserialize(currVal) + 1); - }); - watch(path, extent); - return val; + try { + // remove entry from local cache + LOG.info("Invalidating extent due to local tablet change: {}", extent); + tabletMetadataCache.invalidate(extent); + // mutate ZK entry for this tablet + this.zrw.mutateOrCreate(path, serializeData(INITIAL_VALUE), (currVal) -> { + return serializeData(deserializeData(currVal) + 1); + }); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException("Interrupted updating ZooKeeper at: " + path, e); + + } catch (AcceptableThriftTableOperationException | KeeperException e) { + throw new RuntimeException("Error updating ZooKeeper at: " + path, e); + } } - public void remove(KeyExtent extent) { - TabletMetadata priorVal = cache.remove(extent); - if (priorVal != null) { - try { - this.zrw.delete(getPath(extent)); - } catch (InterruptedException | KeeperException e) { - // TODO: handle this - } + public static String serializeKeyExtent(KeyExtent ke) { + Text entry = new Text(ke.tableId().canonical()); + entry.append(new byte[] {';'}, 0, 1); + if (ke.endRow() != null) { + entry.append(ke.endRow().getBytes(), 0, ke.endRow().getLength()); + } + entry.append(new byte[] {';'}, 0, 1); + if (ke.prevEndRow() != null) { + entry.append(ke.prevEndRow().getBytes(), 0, ke.prevEndRow().getLength()); } + return entry.toString(); } - private String getPath(KeyExtent extent) { - return Constants.ZROOT + "/" + this.iid + Constants.ZTABLET_CACHE + "/" + extent.toString(); + private static KeyExtent deserializeKeyExtent(String zkNode) { + String[] parts = zkNode.split(";"); + assert (parts.length == 3); + String tid = parts[0]; + String end = parts[1]; + String prev = parts[2]; + return new KeyExtent(TableId.of(tid), end == null ? null : new Text(end), + prev == null ? null : new Text(prev)); } - private Long deserialize(byte[] value) { + private static Long deserializeData(byte[] value) { return Long.parseLong(new String(value, UTF_8)); } - private byte[] serialize(Long value) { + private static byte[] serializeData(Long value) { return value.toString().getBytes(UTF_8); } - public void tabletMetadataChanged(KeyExtent extent) - throws AcceptableThriftTableOperationException, KeeperException, InterruptedException { - this.zrw.mutateOrCreate(getPath(extent), serialize(INITIAL_VALUE), (currVal) -> { - return serialize(deserialize(currVal) + 1); - }); + @Override + public void close() { + tabletMetadataCache.invalidateAll(); + tabletMetadataCache.cleanUp(); + tabletStateCache.close(); + watcherWrapper.removeWatchers(); + cf.close(); } + } diff --git a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java index 62d13017b49..f252fa596e3 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java +++ b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java @@ -56,13 +56,15 @@ public abstract class TabletMutatorBase implements Ample.TabletMutator { - private final ServerContext context; + protected final ServerContext context; + protected final KeyExtent extent; private final Mutation mutation; protected AutoCloseable closeAfterMutate; private boolean updatesEnabled = true; protected TabletMutatorBase(ServerContext context, KeyExtent extent) { this.context = context; + this.extent = extent; mutation = new Mutation(extent.toMetaRow()); } diff --git a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorImpl.java b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorImpl.java index e4889871da8..c68e09ce771 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorImpl.java +++ b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorImpl.java @@ -43,6 +43,7 @@ public void mutate() { } catch (Exception e) { throw new RuntimeException(e); } + this.context.getTabletMetadataCache().tabletMetadataChanged(extent); } } diff --git a/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java b/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java new file mode 100644 index 00000000000..3a077f10c26 --- /dev/null +++ b/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java @@ -0,0 +1,235 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.accumulo.test.functional; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.apache.accumulo.harness.AccumuloITBase.ZOOKEEPER_TESTING_SERVER; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.File; +import java.util.UUID; + +import org.apache.accumulo.core.Constants; +import org.apache.accumulo.core.clientImpl.AcceptableThriftTableOperationException; +import org.apache.accumulo.core.data.InstanceId; +import org.apache.accumulo.core.data.TableId; +import org.apache.accumulo.core.dataImpl.KeyExtent; +import org.apache.accumulo.core.fate.zookeeper.ZooUtil; +import org.apache.accumulo.core.metadata.schema.AmpleImpl; +import org.apache.accumulo.core.metadata.schema.TabletMetadata; +import org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType; +import org.apache.accumulo.server.ServerContext; +import org.apache.accumulo.server.metadata.TabletMetadataCache; +import org.apache.accumulo.test.zookeeper.ZooKeeperTestingServer; +import org.apache.hadoop.io.Text; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.ZooKeeper; +import org.easymock.EasyMock; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.github.benmanes.caffeine.cache.stats.CacheStats; + +@Tag(ZOOKEEPER_TESTING_SERVER) +public class TabletMetadataCacheIT { + + /** + * Class for test that exists to expose protected methods for test + */ + public static class TestTabletMetadataCache extends TabletMetadataCache { + + public TestTabletMetadataCache(ServerContext ctx) { + super(ctx); + } + + @Override + public KeyExtent getExtent(String path) { + return super.getExtent(path); + } + + @Override + public String getPath(KeyExtent extent) { + return super.getPath(extent); + } + + @Override + public int getTabletMetadataCacheSize() { + return super.getTabletMetadataCacheSize(); + } + + @Override + protected CacheStats getTabletMetadataCacheStats() { + return super.getTabletMetadataCacheStats(); + } + + } + + @TempDir + private static File tempDir; + + private static final InstanceId IID = InstanceId.of(UUID.randomUUID()); + + private static ZooKeeperTestingServer szk = null; + private static ZooKeeper zooKeeper; + + private ServerContext context; + private KeyExtent ke1, ke2, ke3, ke4; + private TabletMetadata tm1, tm2, tm3, tm4; + + @BeforeAll + public static void setup() throws Exception { + szk = new ZooKeeperTestingServer(tempDir); + szk.initPaths(ZooUtil.getRoot(IID) + Constants.ZTABLET_CACHE); + zooKeeper = szk.getZooKeeper(); + ZooUtil.digestAuth(zooKeeper, ZooKeeperTestingServer.SECRET); + } + + @AfterAll + public static void teardown() throws Exception { + szk.close(); + } + + @BeforeEach + public void before() { + context = EasyMock.createNiceMock(ServerContext.class); + AmpleImpl ample = EasyMock.createStrictMock(AmpleImpl.class); + expect(context.getInstanceID()).andReturn(IID).anyTimes(); + expect(context.getZooReaderWriter()).andReturn(szk.getZooReaderWriter()).anyTimes(); + expect(context.getAmple()).andReturn(ample).anyTimes(); + expect(context.getZooKeepers()).andReturn(szk.getConn()); + expect(context.getZooKeepersSessionTimeOut()).andReturn((30000)); + + TableId tid = TableId.of("2"); + ke1 = new KeyExtent(tid, new Text("b"), null); + ke2 = new KeyExtent(tid, new Text("d"), new Text("b")); + ke3 = new KeyExtent(tid, new Text("g"), new Text("d")); + ke4 = new KeyExtent(tid, new Text("m"), new Text("g")); + + tm1 = TabletMetadata.create(tid.canonical(), null, "b"); + tm2 = TabletMetadata.create(tid.canonical(), "b", "d"); + tm3 = TabletMetadata.create(tid.canonical(), "d", "g"); + tm4 = TabletMetadata.create(tid.canonical(), "g", "m"); + + expect(ample.readTablet(ke1, ColumnType.values())).andReturn(tm1).anyTimes(); + expect(ample.readTablet(ke2, ColumnType.values())).andReturn(tm2).anyTimes(); + expect(ample.readTablet(ke3, ColumnType.values())).andReturn(tm3).anyTimes(); + expect(ample.readTablet(ke4, ColumnType.values())).andReturn(tm4).anyTimes(); + + replay(context, ample); + } + + @Test + public void testPathParsing() { + try (TestTabletMetadataCache cache = new TestTabletMetadataCache(context)) { + String path = cache.getPath(ke1); + KeyExtent result = cache.getExtent(path); + assertEquals(ke1.toMetaRow(), result.toMetaRow()); + } + } + + @Test + public void testCachingLocalTabletChange() { + try (TestTabletMetadataCache cache = new TestTabletMetadataCache(context)) { + cache.tabletMetadataChanged(ke1); // This will perform a create in ZK + assertEquals(0, cache.getTabletMetadataCacheStats().loadCount()); + assertEquals(0, cache.getTabletMetadataCacheSize()); + TabletMetadata result = cache.get(ke1); + assertEquals(1, cache.getTabletMetadataCacheSize()); + assertEquals(1, cache.getTabletMetadataCacheStats().requestCount()); + assertEquals(1, cache.getTabletMetadataCacheStats().loadSuccessCount()); + assertEquals(tm1, result); + cache.tabletMetadataChanged(ke1); // This will perform an update and trigger invalidation + assertEquals(0, cache.getTabletMetadataCacheSize()); + result = cache.get(ke1); + assertEquals(1, cache.getTabletMetadataCacheSize()); + assertEquals(2, cache.getTabletMetadataCacheStats().requestCount()); + assertEquals(2, cache.getTabletMetadataCacheStats().loadSuccessCount()); + assertEquals(tm1, result); + } + } + + public class TabletChangedThread implements Runnable { + + private final Logger LOG = LoggerFactory.getLogger(TabletChangedThread.class); + + private Long deserialize(byte[] value) { + return Long.parseLong(new String(value, UTF_8)); + } + + private byte[] serialize(Long value) { + return value.toString().getBytes(UTF_8); + } + + @Override + public void run() { + + // mutate ZK entry for this tablet + String path = Constants.ZROOT + "/" + IID + Constants.ZTABLET_CACHE + "/" + + TabletMetadataCache.serializeKeyExtent(ke2); + try { + LOG.info("Mutating node at path: {}", path); + szk.getZooReaderWriter().mutateOrCreate(path, serialize(Long.valueOf(0)), (currVal) -> { + return serialize(deserialize(currVal) + 1); + }); + } catch (AcceptableThriftTableOperationException | KeeperException | InterruptedException e) { + LOG.error("***ERROR***", e); + } + } + } + + @Test + public void testCachingThreadTabletChange() throws InterruptedException { + try (TestTabletMetadataCache cache = new TestTabletMetadataCache(context)) { + cache.tabletMetadataChanged(ke2); // This will perform a create in ZK + + assertEquals(0, cache.getTabletMetadataCacheStats().loadCount()); + assertEquals(0, cache.getTabletMetadataCacheSize()); + TabletMetadata result = cache.get(ke2); + assertEquals(1, cache.getTabletMetadataCacheSize()); + assertEquals(1, cache.getTabletMetadataCacheStats().requestCount()); + assertEquals(1, cache.getTabletMetadataCacheStats().loadSuccessCount()); + assertEquals(tm2, result); + + Thread t = new Thread(new TabletChangedThread()); + t.start(); + t.join(); + + // wait for zk watcher to remove cache entry + while (cache.getTabletMetadataCacheSize() != 0) { + Thread.sleep(100); + } + + assertEquals(0, cache.getTabletMetadataCacheSize()); + result = cache.get(ke2); + assertEquals(1, cache.getTabletMetadataCacheSize()); + assertEquals(2, cache.getTabletMetadataCacheStats().requestCount()); + assertEquals(2, cache.getTabletMetadataCacheStats().loadSuccessCount()); + assertEquals(tm2, result); + } + } + +} From 2d36d351092903c39f588ba2820b5e27c0bad16a Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 22 Feb 2023 14:10:02 +0000 Subject: [PATCH 03/12] Removed curator, added javadoc --- pom.xml | 5 - server/base/pom.xml | 4 - .../server/metadata/TabletMetadataCache.java | 183 ++++++++++-------- .../server/util/MetadataTableUtil.java | 3 + .../functional/TabletMetadataCacheIT.java | 7 +- 5 files changed, 111 insertions(+), 91 deletions(-) diff --git a/pom.xml b/pom.xml index 90fd60d0d8e..e4325ea40e5 100644 --- a/pom.xml +++ b/pom.xml @@ -452,11 +452,6 @@ curator-framework ${curator.version} - - org.apache.curator - curator-recipes - ${curator.version} - org.apache.curator curator-test diff --git a/server/base/pom.xml b/server/base/pom.xml index 81047009d00..5888ffa259d 100644 --- a/server/base/pom.xml +++ b/server/base/pom.xml @@ -84,10 +84,6 @@ org.apache.commons commons-lang3 - - org.apache.curator - curator-recipes - org.apache.hadoop hadoop-client-api diff --git a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java index a94e969d1eb..11ddc03167a 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java +++ b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java @@ -21,7 +21,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import java.io.Closeable; -import java.util.concurrent.CountDownLatch; import org.apache.accumulo.core.Constants; import org.apache.accumulo.core.clientImpl.AcceptableThriftTableOperationException; @@ -31,17 +30,12 @@ import org.apache.accumulo.core.metadata.schema.TabletMetadata; import org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType; import org.apache.accumulo.server.ServerContext; -import org.apache.curator.framework.CuratorFramework; -import org.apache.curator.framework.CuratorFrameworkFactory; -import org.apache.curator.framework.WatcherRemoveCuratorFramework; -import org.apache.curator.framework.recipes.cache.ChildData; -import org.apache.curator.framework.recipes.cache.CuratorCache; -import org.apache.curator.framework.recipes.cache.CuratorCacheBuilder; -import org.apache.curator.framework.recipes.cache.CuratorCacheListener; -import org.apache.curator.framework.recipes.cache.CuratorCacheStorage; -import org.apache.curator.retry.RetryNTimes; import org.apache.hadoop.io.Text; +import org.apache.zookeeper.AddWatchMode; import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.WatchedEvent; +import org.apache.zookeeper.Watcher; +import org.apache.zookeeper.Watcher.WatcherType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,108 +45,117 @@ import com.github.benmanes.caffeine.cache.stats.CacheStats; /** - * Object that uses ZooKeeper for signaling Tablet metadata changes to keep a cache of Tablet + * Object that uses ZooKeeper for signaling Tablet metadata changes to keep a local cache of Tablet * metadata up to date. - * */ public class TabletMetadataCache implements Closeable { + private Watcher tabletCacheZNodeWatcher = new Watcher() { + @Override + public void process(WatchedEvent event) { + switch (event.getType()) { + case NodeCreated: + // Do nothing, cache will be populated on clients first call to get() + break; + case NodeDataChanged: + case NodeDeleted: + // Remove the tablet metadata cache entry on update or delete. + KeyExtent extent = getExtent(event.getPath()); + LOG.info("Invalidating extent: {}", extent); + tabletMetadataCache.invalidate(extent); + break; + case None: + case ChildWatchRemoved: + case DataWatchRemoved: + case NodeChildrenChanged: + case PersistentWatchRemoved: + default: + break; + + } + } + }; + private static final Logger LOG = LoggerFactory.getLogger(TabletMetadataCache.class); private static final Long INITIAL_VALUE = Long.valueOf(0); - private final String znodeBasePath; + private final String watcherPath; + private final String znodePath; private final ZooReaderWriter zrw; - private final CuratorFramework cf; - private final WatcherRemoveCuratorFramework watcherWrapper; - private final CuratorCache tabletStateCache; private final LoadingCache tabletMetadataCache; public TabletMetadataCache(final ServerContext ctx) { this.zrw = ctx.getZooReaderWriter(); - final String rootPath = Constants.ZROOT + "/" + ctx.getInstanceID() + Constants.ZTABLET_CACHE; - znodeBasePath = rootPath + "/"; + watcherPath = Constants.ZROOT + "/" + ctx.getInstanceID() + Constants.ZTABLET_CACHE; + znodePath = watcherPath + "/"; // TODO: Likely want to set a max size and eviction parameters tabletMetadataCache = Caffeine.newBuilder().recordStats().scheduler(Scheduler.systemScheduler()).build((k) -> { - try { - TabletMetadata tm = ctx.getAmple().readTablet(k, ColumnType.values()); - LOG.info("Loading tablet metadata for extent: {}. Returned: {}", k, tm); - return tm; - } catch (Exception e) { - LOG.error("Error loading tablet metadata for extent: {}", k, e); - throw e; - } + TabletMetadata tm = ctx.getAmple().readTablet(k, ColumnType.values()); + LOG.debug("Loading tablet metadata for extent: {}. Returned: {}", k, tm); + return tm; }); - CuratorFrameworkFactory.Builder curatorBuilder = CuratorFrameworkFactory.builder(); - curatorBuilder.connectString(ctx.getZooKeepers()); - curatorBuilder.sessionTimeoutMs(ctx.getZooKeepersSessionTimeOut()); - curatorBuilder.retryPolicy(new RetryNTimes(5, 2000)); - cf = curatorBuilder.build(); - cf.start(); - watcherWrapper = cf.newWatcherRemoveCuratorFramework(); - - CuratorCacheBuilder cacheBuilder = CuratorCache.builder(watcherWrapper, rootPath); - cacheBuilder.withStorage(CuratorCacheStorage.standard()); - this.tabletStateCache = cacheBuilder.build(); - CountDownLatch initializedLatch = new CountDownLatch(1); - CuratorCacheListener listener = new CuratorCacheListener() { - @Override - public void event(Type type, ChildData oldData, ChildData newData) { - LOG.info("Received event type: {}, old: {}, new: {}", type, oldData, newData); - String path = newData.getPath(); - switch (type) { - case NODE_CREATED: - // Do nothing, cache will be populated on clients first call to get() - break; - case NODE_CHANGED: - case NODE_DELETED: - default: - // Remove the tablet metadata cache entry on update or delete. - KeyExtent extent = getExtent(path); - LOG.info("Invalidating extent: {}", extent); - tabletMetadataCache.invalidate(extent); - break; - - } - } - - @Override - public void initialized() { - initializedLatch.countDown(); - } - }; - tabletStateCache.listenable() - .addListener(CuratorCacheListener.builder().forAll(listener).afterInitialized().build()); - tabletStateCache.start(); try { - initializedLatch.await(); + this.zrw.getZooKeeper().addWatch(watcherPath, tabletCacheZNodeWatcher, + AddWatchMode.PERSISTENT_RECURSIVE); + } catch (KeeperException e) { + throw new RuntimeException("Error setting watch", e); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - throw new RuntimeException("Thread interrupted waiting for tabletStateCache to initialize", - e); + throw new RuntimeException("Thread interrupted setting watch", e); } + } + /** + * Return a KeyExtent object from the tablet_cache znode entry + * + * @param path tablet_cache znode path + * @return KeyExtent object + */ protected KeyExtent getExtent(String path) { - return deserializeKeyExtent(path.substring(znodeBasePath.length())); + return deserializeKeyExtent(path.substring(znodePath.length())); } + /** + * Return the tablet_cache znode path for a KeyExtent + * + * @param extent KeyExtent + * @return tablet_cache znode path + */ protected String getPath(KeyExtent extent) { - return znodeBasePath + serializeKeyExtent(extent); + return znodePath + serializeKeyExtent(extent); } + /** + * Cached TabletMetadata for the KeyExtent. If it does not exist, then it will be loaded into the + * cache via the CacheLoader which uses Ample to read the TabletMetadata and all of its columns. + * + * @param extent KeyExtent + * @return TabletMetadata for the KeyExtent + */ public TabletMetadata get(KeyExtent extent) { return tabletMetadataCache.get(extent); } + /** + * Return size of the cache + * + * @return size + */ protected int getTabletMetadataCacheSize() { return tabletMetadataCache.asMap().size(); } + /** + * Return Cache stats object + * + * @return cache stats + */ protected CacheStats getTabletMetadataCacheStats() { return tabletMetadataCache.stats(); } @@ -161,7 +164,7 @@ protected CacheStats getTabletMetadataCacheStats() { * Updates the data of the znode for this extent which will trigger TabletMetadataCache watchers * to reload the TabletMetadata for this extent. * - * @param extent + * @param extent KeyExtent */ public void tabletMetadataChanged(KeyExtent extent) { final String path = getPath(extent); @@ -176,15 +179,24 @@ public void tabletMetadataChanged(KeyExtent extent) { } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new RuntimeException("Interrupted updating ZooKeeper at: " + path, e); - } catch (AcceptableThriftTableOperationException | KeeperException e) { throw new RuntimeException("Error updating ZooKeeper at: " + path, e); } } + /** + * Returns a String that represents this KeyExtent's TableId, endRow, and prevEndRow. + * TabletsSection#encodeRow(TableId, Text) does not preserve prevEndRow when serializing the + * KeyExtent. + * + * @param ke KeyExtent + * @return string representation of KeyExtent + */ public static String serializeKeyExtent(KeyExtent ke) { Text entry = new Text(ke.tableId().canonical()); entry.append(new byte[] {';'}, 0, 1); + // TODO: May need to do semi-colon escaping in endRow and prevEndRow + // like KeyExtent.toString() if (ke.endRow() != null) { entry.append(ke.endRow().getBytes(), 0, ke.endRow().getLength()); } @@ -195,12 +207,18 @@ public static String serializeKeyExtent(KeyExtent ke) { return entry.toString(); } - private static KeyExtent deserializeKeyExtent(String zkNode) { - String[] parts = zkNode.split(";"); - assert (parts.length == 3); + /** + * Parses a String created by {@link #serializeKeyExtent(KeyExtent)} and returns a KeyExtent. + * + * @param serializedKeyExtent serialized KeyExtent + * @return KeyExtent + */ + private static KeyExtent deserializeKeyExtent(String serializedKeyExtent) { + String[] parts = serializedKeyExtent.split(";"); + assert (parts.length == 2 || parts.length == 3); String tid = parts[0]; String end = parts[1]; - String prev = parts[2]; + String prev = parts.length == 3 ? parts[2] : null; return new KeyExtent(TableId.of(tid), end == null ? null : new Text(end), prev == null ? null : new Text(prev)); } @@ -217,9 +235,12 @@ private static byte[] serializeData(Long value) { public void close() { tabletMetadataCache.invalidateAll(); tabletMetadataCache.cleanUp(); - tabletStateCache.close(); - watcherWrapper.removeWatchers(); - cf.close(); + try { + this.zrw.getZooKeeper().removeWatches(watcherPath, tabletCacheZNodeWatcher, WatcherType.Any, + false); + } catch (InterruptedException | KeeperException e) { + LOG.error("Error removing persistent watcher", e); + } } } diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java b/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java index 33c0a415a5a..c4d914f45cb 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java +++ b/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java @@ -157,6 +157,9 @@ public static void update(ServerContext context, Writer t, ServiceLock zooLock, while (true) { try { t.update(m); + if (!extent.isMeta()) { + context.getTabletMetadataCache().tabletMetadataChanged(extent); + } return; } catch (AccumuloException | TableNotFoundException | AccumuloSecurityException e) { logUpdateFailure(m, extent, e); diff --git a/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java b/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java index 3a077f10c26..06bd197bbf2 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java @@ -146,8 +146,13 @@ public void before() { public void testPathParsing() { try (TestTabletMetadataCache cache = new TestTabletMetadataCache(context)) { String path = cache.getPath(ke1); + assertEquals("/accumulo/" + IID + Constants.ZTABLET_CACHE + "/2;b;", path); KeyExtent result = cache.getExtent(path); - assertEquals(ke1.toMetaRow(), result.toMetaRow()); + assertEquals(ke1, result); + String path2 = cache.getPath(ke2); + assertEquals("/accumulo/" + IID + Constants.ZTABLET_CACHE + "/2;d;b", path2); + KeyExtent result2 = cache.getExtent(path2); + assertEquals(ke2.toMetaRow(), result2.toMetaRow()); } } From d400d36ec41426fc3f03cc578b2263df449c378c Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 22 Feb 2023 16:22:22 +0000 Subject: [PATCH 04/12] Refactor code so that marking a tablet as modified doesnt initialize the cache --- .../apache/accumulo/server/ServerContext.java | 4 + .../server/metadata/TabletMetadataCache.java | 204 +++++++++--------- .../server/metadata/TabletMutatorImpl.java | 2 +- .../server/util/MetadataTableUtil.java | 3 +- .../functional/TabletMetadataCacheIT.java | 34 +-- 5 files changed, 126 insertions(+), 121 deletions(-) diff --git a/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java b/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java index 82ae5bdbace..48fe18f4118 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java +++ b/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java @@ -474,6 +474,10 @@ public TabletMetadataCache getTabletMetadataCache() { } } + public boolean isTabletMetadataCacheInitialized() { + return tabletMetadataCacheInitialized; + } + @Override public synchronized void close() { super.close(); diff --git a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java index 11ddc03167a..8e94e386f28 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java +++ b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java @@ -42,7 +42,6 @@ import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.cache.LoadingCache; import com.github.benmanes.caffeine.cache.Scheduler; -import com.github.benmanes.caffeine.cache.stats.CacheStats; /** * Object that uses ZooKeeper for signaling Tablet metadata changes to keep a local cache of Tablet @@ -50,6 +49,101 @@ */ public class TabletMetadataCache implements Closeable { + private static final Logger LOG = LoggerFactory.getLogger(TabletMetadataCache.class); + private static final Long INITIAL_VALUE = Long.valueOf(0); + + private static String getWatcherPath(ServerContext ctx) { + return Constants.ZROOT + "/" + ctx.getInstanceID() + Constants.ZTABLET_CACHE; + } + + private static String getZNodePath(ServerContext ctx) { + return getWatcherPath(ctx) + "/"; + } + + /** + * Returns a String that represents this KeyExtent's TableId, endRow, and prevEndRow. + * TabletsSection#encodeRow(TableId, Text) does not preserve prevEndRow when serializing the + * KeyExtent. + * + * @param ke KeyExtent + * @return string representation of KeyExtent + */ + public static String serializeKeyExtent(KeyExtent ke) { + Text entry = new Text(ke.tableId().canonical()); + entry.append(new byte[] {';'}, 0, 1); + // TODO: May need to do semi-colon escaping in endRow and prevEndRow + // like KeyExtent.toString() + if (ke.endRow() != null) { + entry.append(ke.endRow().getBytes(), 0, ke.endRow().getLength()); + } + entry.append(new byte[] {';'}, 0, 1); + if (ke.prevEndRow() != null) { + entry.append(ke.prevEndRow().getBytes(), 0, ke.prevEndRow().getLength()); + } + return entry.toString(); + } + + /** + * Parses a String created by {@link #serializeKeyExtent(KeyExtent)} and returns a KeyExtent. + * + * @param serializedKeyExtent serialized KeyExtent + * @return KeyExtent + */ + private static KeyExtent deserializeKeyExtent(String serializedKeyExtent) { + String[] parts = serializedKeyExtent.split(";"); + assert (parts.length == 2 || parts.length == 3); + String tid = parts[0]; + String end = parts[1]; + String prev = parts.length == 3 ? parts[2] : null; + return new KeyExtent(TableId.of(tid), end == null ? null : new Text(end), + prev == null ? null : new Text(prev)); + } + + private static Long deserializeData(byte[] value) { + return Long.parseLong(new String(value, UTF_8)); + } + + private static byte[] serializeData(Long value) { + return value.toString().getBytes(UTF_8); + } + + /** + * Return the tablet_cache znode path for a KeyExtent + * + * @param extent KeyExtent + * @return tablet_cache znode path + */ + public static String getPath(ServerContext ctx, KeyExtent extent) { + return getZNodePath(ctx) + serializeKeyExtent(extent); + } + + /** + * Updates the data of the znode for this extent which will trigger TabletMetadataCache watchers + * to reload the TabletMetadata for this extent. + * + * @param extent KeyExtent + */ + public static void tabletMetadataChanged(ServerContext ctx, KeyExtent extent) { + final String path = getPath(ctx, extent); + try { + // remove entry from local cache only if the local + // TabletMetadataCache has been initialized + if (ctx.isTabletMetadataCacheInitialized()) { + LOG.info("Invalidating extent due to local tablet change: {}", extent); + ctx.getTabletMetadataCache().invalidate(extent); + } + // mutate ZK entry for this tablet + ctx.getZooReaderWriter().mutateOrCreate(path, serializeData(INITIAL_VALUE), (currVal) -> { + return serializeData(deserializeData(currVal) + 1); + }); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException("Interrupted updating ZooKeeper at: " + path, e); + } catch (AcceptableThriftTableOperationException | KeeperException e) { + throw new RuntimeException("Error updating ZooKeeper at: " + path, e); + } + } + private Watcher tabletCacheZNodeWatcher = new Watcher() { @Override public void process(WatchedEvent event) { @@ -76,20 +170,17 @@ public void process(WatchedEvent event) { } }; - private static final Logger LOG = LoggerFactory.getLogger(TabletMetadataCache.class); - private static final Long INITIAL_VALUE = Long.valueOf(0); - private final String watcherPath; private final String znodePath; private final ZooReaderWriter zrw; - private final LoadingCache tabletMetadataCache; + protected final LoadingCache tabletMetadataCache; public TabletMetadataCache(final ServerContext ctx) { this.zrw = ctx.getZooReaderWriter(); - watcherPath = Constants.ZROOT + "/" + ctx.getInstanceID() + Constants.ZTABLET_CACHE; - znodePath = watcherPath + "/"; + watcherPath = getWatcherPath(ctx); + znodePath = getZNodePath(ctx); // TODO: Likely want to set a max size and eviction parameters tabletMetadataCache = @@ -121,16 +212,6 @@ protected KeyExtent getExtent(String path) { return deserializeKeyExtent(path.substring(znodePath.length())); } - /** - * Return the tablet_cache znode path for a KeyExtent - * - * @param extent KeyExtent - * @return tablet_cache znode path - */ - protected String getPath(KeyExtent extent) { - return znodePath + serializeKeyExtent(extent); - } - /** * Cached TabletMetadata for the KeyExtent. If it does not exist, then it will be loaded into the * cache via the CacheLoader which uses Ample to read the TabletMetadata and all of its columns. @@ -142,93 +223,8 @@ public TabletMetadata get(KeyExtent extent) { return tabletMetadataCache.get(extent); } - /** - * Return size of the cache - * - * @return size - */ - protected int getTabletMetadataCacheSize() { - return tabletMetadataCache.asMap().size(); - } - - /** - * Return Cache stats object - * - * @return cache stats - */ - protected CacheStats getTabletMetadataCacheStats() { - return tabletMetadataCache.stats(); - } - - /** - * Updates the data of the znode for this extent which will trigger TabletMetadataCache watchers - * to reload the TabletMetadata for this extent. - * - * @param extent KeyExtent - */ - public void tabletMetadataChanged(KeyExtent extent) { - final String path = getPath(extent); - try { - // remove entry from local cache - LOG.info("Invalidating extent due to local tablet change: {}", extent); - tabletMetadataCache.invalidate(extent); - // mutate ZK entry for this tablet - this.zrw.mutateOrCreate(path, serializeData(INITIAL_VALUE), (currVal) -> { - return serializeData(deserializeData(currVal) + 1); - }); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new RuntimeException("Interrupted updating ZooKeeper at: " + path, e); - } catch (AcceptableThriftTableOperationException | KeeperException e) { - throw new RuntimeException("Error updating ZooKeeper at: " + path, e); - } - } - - /** - * Returns a String that represents this KeyExtent's TableId, endRow, and prevEndRow. - * TabletsSection#encodeRow(TableId, Text) does not preserve prevEndRow when serializing the - * KeyExtent. - * - * @param ke KeyExtent - * @return string representation of KeyExtent - */ - public static String serializeKeyExtent(KeyExtent ke) { - Text entry = new Text(ke.tableId().canonical()); - entry.append(new byte[] {';'}, 0, 1); - // TODO: May need to do semi-colon escaping in endRow and prevEndRow - // like KeyExtent.toString() - if (ke.endRow() != null) { - entry.append(ke.endRow().getBytes(), 0, ke.endRow().getLength()); - } - entry.append(new byte[] {';'}, 0, 1); - if (ke.prevEndRow() != null) { - entry.append(ke.prevEndRow().getBytes(), 0, ke.prevEndRow().getLength()); - } - return entry.toString(); - } - - /** - * Parses a String created by {@link #serializeKeyExtent(KeyExtent)} and returns a KeyExtent. - * - * @param serializedKeyExtent serialized KeyExtent - * @return KeyExtent - */ - private static KeyExtent deserializeKeyExtent(String serializedKeyExtent) { - String[] parts = serializedKeyExtent.split(";"); - assert (parts.length == 2 || parts.length == 3); - String tid = parts[0]; - String end = parts[1]; - String prev = parts.length == 3 ? parts[2] : null; - return new KeyExtent(TableId.of(tid), end == null ? null : new Text(end), - prev == null ? null : new Text(prev)); - } - - private static Long deserializeData(byte[] value) { - return Long.parseLong(new String(value, UTF_8)); - } - - private static byte[] serializeData(Long value) { - return value.toString().getBytes(UTF_8); + public void invalidate(KeyExtent extent) { + tabletMetadataCache.invalidate(extent); } @Override diff --git a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorImpl.java b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorImpl.java index c68e09ce771..896c474d82a 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorImpl.java +++ b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorImpl.java @@ -43,7 +43,7 @@ public void mutate() { } catch (Exception e) { throw new RuntimeException(e); } - this.context.getTabletMetadataCache().tabletMetadataChanged(extent); + TabletMetadataCache.tabletMetadataChanged(context, extent); } } diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java b/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java index c4d914f45cb..d93d06894d7 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java +++ b/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java @@ -96,6 +96,7 @@ import org.apache.accumulo.core.util.Pair; import org.apache.accumulo.server.ServerContext; import org.apache.accumulo.server.gc.AllVolumesDirectory; +import org.apache.accumulo.server.metadata.TabletMetadataCache; import org.apache.hadoop.io.Text; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -158,7 +159,7 @@ public static void update(ServerContext context, Writer t, ServiceLock zooLock, try { t.update(m); if (!extent.isMeta()) { - context.getTabletMetadataCache().tabletMetadataChanged(extent); + TabletMetadataCache.tabletMetadataChanged(context, extent); } return; } catch (AccumuloException | TableNotFoundException | AccumuloSecurityException e) { diff --git a/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java b/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java index 06bd197bbf2..3d7ba2cb8a3 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java @@ -71,19 +71,22 @@ public KeyExtent getExtent(String path) { return super.getExtent(path); } - @Override - public String getPath(KeyExtent extent) { - return super.getPath(extent); + /** + * Return size of the cache + * + * @return size + */ + protected int getTabletMetadataCacheSize() { + return tabletMetadataCache.asMap().size(); } - @Override - public int getTabletMetadataCacheSize() { - return super.getTabletMetadataCacheSize(); - } - - @Override + /** + * Return Cache stats object + * + * @return cache stats + */ protected CacheStats getTabletMetadataCacheStats() { - return super.getTabletMetadataCacheStats(); + return tabletMetadataCache.stats(); } } @@ -145,11 +148,11 @@ public void before() { @Test public void testPathParsing() { try (TestTabletMetadataCache cache = new TestTabletMetadataCache(context)) { - String path = cache.getPath(ke1); + String path = TabletMetadataCache.getPath(context, ke1); assertEquals("/accumulo/" + IID + Constants.ZTABLET_CACHE + "/2;b;", path); KeyExtent result = cache.getExtent(path); assertEquals(ke1, result); - String path2 = cache.getPath(ke2); + String path2 = TabletMetadataCache.getPath(context, ke2); assertEquals("/accumulo/" + IID + Constants.ZTABLET_CACHE + "/2;d;b", path2); KeyExtent result2 = cache.getExtent(path2); assertEquals(ke2.toMetaRow(), result2.toMetaRow()); @@ -159,7 +162,7 @@ public void testPathParsing() { @Test public void testCachingLocalTabletChange() { try (TestTabletMetadataCache cache = new TestTabletMetadataCache(context)) { - cache.tabletMetadataChanged(ke1); // This will perform a create in ZK + TabletMetadataCache.tabletMetadataChanged(context, ke1); // This will perform a create in ZK assertEquals(0, cache.getTabletMetadataCacheStats().loadCount()); assertEquals(0, cache.getTabletMetadataCacheSize()); TabletMetadata result = cache.get(ke1); @@ -167,7 +170,8 @@ public void testCachingLocalTabletChange() { assertEquals(1, cache.getTabletMetadataCacheStats().requestCount()); assertEquals(1, cache.getTabletMetadataCacheStats().loadSuccessCount()); assertEquals(tm1, result); - cache.tabletMetadataChanged(ke1); // This will perform an update and trigger invalidation + TabletMetadataCache.tabletMetadataChanged(context, ke1); // This will perform an update and + // trigger invalidation assertEquals(0, cache.getTabletMetadataCacheSize()); result = cache.get(ke1); assertEquals(1, cache.getTabletMetadataCacheSize()); @@ -209,7 +213,7 @@ public void run() { @Test public void testCachingThreadTabletChange() throws InterruptedException { try (TestTabletMetadataCache cache = new TestTabletMetadataCache(context)) { - cache.tabletMetadataChanged(ke2); // This will perform a create in ZK + TabletMetadataCache.tabletMetadataChanged(context, ke2); // This will perform a create in ZK assertEquals(0, cache.getTabletMetadataCacheStats().loadCount()); assertEquals(0, cache.getTabletMetadataCacheSize()); From 13b4108a7673d87fab4c750f4a03488c432c5ce9 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 22 Feb 2023 17:59:52 +0000 Subject: [PATCH 05/12] Fix TabletMetadataCacheIT --- .../apache/accumulo/server/ServerContext.java | 4 +- .../server/metadata/TabletMetadataCache.java | 28 ++-- .../functional/TabletMetadataCacheIT.java | 152 +++++++++--------- 3 files changed, 94 insertions(+), 90 deletions(-) diff --git a/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java b/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java index 48fe18f4118..5f12faf5bce 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java +++ b/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java @@ -53,6 +53,7 @@ import org.apache.accumulo.core.fate.zookeeper.ZooReader; import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter; import org.apache.accumulo.core.metadata.schema.Ample; +import org.apache.accumulo.core.metadata.schema.AmpleImpl; import org.apache.accumulo.core.rpc.SslConnectionParams; import org.apache.accumulo.core.singletons.SingletonReservation; import org.apache.accumulo.core.spi.crypto.CryptoServiceFactory; @@ -135,7 +136,8 @@ private ServerContext(ServerInfo info) { memoize(() -> new AuditedSecurityOperation(this, SecurityOperation.getAuthorizor(this), SecurityOperation.getAuthenticator(this), SecurityOperation.getPermHandler(this))); lowMemoryDetector = memoize(() -> new LowMemoryDetector()); - tabletMetadataCache = memoize(() -> new TabletMetadataCache(this)); + tabletMetadataCache = memoize( + () -> new TabletMetadataCache(info.getInstanceID(), zooReaderWriter, new AmpleImpl(this))); } /** diff --git a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java index 8e94e386f28..b7e7faf57be 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java +++ b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java @@ -24,9 +24,11 @@ import org.apache.accumulo.core.Constants; import org.apache.accumulo.core.clientImpl.AcceptableThriftTableOperationException; +import org.apache.accumulo.core.data.InstanceId; import org.apache.accumulo.core.data.TableId; import org.apache.accumulo.core.dataImpl.KeyExtent; import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.core.metadata.schema.Ample; import org.apache.accumulo.core.metadata.schema.TabletMetadata; import org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType; import org.apache.accumulo.server.ServerContext; @@ -52,12 +54,12 @@ public class TabletMetadataCache implements Closeable { private static final Logger LOG = LoggerFactory.getLogger(TabletMetadataCache.class); private static final Long INITIAL_VALUE = Long.valueOf(0); - private static String getWatcherPath(ServerContext ctx) { - return Constants.ZROOT + "/" + ctx.getInstanceID() + Constants.ZTABLET_CACHE; + private static String getWatcherPath(InstanceId iid) { + return Constants.ZROOT + "/" + iid + Constants.ZTABLET_CACHE; } - private static String getZNodePath(ServerContext ctx) { - return getWatcherPath(ctx) + "/"; + private static String getZNodePath(InstanceId iid) { + return getWatcherPath(iid) + "/"; } /** @@ -113,8 +115,8 @@ private static byte[] serializeData(Long value) { * @param extent KeyExtent * @return tablet_cache znode path */ - public static String getPath(ServerContext ctx, KeyExtent extent) { - return getZNodePath(ctx) + serializeKeyExtent(extent); + public static String getPath(InstanceId iid, KeyExtent extent) { + return getZNodePath(iid) + serializeKeyExtent(extent); } /** @@ -124,7 +126,7 @@ public static String getPath(ServerContext ctx, KeyExtent extent) { * @param extent KeyExtent */ public static void tabletMetadataChanged(ServerContext ctx, KeyExtent extent) { - final String path = getPath(ctx, extent); + final String path = getPath(ctx.getInstanceID(), extent); try { // remove entry from local cache only if the local // TabletMetadataCache has been initialized @@ -147,6 +149,7 @@ public static void tabletMetadataChanged(ServerContext ctx, KeyExtent extent) { private Watcher tabletCacheZNodeWatcher = new Watcher() { @Override public void process(WatchedEvent event) { + LOG.info("Event received: {}", event); switch (event.getType()) { case NodeCreated: // Do nothing, cache will be populated on clients first call to get() @@ -175,17 +178,16 @@ public void process(WatchedEvent event) { private final ZooReaderWriter zrw; protected final LoadingCache tabletMetadataCache; - public TabletMetadataCache(final ServerContext ctx) { + public TabletMetadataCache(final InstanceId iid, final ZooReaderWriter zrw, final Ample ample) { - this.zrw = ctx.getZooReaderWriter(); - - watcherPath = getWatcherPath(ctx); - znodePath = getZNodePath(ctx); + this.zrw = zrw; + watcherPath = getWatcherPath(iid); + znodePath = getZNodePath(iid); // TODO: Likely want to set a max size and eviction parameters tabletMetadataCache = Caffeine.newBuilder().recordStats().scheduler(Scheduler.systemScheduler()).build((k) -> { - TabletMetadata tm = ctx.getAmple().readTablet(k, ColumnType.values()); + TabletMetadata tm = ample.readTablet(k, ColumnType.values()); LOG.debug("Loading tablet metadata for extent: {}. Returned: {}", k, tm); return tm; }); diff --git a/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java b/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java index 3d7ba2cb8a3..d697b9a4db6 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java @@ -32,7 +32,9 @@ import org.apache.accumulo.core.data.InstanceId; import org.apache.accumulo.core.data.TableId; import org.apache.accumulo.core.dataImpl.KeyExtent; +import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter; import org.apache.accumulo.core.fate.zookeeper.ZooUtil; +import org.apache.accumulo.core.metadata.schema.Ample; import org.apache.accumulo.core.metadata.schema.AmpleImpl; import org.apache.accumulo.core.metadata.schema.TabletMetadata; import org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType; @@ -44,6 +46,7 @@ import org.apache.zookeeper.ZooKeeper; import org.easymock.EasyMock; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; @@ -62,8 +65,8 @@ public class TabletMetadataCacheIT { */ public static class TestTabletMetadataCache extends TabletMetadataCache { - public TestTabletMetadataCache(ServerContext ctx) { - super(ctx); + public TestTabletMetadataCache(InstanceId iid, ZooReaderWriter zrw, Ample ample) { + super(iid, zrw, ample); } @Override @@ -99,9 +102,17 @@ protected CacheStats getTabletMetadataCacheStats() { private static ZooKeeperTestingServer szk = null; private static ZooKeeper zooKeeper; - private ServerContext context; - private KeyExtent ke1, ke2, ke3, ke4; - private TabletMetadata tm1, tm2, tm3, tm4; + private ServerContext context = null; + private TestTabletMetadataCache cache = null; + private TableId tid = TableId.of("2"); + private KeyExtent ke1 = new KeyExtent(tid, new Text("b"), null); + private KeyExtent ke2 = new KeyExtent(tid, new Text("d"), new Text("b")); + private KeyExtent ke3 = new KeyExtent(tid, new Text("g"), new Text("d")); + private KeyExtent ke4 = new KeyExtent(tid, new Text("m"), new Text("g")); + private TabletMetadata tm1 = TabletMetadata.create(tid.canonical(), null, "b"); + private TabletMetadata tm2 = TabletMetadata.create(tid.canonical(), "b", "d"); + private TabletMetadata tm3 = TabletMetadata.create(tid.canonical(), "d", "g"); + private TabletMetadata tm4 = TabletMetadata.create(tid.canonical(), "g", "m"); @BeforeAll public static void setup() throws Exception { @@ -120,65 +131,56 @@ public static void teardown() throws Exception { public void before() { context = EasyMock.createNiceMock(ServerContext.class); AmpleImpl ample = EasyMock.createStrictMock(AmpleImpl.class); + expect(ample.readTablet(ke1, ColumnType.values())).andReturn(tm1).anyTimes(); + expect(ample.readTablet(ke2, ColumnType.values())).andReturn(tm2).anyTimes(); + expect(ample.readTablet(ke3, ColumnType.values())).andReturn(tm3).anyTimes(); + expect(ample.readTablet(ke4, ColumnType.values())).andReturn(tm4).anyTimes(); + cache = new TestTabletMetadataCache(IID, szk.getZooReaderWriter(), ample); expect(context.getInstanceID()).andReturn(IID).anyTimes(); expect(context.getZooReaderWriter()).andReturn(szk.getZooReaderWriter()).anyTimes(); expect(context.getAmple()).andReturn(ample).anyTimes(); expect(context.getZooKeepers()).andReturn(szk.getConn()); expect(context.getZooKeepersSessionTimeOut()).andReturn((30000)); - - TableId tid = TableId.of("2"); - ke1 = new KeyExtent(tid, new Text("b"), null); - ke2 = new KeyExtent(tid, new Text("d"), new Text("b")); - ke3 = new KeyExtent(tid, new Text("g"), new Text("d")); - ke4 = new KeyExtent(tid, new Text("m"), new Text("g")); - - tm1 = TabletMetadata.create(tid.canonical(), null, "b"); - tm2 = TabletMetadata.create(tid.canonical(), "b", "d"); - tm3 = TabletMetadata.create(tid.canonical(), "d", "g"); - tm4 = TabletMetadata.create(tid.canonical(), "g", "m"); - - expect(ample.readTablet(ke1, ColumnType.values())).andReturn(tm1).anyTimes(); - expect(ample.readTablet(ke2, ColumnType.values())).andReturn(tm2).anyTimes(); - expect(ample.readTablet(ke3, ColumnType.values())).andReturn(tm3).anyTimes(); - expect(ample.readTablet(ke4, ColumnType.values())).andReturn(tm4).anyTimes(); - + expect(context.isTabletMetadataCacheInitialized()).andReturn(Boolean.TRUE).anyTimes(); + expect(context.getTabletMetadataCache()).andReturn(cache).anyTimes(); replay(context, ample); } + @AfterEach + public void after() { + cache.close(); + } + @Test public void testPathParsing() { - try (TestTabletMetadataCache cache = new TestTabletMetadataCache(context)) { - String path = TabletMetadataCache.getPath(context, ke1); - assertEquals("/accumulo/" + IID + Constants.ZTABLET_CACHE + "/2;b;", path); - KeyExtent result = cache.getExtent(path); - assertEquals(ke1, result); - String path2 = TabletMetadataCache.getPath(context, ke2); - assertEquals("/accumulo/" + IID + Constants.ZTABLET_CACHE + "/2;d;b", path2); - KeyExtent result2 = cache.getExtent(path2); - assertEquals(ke2.toMetaRow(), result2.toMetaRow()); - } + String path = TabletMetadataCache.getPath(IID, ke1); + assertEquals("/accumulo/" + IID + Constants.ZTABLET_CACHE + "/2;b;", path); + KeyExtent result = cache.getExtent(path); + assertEquals(ke1, result); + String path2 = TabletMetadataCache.getPath(IID, ke2); + assertEquals("/accumulo/" + IID + Constants.ZTABLET_CACHE + "/2;d;b", path2); + KeyExtent result2 = cache.getExtent(path2); + assertEquals(ke2.toMetaRow(), result2.toMetaRow()); } @Test public void testCachingLocalTabletChange() { - try (TestTabletMetadataCache cache = new TestTabletMetadataCache(context)) { - TabletMetadataCache.tabletMetadataChanged(context, ke1); // This will perform a create in ZK - assertEquals(0, cache.getTabletMetadataCacheStats().loadCount()); - assertEquals(0, cache.getTabletMetadataCacheSize()); - TabletMetadata result = cache.get(ke1); - assertEquals(1, cache.getTabletMetadataCacheSize()); - assertEquals(1, cache.getTabletMetadataCacheStats().requestCount()); - assertEquals(1, cache.getTabletMetadataCacheStats().loadSuccessCount()); - assertEquals(tm1, result); - TabletMetadataCache.tabletMetadataChanged(context, ke1); // This will perform an update and - // trigger invalidation - assertEquals(0, cache.getTabletMetadataCacheSize()); - result = cache.get(ke1); - assertEquals(1, cache.getTabletMetadataCacheSize()); - assertEquals(2, cache.getTabletMetadataCacheStats().requestCount()); - assertEquals(2, cache.getTabletMetadataCacheStats().loadSuccessCount()); - assertEquals(tm1, result); - } + TabletMetadataCache.tabletMetadataChanged(context, ke1); // This will perform a create in ZK + assertEquals(0, cache.getTabletMetadataCacheStats().loadCount()); + assertEquals(0, cache.getTabletMetadataCacheSize()); + TabletMetadata result = cache.get(ke1); + assertEquals(1, cache.getTabletMetadataCacheSize()); + assertEquals(1, cache.getTabletMetadataCacheStats().requestCount()); + assertEquals(1, cache.getTabletMetadataCacheStats().loadSuccessCount()); + assertEquals(tm1, result); + TabletMetadataCache.tabletMetadataChanged(context, ke1); // This will perform an update and + // trigger invalidation + assertEquals(0, cache.getTabletMetadataCacheSize()); + result = cache.get(ke1); + assertEquals(1, cache.getTabletMetadataCacheSize()); + assertEquals(2, cache.getTabletMetadataCacheStats().requestCount()); + assertEquals(2, cache.getTabletMetadataCacheStats().loadSuccessCount()); + assertEquals(tm1, result); } public class TabletChangedThread implements Runnable { @@ -212,33 +214,31 @@ public void run() { @Test public void testCachingThreadTabletChange() throws InterruptedException { - try (TestTabletMetadataCache cache = new TestTabletMetadataCache(context)) { - TabletMetadataCache.tabletMetadataChanged(context, ke2); // This will perform a create in ZK - - assertEquals(0, cache.getTabletMetadataCacheStats().loadCount()); - assertEquals(0, cache.getTabletMetadataCacheSize()); - TabletMetadata result = cache.get(ke2); - assertEquals(1, cache.getTabletMetadataCacheSize()); - assertEquals(1, cache.getTabletMetadataCacheStats().requestCount()); - assertEquals(1, cache.getTabletMetadataCacheStats().loadSuccessCount()); - assertEquals(tm2, result); - - Thread t = new Thread(new TabletChangedThread()); - t.start(); - t.join(); - - // wait for zk watcher to remove cache entry - while (cache.getTabletMetadataCacheSize() != 0) { - Thread.sleep(100); - } - - assertEquals(0, cache.getTabletMetadataCacheSize()); - result = cache.get(ke2); - assertEquals(1, cache.getTabletMetadataCacheSize()); - assertEquals(2, cache.getTabletMetadataCacheStats().requestCount()); - assertEquals(2, cache.getTabletMetadataCacheStats().loadSuccessCount()); - assertEquals(tm2, result); + TabletMetadataCache.tabletMetadataChanged(context, ke2); // This will perform a create in ZK + + assertEquals(0, cache.getTabletMetadataCacheStats().loadCount()); + assertEquals(0, cache.getTabletMetadataCacheSize()); + TabletMetadata result = cache.get(ke2); + assertEquals(1, cache.getTabletMetadataCacheSize()); + assertEquals(1, cache.getTabletMetadataCacheStats().requestCount()); + assertEquals(1, cache.getTabletMetadataCacheStats().loadSuccessCount()); + assertEquals(tm2, result); + + Thread t = new Thread(new TabletChangedThread()); + t.start(); + t.join(); + + // wait for zk watcher to remove cache entry + while (cache.getTabletMetadataCacheSize() != 0) { + Thread.sleep(100); } + + assertEquals(0, cache.getTabletMetadataCacheSize()); + result = cache.get(ke2); + assertEquals(1, cache.getTabletMetadataCacheSize()); + assertEquals(2, cache.getTabletMetadataCacheStats().requestCount()); + assertEquals(2, cache.getTabletMetadataCacheStats().loadSuccessCount()); + assertEquals(tm2, result); } } From c96da16ce8ab25b6c8bdabe8c9a9c070e6904da1 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 22 Feb 2023 22:00:56 +0000 Subject: [PATCH 06/12] Attempt at capturing connection state changes, added tests --- .../server/metadata/TabletMetadataCache.java | 65 ++++++++--- .../functional/TabletMetadataCacheIT.java | 101 ++++++++++++++++++ .../zookeeper/ZooKeeperTestingServer.java | 6 ++ 3 files changed, 160 insertions(+), 12 deletions(-) diff --git a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java index b7e7faf57be..13d68e4f12d 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java +++ b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMetadataCache.java @@ -21,6 +21,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import java.io.Closeable; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.accumulo.core.Constants; import org.apache.accumulo.core.clientImpl.AcceptableThriftTableOperationException; @@ -38,6 +39,8 @@ import org.apache.zookeeper.WatchedEvent; import org.apache.zookeeper.Watcher; import org.apache.zookeeper.Watcher.WatcherType; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.ZooKeeper.States; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -132,7 +135,7 @@ public static void tabletMetadataChanged(ServerContext ctx, KeyExtent extent) { // TabletMetadataCache has been initialized if (ctx.isTabletMetadataCacheInitialized()) { LOG.info("Invalidating extent due to local tablet change: {}", extent); - ctx.getTabletMetadataCache().invalidate(extent); + ctx.getTabletMetadataCache().remove(extent); } // mutate ZK entry for this tablet ctx.getZooReaderWriter().mutateOrCreate(path, serializeData(INITIAL_VALUE), (currVal) -> { @@ -147,9 +150,37 @@ public static void tabletMetadataChanged(ServerContext ctx, KeyExtent extent) { } private Watcher tabletCacheZNodeWatcher = new Watcher() { + @SuppressWarnings("deprecation") @Override public void process(WatchedEvent event) { - LOG.info("Event received: {}", event); + LOG.trace("Event received: {}", event); + switch (event.getState()) { + case Expired: + case Disconnected: + case AuthFailed: + case Closed: + case ConnectedReadOnly: + // Connection issue, invalidate all and stop caching + // until we are connected again. + if (connected.compareAndSet(true, false)) { + LOG.info("Connection issue, clearing cache."); + tabletMetadataCache.invalidateAll(); + } + break; + case SyncConnected: + // Connected, begin caching + if (connected.compareAndSet(false, true)) { + LOG.info("Connection established."); + } + break; + case NoSyncConnected: + case SaslAuthenticated: + case Unknown: + default: + // No ops + break; + + } switch (event.getType()) { case NodeCreated: // Do nothing, cache will be populated on clients first call to get() @@ -162,9 +193,9 @@ public void process(WatchedEvent event) { tabletMetadataCache.invalidate(extent); break; case None: + case NodeChildrenChanged: // These events are not triggered by persistent recursive watches case ChildWatchRemoved: case DataWatchRemoved: - case NodeChildrenChanged: case PersistentWatchRemoved: default: break; @@ -175,12 +206,15 @@ public void process(WatchedEvent event) { private final String watcherPath; private final String znodePath; - private final ZooReaderWriter zrw; + private final Ample ample; + private final ZooKeeper zoo; + protected final AtomicBoolean connected = new AtomicBoolean(false); protected final LoadingCache tabletMetadataCache; public TabletMetadataCache(final InstanceId iid, final ZooReaderWriter zrw, final Ample ample) { - this.zrw = zrw; + this.zoo = zrw.getZooKeeper(); + this.ample = ample; watcherPath = getWatcherPath(iid); znodePath = getZNodePath(iid); @@ -193,8 +227,7 @@ public TabletMetadataCache(final InstanceId iid, final ZooReaderWriter zrw, fina }); try { - this.zrw.getZooKeeper().addWatch(watcherPath, tabletCacheZNodeWatcher, - AddWatchMode.PERSISTENT_RECURSIVE); + zoo.addWatch(watcherPath, tabletCacheZNodeWatcher, AddWatchMode.PERSISTENT_RECURSIVE); } catch (KeeperException e) { throw new RuntimeException("Error setting watch", e); } catch (InterruptedException e) { @@ -215,17 +248,26 @@ protected KeyExtent getExtent(String path) { } /** - * Cached TabletMetadata for the KeyExtent. If it does not exist, then it will be loaded into the - * cache via the CacheLoader which uses Ample to read the TabletMetadata and all of its columns. + * Returns cached TabletMetadata for the KeyExtent. If it does not exist, then it will be loaded + * into the cache via the CacheLoader which uses Ample to read the TabletMetadata and all of its + * columns. If the ZooKeeper connection is down, this will read from ample and skip the cache. * * @param extent KeyExtent * @return TabletMetadata for the KeyExtent */ public TabletMetadata get(KeyExtent extent) { + if (!States.CONNECTED.equals(zoo.getState()) || !connected.get()) { + return ample.readTablet(extent, ColumnType.values()); + } return tabletMetadataCache.get(extent); } - public void invalidate(KeyExtent extent) { + /** + * Remove the cache entry for this KeyExtent + * + * @param extent KeyExtent + */ + private void remove(KeyExtent extent) { tabletMetadataCache.invalidate(extent); } @@ -234,8 +276,7 @@ public void close() { tabletMetadataCache.invalidateAll(); tabletMetadataCache.cleanUp(); try { - this.zrw.getZooKeeper().removeWatches(watcherPath, tabletCacheZNodeWatcher, WatcherType.Any, - false); + zoo.removeWatches(watcherPath, tabletCacheZNodeWatcher, WatcherType.Any, false); } catch (InterruptedException | KeeperException e) { LOG.error("Error removing persistent watcher", e); } diff --git a/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java b/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java index d697b9a4db6..d9c0a046a0f 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java @@ -26,6 +26,7 @@ import java.io.File; import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.accumulo.core.Constants; import org.apache.accumulo.core.clientImpl.AcceptableThriftTableOperationException; @@ -49,8 +50,11 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestMethodOrder; import org.junit.jupiter.api.io.TempDir; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -58,6 +62,7 @@ import com.github.benmanes.caffeine.cache.stats.CacheStats; @Tag(ZOOKEEPER_TESTING_SERVER) +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class TabletMetadataCacheIT { /** @@ -74,6 +79,10 @@ public KeyExtent getExtent(String path) { return super.getExtent(path); } + public AtomicBoolean getConnected() { + return connected; + } + /** * Return size of the cache * @@ -152,6 +161,7 @@ public void after() { } @Test + @Order(1) public void testPathParsing() { String path = TabletMetadataCache.getPath(IID, ke1); assertEquals("/accumulo/" + IID + Constants.ZTABLET_CACHE + "/2;b;", path); @@ -164,6 +174,7 @@ public void testPathParsing() { } @Test + @Order(2) public void testCachingLocalTabletChange() { TabletMetadataCache.tabletMetadataChanged(context, ke1); // This will perform a create in ZK assertEquals(0, cache.getTabletMetadataCacheStats().loadCount()); @@ -213,6 +224,7 @@ public void run() { } @Test + @Order(3) public void testCachingThreadTabletChange() throws InterruptedException { TabletMetadataCache.tabletMetadataChanged(context, ke2); // This will perform a create in ZK @@ -241,4 +253,93 @@ public void testCachingThreadTabletChange() throws InterruptedException { assertEquals(tm2, result); } + + @Test + @Order(4) + public void testZooKeeperConnectionRestart() throws Exception { + TabletMetadataCache.tabletMetadataChanged(context, ke1); // This will perform a create in ZK + assertEquals(0, cache.getTabletMetadataCacheStats().loadCount()); + assertEquals(0, cache.getTabletMetadataCacheSize()); + TabletMetadata result = cache.get(ke1); + assertEquals(1, cache.getTabletMetadataCacheSize()); + assertEquals(1, cache.getTabletMetadataCacheStats().requestCount()); + assertEquals(1, cache.getTabletMetadataCacheStats().loadSuccessCount()); + assertEquals(tm1, result); + TabletMetadataCache.tabletMetadataChanged(context, ke1); // This will perform an update and + // trigger invalidation + assertEquals(0, cache.getTabletMetadataCacheSize()); + result = cache.get(ke1); + assertEquals(1, cache.getTabletMetadataCacheSize()); + assertEquals(2, cache.getTabletMetadataCacheStats().requestCount()); + assertEquals(2, cache.getTabletMetadataCacheStats().loadSuccessCount()); + assertEquals(tm1, result); + + // restart ZooKeeper + szk.restart(); + + // Wait to be disconnected + while (cache.getConnected().get()) { + // TODO: The ZooKeeper connection does not realize that it's disconnected + // until the timeout. The Watcher does not get the disconnected state until + // the server is restarted. The ZooKeeper connection State object is still + // CONNECTED, so the below does not work. This means that because the ZooKeeper + // client does not know immediately that it's disconnected, that it's possible + // that the cache will return a possibly stale cached result until the ZooKeeper + // timeout occurs and the ZooKeeper client object realizes it's disconnected. + // result = cache.get(ke1); + // assertEquals(1, cache.getTabletMetadataCacheSize()); + // assertEquals(2, cache.getTabletMetadataCacheStats().requestCount()); + // assertEquals(2, cache.getTabletMetadataCacheStats().loadSuccessCount()); + // assertEquals(tm1, result); + Thread.sleep(10); + } + + // Wait to be reconnected + while (!cache.getConnected().get()) { + Thread.sleep(10); + } + + assertEquals(0, cache.getTabletMetadataCacheSize()); + result = cache.get(ke1); + assertEquals(1, cache.getTabletMetadataCacheSize()); + assertEquals(3, cache.getTabletMetadataCacheStats().requestCount()); + assertEquals(3, cache.getTabletMetadataCacheStats().loadSuccessCount()); + assertEquals(tm1, result); + + } + + @Test + @Order(5) + public void testZooKeeperDies() throws Exception { + TabletMetadataCache.tabletMetadataChanged(context, ke1); // This will perform a create in ZK + assertEquals(0, cache.getTabletMetadataCacheStats().loadCount()); + assertEquals(0, cache.getTabletMetadataCacheSize()); + TabletMetadata result = cache.get(ke1); + assertEquals(1, cache.getTabletMetadataCacheSize()); + assertEquals(1, cache.getTabletMetadataCacheStats().requestCount()); + assertEquals(1, cache.getTabletMetadataCacheStats().loadSuccessCount()); + assertEquals(tm1, result); + TabletMetadataCache.tabletMetadataChanged(context, ke1); // This will perform an update and + // trigger invalidation + assertEquals(0, cache.getTabletMetadataCacheSize()); + result = cache.get(ke1); + assertEquals(1, cache.getTabletMetadataCacheSize()); + assertEquals(2, cache.getTabletMetadataCacheStats().requestCount()); + assertEquals(2, cache.getTabletMetadataCacheStats().loadSuccessCount()); + assertEquals(tm1, result); + + // close ZooKeeper + szk.close(); + + // Wait to be disconnected + while (cache.getConnected().get()) { + Thread.sleep(10); + } + + assertEquals(0, cache.getTabletMetadataCacheSize()); + result = cache.get(ke1); + assertEquals(0, cache.getTabletMetadataCacheSize()); + + } + } diff --git a/test/src/main/java/org/apache/accumulo/test/zookeeper/ZooKeeperTestingServer.java b/test/src/main/java/org/apache/accumulo/test/zookeeper/ZooKeeperTestingServer.java index dd83d8cf4b6..dd894e1390c 100644 --- a/test/src/main/java/org/apache/accumulo/test/zookeeper/ZooKeeperTestingServer.java +++ b/test/src/main/java/org/apache/accumulo/test/zookeeper/ZooKeeperTestingServer.java @@ -130,6 +130,12 @@ public void initPaths(String s) { } } + public void restart() throws Exception { + if (zkServer != null) { + zkServer.restart(); + } + } + @Override public void close() throws IOException { if (zkServer != null) { From e99d5d16c0c5b05d66fda3115c3e3152c1d0adb3 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Thu, 23 Feb 2023 12:07:21 +0000 Subject: [PATCH 07/12] fix formatting --- .../apache/accumulo/test/functional/TabletMetadataCacheIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java b/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java index d9c0a046a0f..742e953493a 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java @@ -253,7 +253,6 @@ public void testCachingThreadTabletChange() throws InterruptedException { assertEquals(tm2, result); } - @Test @Order(4) public void testZooKeeperConnectionRestart() throws Exception { From 6a98014a15d2a688cec4792a8528646ddf494c05 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Thu, 23 Feb 2023 21:22:23 +0000 Subject: [PATCH 08/12] Try to improve test --- .../functional/TabletMetadataCacheIT.java | 38 +++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java b/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java index 742e953493a..3b32108283b 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataCacheIT.java @@ -23,6 +23,7 @@ import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.File; import java.util.UUID; @@ -273,26 +274,33 @@ public void testZooKeeperConnectionRestart() throws Exception { assertEquals(2, cache.getTabletMetadataCacheStats().loadSuccessCount()); assertEquals(tm1, result); - // restart ZooKeeper - szk.restart(); + // Close ZK Server + szk.close(); + LoggerFactory.getLogger(TabletMetadataCacheIT.class).info("ZK Server closed"); + long requestCount = 0L; // Wait to be disconnected while (cache.getConnected().get()) { // TODO: The ZooKeeper connection does not realize that it's disconnected - // until the timeout. The Watcher does not get the disconnected state until - // the server is restarted. The ZooKeeper connection State object is still - // CONNECTED, so the below does not work. This means that because the ZooKeeper - // client does not know immediately that it's disconnected, that it's possible - // that the cache will return a possibly stale cached result until the ZooKeeper - // timeout occurs and the ZooKeeper client object realizes it's disconnected. - // result = cache.get(ke1); - // assertEquals(1, cache.getTabletMetadataCacheSize()); - // assertEquals(2, cache.getTabletMetadataCacheStats().requestCount()); - // assertEquals(2, cache.getTabletMetadataCacheStats().loadSuccessCount()); - // assertEquals(tm1, result); + // immediately. Depending on the type of connection failure there is some + // amount of time before the Watcher gets notified. For example, it appears + // that if the server closes, like in this test, then the Watcher gets notified + // rather quickly (less than 100ms). In the case where there is a network partition + // or the ZK Server is restarted, the Watcher might not get the event for some time. + // This means that because the ZooKeeper client does not know immediately that it's + // disconnected, that it's possible that the cache will return a possibly stale result + result = cache.get(ke1); + requestCount = cache.getTabletMetadataCacheStats().requestCount(); + assertEquals(1, cache.getTabletMetadataCacheSize()); + assertTrue(requestCount > 2); + assertEquals(3, cache.getTabletMetadataCacheStats().loadSuccessCount()); + assertEquals(tm1, result); Thread.sleep(10); } + // Start ZK Server + szk.restart(); + // Wait to be reconnected while (!cache.getConnected().get()) { Thread.sleep(10); @@ -301,8 +309,8 @@ public void testZooKeeperConnectionRestart() throws Exception { assertEquals(0, cache.getTabletMetadataCacheSize()); result = cache.get(ke1); assertEquals(1, cache.getTabletMetadataCacheSize()); - assertEquals(3, cache.getTabletMetadataCacheStats().requestCount()); - assertEquals(3, cache.getTabletMetadataCacheStats().loadSuccessCount()); + assertEquals(requestCount + 1, cache.getTabletMetadataCacheStats().requestCount()); + assertEquals(4, cache.getTabletMetadataCacheStats().loadSuccessCount()); assertEquals(tm1, result); } From 9921c98f13c1761f87609cb643845ab033c69449 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Fri, 24 Feb 2023 14:43:36 +0000 Subject: [PATCH 09/12] Bump minimum ZK version from 3.5.10 to 3.6.0 --- .github/workflows/maven.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/maven.yaml b/.github/workflows/maven.yaml index 54368e245a4..1c133da0077 100644 --- a/.github/workflows/maven.yaml +++ b/.github/workflows/maven.yaml @@ -70,7 +70,7 @@ jobs: profile: - {name: 'unit-tests', javaver: 11, args: 'verify -PskipQA -DskipTests=false'} - {name: 'qa-checks', javaver: 11, args: 'verify javadoc:jar -Psec-bugs -DskipTests -Dspotbugs.timeout=3600000'} - - {name: 'compat', javaver: 11, args: 'package -DskipTests -Dhadoop.version=3.0.3 -Dzookeeper.version=3.5.10'} + - {name: 'compat', javaver: 11, args: 'package -DskipTests -Dhadoop.version=3.0.3 -Dzookeeper.version=3.6.0'} - {name: 'errorprone', javaver: 11, args: 'verify -Perrorprone,skipQA'} - {name: 'jdk17', javaver: 17, args: 'verify -DskipITs'} fail-fast: false From 58fc7aaca68898e38e74a5e74249c70155e73e23 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Fri, 24 Feb 2023 15:12:28 +0000 Subject: [PATCH 10/12] Bump required ZooKeeper minimum to 3.6.1, exclude banned dependencies --- .github/workflows/maven.yaml | 2 +- pom.xml | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/maven.yaml b/.github/workflows/maven.yaml index 1c133da0077..cfb343d1bcd 100644 --- a/.github/workflows/maven.yaml +++ b/.github/workflows/maven.yaml @@ -70,7 +70,7 @@ jobs: profile: - {name: 'unit-tests', javaver: 11, args: 'verify -PskipQA -DskipTests=false'} - {name: 'qa-checks', javaver: 11, args: 'verify javadoc:jar -Psec-bugs -DskipTests -Dspotbugs.timeout=3600000'} - - {name: 'compat', javaver: 11, args: 'package -DskipTests -Dhadoop.version=3.0.3 -Dzookeeper.version=3.6.0'} + - {name: 'compat', javaver: 11, args: 'package -DskipTests -Dhadoop.version=3.0.3 -Dzookeeper.version=3.6.1'} - {name: 'errorprone', javaver: 11, args: 'verify -Perrorprone,skipQA'} - {name: 'jdk17', javaver: 17, args: 'verify -DskipITs'} fail-fast: false diff --git a/pom.xml b/pom.xml index e4325ea40e5..fa0e49bf5d8 100644 --- a/pom.xml +++ b/pom.xml @@ -545,6 +545,14 @@ ch.qos.logback logback-core + + log4j + log4j + + + org.slf4j + slf4j-log4j12 + From d1413e5db62eacef9b1e317e4739f372665f78bf Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 1 Mar 2023 20:48:56 +0000 Subject: [PATCH 11/12] Bump minimum version of ZK for build to 3.6.4, try to use older version of Maven --- .github/workflows/maven.yaml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/maven.yaml b/.github/workflows/maven.yaml index cfb343d1bcd..11fc3e2ffe1 100644 --- a/.github/workflows/maven.yaml +++ b/.github/workflows/maven.yaml @@ -57,6 +57,13 @@ jobs: run: contrib/ci/find-unapproved-chars.sh - name: Check for unapproved JUnit API usage run: contrib/ci/find-unapproved-junit.sh + # + # Use an older version of Maven due to issues with plugins in Maven 3.9.0 + # + - name: Set up Maven + uses: stCarolas/setup-maven@v4.5 + with: + maven-version: 3.8.7 - name: Build with Maven (Fast Build) timeout-minutes: 20 run: mvn -B -V -e -ntp "-Dstyle.color=always" clean package dependency:resolve -DskipTests -DskipFormat -DverifyFormat @@ -70,7 +77,7 @@ jobs: profile: - {name: 'unit-tests', javaver: 11, args: 'verify -PskipQA -DskipTests=false'} - {name: 'qa-checks', javaver: 11, args: 'verify javadoc:jar -Psec-bugs -DskipTests -Dspotbugs.timeout=3600000'} - - {name: 'compat', javaver: 11, args: 'package -DskipTests -Dhadoop.version=3.0.3 -Dzookeeper.version=3.6.1'} + - {name: 'compat', javaver: 11, args: 'package -DskipTests -Dhadoop.version=3.0.3 -Dzookeeper.version=3.6.4'} - {name: 'errorprone', javaver: 11, args: 'verify -Perrorprone,skipQA'} - {name: 'jdk17', javaver: 17, args: 'verify -DskipITs'} fail-fast: false From b9961e27f5b41c6d32be4cddc53846d42c1af88a Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Fri, 3 Mar 2023 13:59:08 +0000 Subject: [PATCH 12/12] Remove changes in github yaml to use different maven version --- .github/workflows/maven.yaml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.github/workflows/maven.yaml b/.github/workflows/maven.yaml index 11fc3e2ffe1..76df8e5de8a 100644 --- a/.github/workflows/maven.yaml +++ b/.github/workflows/maven.yaml @@ -57,13 +57,6 @@ jobs: run: contrib/ci/find-unapproved-chars.sh - name: Check for unapproved JUnit API usage run: contrib/ci/find-unapproved-junit.sh - # - # Use an older version of Maven due to issues with plugins in Maven 3.9.0 - # - - name: Set up Maven - uses: stCarolas/setup-maven@v4.5 - with: - maven-version: 3.8.7 - name: Build with Maven (Fast Build) timeout-minutes: 20 run: mvn -B -V -e -ntp "-Dstyle.color=always" clean package dependency:resolve -DskipTests -DskipFormat -DverifyFormat