This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push:
new cc9aff0bbc Remove static methods and fields from TableManager (#5420)
cc9aff0bbc is described below
commit cc9aff0bbc563afb74a232e36f6473e9f9bae911
Author: Christopher Tubbs <[email protected]>
AuthorDate: Thu Mar 20 18:16:06 2025 -0400
Remove static methods and fields from TableManager (#5420)
Static methods are removed in favor of non-static ones, since these
methods are only ever called with the ServerContext, which already has
an instance of the TableManager object. This allows removing an
overloaded prepareNewTableState method, since the extra parameters on
the overloaded method are always provided by methods on the
ServerContext anyway.
Additionally, this change removes the static set of registered ZooCache
observers and table state cache, since the lifetime of ZooCache is
already bound to the lifetime of the ServerContext, and these static
objects should not exist longer than the TableManager instance attached
to the same ServerContext.
This fixes a potential bug with the ZooCache observers registered
statically never firing again if the ServerContext is closed, even if an
attempt is made to register them again with a new ServerContext. There
is usually only one ServerContext for the lifetime of a server process,
so this bug would probably not exist in practice, but this change avoids
a potential bug if that assumption did not hold.
---
.../accumulo/server/init/ZooKeeperInitializer.java | 15 +++----
.../accumulo/server/tables/TableManager.java | 52 ++++++++++------------
.../create/PopulateZookeeperWithNamespace.java | 3 +-
3 files changed, 31 insertions(+), 39 deletions(-)
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 0ef60f0f49..c9d8706946 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
@@ -49,7 +49,6 @@ import
org.apache.accumulo.server.conf.codec.VersionedProperties;
import org.apache.accumulo.server.conf.store.SystemPropKey;
import org.apache.accumulo.server.log.WalStateManager;
import org.apache.accumulo.server.metadata.RootGcCandidates;
-import org.apache.accumulo.server.tables.TableManager;
import org.apache.zookeeper.KeeperException;
import org.apache.zookeeper.ZooDefs;
@@ -103,15 +102,15 @@ public class ZooKeeperInitializer {
Namespace.ACCUMULO.id().canonical(),
Namespace.ACCUMULO.name())),
ZooUtil.NodeExistsPolicy.FAIL);
- TableManager.prepareNewNamespaceState(context, Namespace.DEFAULT.id(),
Namespace.DEFAULT.name(),
- ZooUtil.NodeExistsPolicy.FAIL);
- TableManager.prepareNewNamespaceState(context, Namespace.ACCUMULO.id(),
+ context.getTableManager().prepareNewNamespaceState(Namespace.DEFAULT.id(),
+ Namespace.DEFAULT.name(), ZooUtil.NodeExistsPolicy.FAIL);
+ context.getTableManager().prepareNewNamespaceState(Namespace.ACCUMULO.id(),
Namespace.ACCUMULO.name(), ZooUtil.NodeExistsPolicy.FAIL);
- TableManager.prepareNewTableState(context, AccumuloTable.ROOT.tableId(),
+
context.getTableManager().prepareNewTableState(AccumuloTable.ROOT.tableId(),
Namespace.ACCUMULO.id(), AccumuloTable.ROOT.tableName(),
TableState.ONLINE,
ZooUtil.NodeExistsPolicy.FAIL);
- TableManager.prepareNewTableState(context,
AccumuloTable.METADATA.tableId(),
+
context.getTableManager().prepareNewTableState(AccumuloTable.METADATA.tableId(),
Namespace.ACCUMULO.id(), AccumuloTable.METADATA.tableName(),
TableState.ONLINE,
ZooUtil.NodeExistsPolicy.FAIL);
// Call this separately so the upgrader code can handle the zk node
creation for scan refs
@@ -183,7 +182,7 @@ public class ZooKeeperInitializer {
public void initScanRefTableState(ServerContext context) {
try {
- TableManager.prepareNewTableState(context,
AccumuloTable.SCAN_REF.tableId(),
+
context.getTableManager().prepareNewTableState(AccumuloTable.SCAN_REF.tableId(),
Namespace.ACCUMULO.id(), AccumuloTable.SCAN_REF.tableName(),
TableState.ONLINE,
ZooUtil.NodeExistsPolicy.FAIL);
} catch (KeeperException | InterruptedException e) {
@@ -193,7 +192,7 @@ public class ZooKeeperInitializer {
public void initFateTableState(ServerContext context) {
try {
- TableManager.prepareNewTableState(context, AccumuloTable.FATE.tableId(),
+
context.getTableManager().prepareNewTableState(AccumuloTable.FATE.tableId(),
Namespace.ACCUMULO.id(), AccumuloTable.FATE.tableName(),
TableState.ONLINE,
ZooUtil.NodeExistsPolicy.FAIL);
} catch (KeeperException | InterruptedException e) {
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/tables/TableManager.java
b/server/base/src/main/java/org/apache/accumulo/server/tables/TableManager.java
index afd0476b20..adab9869f7 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/tables/TableManager.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/tables/TableManager.java
@@ -57,29 +57,37 @@ import com.google.common.base.Preconditions;
public class TableManager {
private static final Logger log =
LoggerFactory.getLogger(TableManager.class);
- private static final Set<TableObserver> observers =
Collections.synchronizedSet(new HashSet<>());
- private static final Map<TableId,TableState> tableStateCache =
- Collections.synchronizedMap(new HashMap<>());
private static final byte[] ZERO_BYTE = {'0'};
+ private final Set<TableObserver> observers = Collections.synchronizedSet(new
HashSet<>());
+ private final Map<TableId,TableState> tableStateCache =
+ Collections.synchronizedMap(new HashMap<>());
+
private final ServerContext context;
private final ZooReaderWriter zoo;
- public static void prepareNewNamespaceState(final ServerContext context,
NamespaceId namespaceId,
- String namespace, NodeExistsPolicy existsPolicy)
- throws KeeperException, InterruptedException {
+ public TableManager(ServerContext context) {
+ this.context = context;
+ this.zoo = context.getZooSession().asReaderWriter();
+
+ // add our Watcher to the shared ZooCache
+ context.getZooCache().addZooCacheWatcher(new TableStateWatcher());
+ updateTableStateCache();
+ }
+
+ public void prepareNewNamespaceState(NamespaceId namespaceId, String
namespace,
+ NodeExistsPolicy existsPolicy) throws KeeperException,
InterruptedException {
final PropStore propStore = context.getPropStore();
log.debug("Creating ZooKeeper entries for new namespace {} (ID: {})",
namespace, namespaceId);
- context.getZooSession().asReaderWriter()
- .putPersistentData(Constants.ZNAMESPACES + "/" + namespaceId, new
byte[0], existsPolicy);
+ zoo.putPersistentData(Constants.ZNAMESPACES + "/" + namespaceId, new
byte[0], existsPolicy);
var propKey = NamespacePropKey.of(namespaceId);
if (!propStore.exists(propKey)) {
propStore.create(propKey, Map.of());
}
}
- public static void prepareNewTableState(ZooReaderWriter zoo, PropStore
propStore, TableId tableId,
- NamespaceId namespaceId, String tableName, TableState state,
NodeExistsPolicy existsPolicy)
+ public void prepareNewTableState(TableId tableId, NamespaceId namespaceId,
String tableName,
+ TableState state, NodeExistsPolicy existsPolicy)
throws KeeperException, InterruptedException {
// state gets created last
log.debug("Creating ZooKeeper entries for new table {} (ID: {}) in
namespace (ID: {})",
@@ -96,26 +104,12 @@ public class TableManager {
zoo.putPersistentData(zTablePath + Constants.ZTABLE_STATE,
state.name().getBytes(UTF_8),
existsPolicy);
var propKey = TablePropKey.of(tableId);
+ var propStore = context.getPropStore();
if (!propStore.exists(propKey)) {
propStore.create(propKey, Map.of());
}
}
- public static void prepareNewTableState(final ServerContext context, TableId
tableId,
- NamespaceId namespaceId, String tableName, TableState state,
NodeExistsPolicy existsPolicy)
- throws KeeperException, InterruptedException {
- prepareNewTableState(context.getZooSession().asReaderWriter(),
context.getPropStore(), tableId,
- namespaceId, tableName, state, existsPolicy);
- }
-
- public TableManager(ServerContext context) {
- this.context = context;
- zoo = context.getZooSession().asReaderWriter();
- // add our Watcher to the shared ZooCache
- context.getZooCache().addZooCacheWatcher(new TableStateWatcher());
- updateTableStateCache();
- }
-
public TableState getTableState(TableId tableId) {
return tableStateCache.get(tableId);
}
@@ -198,16 +192,16 @@ public class TableManager {
public void addTable(TableId tableId, NamespaceId namespaceId, String
tableName)
throws KeeperException, InterruptedException, NamespaceNotFoundException
{
- prepareNewTableState(zoo, context.getPropStore(), tableId, namespaceId,
tableName,
- TableState.NEW, NodeExistsPolicy.OVERWRITE);
+ prepareNewTableState(tableId, namespaceId, tableName, TableState.NEW,
+ NodeExistsPolicy.OVERWRITE);
updateTableStateCache(tableId);
}
public void cloneTable(TableId srcTableId, TableId tableId, String tableName,
NamespaceId namespaceId, Map<String,String> propertiesToSet, Set<String>
propertiesToExclude)
throws KeeperException, InterruptedException {
- prepareNewTableState(zoo, context.getPropStore(), tableId, namespaceId,
tableName,
- TableState.NEW, NodeExistsPolicy.OVERWRITE);
+ prepareNewTableState(tableId, namespaceId, tableName, TableState.NEW,
+ NodeExistsPolicy.OVERWRITE);
String srcTablePath = Constants.ZTABLES + "/" + srcTableId +
Constants.ZCONFIG;
String newTablePath = Constants.ZTABLES + "/" + tableId +
Constants.ZCONFIG;
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/namespace/create/PopulateZookeeperWithNamespace.java
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/namespace/create/PopulateZookeeperWithNamespace.java
index b9ab849390..48f2033243 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/namespace/create/PopulateZookeeperWithNamespace.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/namespace/create/PopulateZookeeperWithNamespace.java
@@ -28,7 +28,6 @@ import org.apache.accumulo.manager.Manager;
import org.apache.accumulo.manager.tableOps.ManagerRepo;
import org.apache.accumulo.manager.tableOps.Utils;
import org.apache.accumulo.server.conf.store.NamespacePropKey;
-import org.apache.accumulo.server.tables.TableManager;
import org.apache.accumulo.server.util.PropUtil;
class PopulateZookeeperWithNamespace extends ManagerRepo {
@@ -55,7 +54,7 @@ class PopulateZookeeperWithNamespace extends ManagerRepo {
var context = manager.getContext();
NamespaceMapping.put(context.getZooSession().asReaderWriter(),
namespaceInfo.namespaceId,
namespaceInfo.namespaceName);
- TableManager.prepareNewNamespaceState(context, namespaceInfo.namespaceId,
+
context.getTableManager().prepareNewNamespaceState(namespaceInfo.namespaceId,
namespaceInfo.namespaceName, NodeExistsPolicy.OVERWRITE);
PropUtil.setProperties(context,
NamespacePropKey.of(namespaceInfo.namespaceId),
