keith-turner commented on code in PR #5451: URL: https://github.com/apache/accumulo/pull/5451#discussion_r2031638428
########## core/src/main/java/org/apache/accumulo/core/util/tables/TableMapping.java: ########## @@ -0,0 +1,201 @@ +/* + * 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.core.util.tables; + +import static java.util.Collections.emptySortedMap; +import static java.util.Objects.requireNonNull; +import static org.apache.accumulo.core.clientImpl.NamespaceMapping.deserializeMap; +import static org.apache.accumulo.core.clientImpl.NamespaceMapping.serializeMap; + +import java.util.Map; +import java.util.Objects; +import java.util.SortedMap; +import java.util.stream.Stream; + +import org.apache.accumulo.core.Constants; +import org.apache.accumulo.core.clientImpl.AcceptableThriftTableOperationException; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.clientImpl.Namespace; +import org.apache.accumulo.core.clientImpl.thrift.TableOperation; +import org.apache.accumulo.core.clientImpl.thrift.TableOperationExceptionType; +import org.apache.accumulo.core.data.NamespaceId; +import org.apache.accumulo.core.data.TableId; +import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.core.metadata.AccumuloTable; +import org.apache.accumulo.core.zookeeper.ZcStat; +import org.apache.accumulo.core.zookeeper.ZooCache; +import org.apache.zookeeper.KeeperException; + +import com.google.common.collect.ImmutableSortedMap; + +public class TableMapping { + + private final ClientContext context; + private final NamespaceId namespaceId; + private volatile SortedMap<TableId,String> currentTableMap = emptySortedMap(); + private volatile SortedMap<String,TableId> currentTableReverseMap = emptySortedMap(); + private volatile long lastMzxid; + + public TableMapping(ClientContext context, NamespaceId namespaceId) { + this.context = context; + this.namespaceId = namespaceId; + } + + public void put(final ClientContext context, TableId tableId, String tableName, + TableOperation operation) + throws InterruptedException, KeeperException, AcceptableThriftTableOperationException { + var zoo = context.getZooSession().asReaderWriter(); + Stream.of(zoo, tableId, namespaceId, tableName).forEach(Objects::requireNonNull); + String zTableMapPath = getZTableMapPath(namespaceId); + if (!zoo.exists(zTableMapPath)) { + throw new KeeperException.NoNodeException(zTableMapPath + " does not exist in ZooKeeper"); + } + if (isBuiltInZKTable(tableId)) { + throw new AssertionError("Putting built-in tables in map should not be possible after init"); + } + zoo.mutateExisting(zTableMapPath, data -> { + var tables = deserializeMap(data); + final String currentName = tables.get(tableId.canonical()); + if (tableName.equals(currentName)) { + return null; // mapping already exists; operation is idempotent, so no change needed + } + if (currentName != null) { + throw new AcceptableThriftTableOperationException(null, tableId.canonical(), operation, + TableOperationExceptionType.EXISTS, "Table Id already exists"); + } + if (tables.containsValue(tableName)) { + throw new AcceptableThriftTableOperationException(null, tableId.canonical(), operation, + TableOperationExceptionType.EXISTS, "Table name already exists"); + } + tables.put(tableId.canonical(), tableName); + return serializeMap(tables); + }); + } + + public void remove(final ClientContext context, final TableId tableId) + throws InterruptedException, KeeperException, AcceptableThriftTableOperationException { + var zoo = context.getZooSession().asReaderWriter(); + Stream.of(zoo, tableId).forEach(Objects::requireNonNull); + if (isBuiltInZKTable(tableId)) { + throw new AssertionError("Removing built-in tables in map should not be possible"); + } + zoo.mutateExisting(getZTableMapPath(getNamespaceOfTableId(zoo, tableId)), data -> { + var tables = deserializeMap(data); + if (!tables.containsKey(tableId.canonical())) { + throw new AcceptableThriftTableOperationException(null, tableId.canonical(), + TableOperation.DELETE, TableOperationExceptionType.NOTFOUND, + "Table already removed while processing"); + } + tables.remove(tableId.canonical()); + return serializeMap(tables); + }); + } + + public void rename(final ClientContext context, final TableId tableId, final String oldName, + final String newName) + throws InterruptedException, KeeperException, AcceptableThriftTableOperationException { + var zoo = context.getZooSession().asReaderWriter(); + Stream.of(zoo, tableId, namespaceId, oldName, newName).forEach(Objects::requireNonNull); + String zTableMapPath = getZTableMapPath(namespaceId); + if (!zoo.exists(zTableMapPath)) { + throw new KeeperException.NoNodeException(zTableMapPath + " does not exist in ZooKeeper"); + } Review Comment: Would be nice to remove this exists check if possible ########## core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java: ########## @@ -1100,6 +1099,16 @@ public NamespaceMapping getNamespaces() { return namespaces; } + public TableMapping getTableMapping(NamespaceId namespaceId) { + ensureOpen(); + TableMapping tableMapping = tableMappings.getIfPresent(namespaceId); Review Comment: This code has race conditions w/ the seperate calls to get and put. Could posibly use `asMap().computeIfAbsent(...)` on the cache. Alternatively could look at calling the `get(K key, Function mappingFunction)` method directly on the cache. ########## core/src/main/java/org/apache/accumulo/core/util/tables/TableZooHelper.java: ########## @@ -62,82 +55,81 @@ public TableZooHelper(ClientContext context) { * getCause() of NamespaceNotFoundException */ public TableId getTableId(String tableName) throws TableNotFoundException { - for (AccumuloTable systemTable : AccumuloTable.values()) { - if (systemTable.tableName().equals(tableName)) { - return systemTable.tableId(); - } + Pair<String,String> qualified = TableNameUtil.qualify(tableName); + NamespaceId nid = context.getNamespaces().getNameToIdMap().get(qualified.getFirst()); + if (nid == null) { + throw new TableNotFoundException(tableName, new NamespaceNotFoundException(null, + qualified.getFirst(), "No mapping found for namespace")); } - try { - return _getTableIdDetectNamespaceNotFound(EXISTING_TABLE_NAME.validate(tableName)); - } catch (NamespaceNotFoundException e) { - throw new TableNotFoundException(tableName, e); + TableId tid = context.getTableMapping(nid).getNameToIdMap().get(qualified.getSecond()); + if (tid == null) { + throw new TableNotFoundException(null, tableName, + "No entry for this table found in the given namespace mapping"); } + return tid; } - /** - * Lookup table ID in ZK. If not found, clears cache and tries again. - */ - public TableId _getTableIdDetectNamespaceNotFound(String tableName) - throws NamespaceNotFoundException, TableNotFoundException { - TableId tableId = getTableMap().getNameToIdMap().get(tableName); - if (tableId == null) { - // maybe the table exist, but the cache was not updated yet... - // so try to clear the cache and check again - clearTableListCache(); - tableId = getTableMap().getNameToIdMap().get(tableName); - if (tableId == null) { - String namespace = TableNameUtil.qualify(tableName).getFirst(); - if (Namespaces.getNameToIdMap(context).containsKey(namespace)) { - throw new TableNotFoundException(null, tableName, null); - } else { - throw new NamespaceNotFoundException(null, namespace, null); - } + public String getTableName(TableId tableId) throws TableNotFoundException { + Map<NamespaceId,String> namespaceMapping = context.getNamespaces().getIdToNameMap(); + for (NamespaceId namespaceId : namespaceMapping.keySet()) { + var tableIdToNameMap = context.getTableMapping(namespaceId).getIdToNameMap(); + if (tableIdToNameMap.containsKey(tableId)) { + return TableNameUtil.qualified(tableIdToNameMap.get(tableId), + namespaceMapping.get(namespaceId)); } } - return tableId; + throw new TableNotFoundException(tableId.canonical(), null, + "No entry for this table Id found in table mappings"); } - public String getTableName(TableId tableId) throws TableNotFoundException { - for (AccumuloTable systemTable : AccumuloTable.values()) { - if (systemTable.tableId().equals(tableId)) { - return systemTable.tableName(); + private Map<String,String> loadQualifiedTableMapping(boolean reverse) { + final var builder = ImmutableMap.<String,String>builder(); + for (NamespaceId namespaceId : context.getNamespaces().getIdToNameMap().keySet()) { Review Comment: Some of these functions used to do a direct map lookup and now they iterator over all namespaces. Was wondering if that matters, looked at what code uses these a little bit and did not see any cases where it matters. Was looking for any code that runs really frequently and calls these functions that iterate over all namespaces, did not see anything but also did not look comprehensively. Curious if you looked into how these are used? ########## core/src/main/java/org/apache/accumulo/core/util/tables/TableZooHelper.java: ########## @@ -62,82 +55,81 @@ public TableZooHelper(ClientContext context) { * getCause() of NamespaceNotFoundException */ public TableId getTableId(String tableName) throws TableNotFoundException { - for (AccumuloTable systemTable : AccumuloTable.values()) { - if (systemTable.tableName().equals(tableName)) { - return systemTable.tableId(); - } + Pair<String,String> qualified = TableNameUtil.qualify(tableName); + NamespaceId nid = context.getNamespaces().getNameToIdMap().get(qualified.getFirst()); + if (nid == null) { + throw new TableNotFoundException(tableName, new NamespaceNotFoundException(null, + qualified.getFirst(), "No mapping found for namespace")); } - try { - return _getTableIdDetectNamespaceNotFound(EXISTING_TABLE_NAME.validate(tableName)); - } catch (NamespaceNotFoundException e) { - throw new TableNotFoundException(tableName, e); + TableId tid = context.getTableMapping(nid).getNameToIdMap().get(qualified.getSecond()); Review Comment: When the namespace does not exists will this code still throw a TableNotFoundException? ########## core/src/main/java/org/apache/accumulo/core/util/tables/TableZooHelper.java: ########## @@ -198,17 +189,15 @@ public NamespaceId getNamespaceId(TableId tableId) throws TableNotFoundException return Namespace.ACCUMULO.id(); } - ZooCache zc = context.getZooCache(); - byte[] n = zc.get(Constants.ZTABLES + "/" + tableId + Constants.ZTABLE_NAMESPACE); - // We might get null out of ZooCache if this tableID doesn't exist - if (n == null) { - throw new TableNotFoundException(tableId.canonical(), null, null); + for (NamespaceId namespaceId : context.getNamespaces().getIdToNameMap().keySet()) { + for (Map.Entry<TableId,String> entry : context.getTableMapping(namespaceId).getIdToNameMap() Review Comment: Could the scan of the map be avoided and map lookup be done instead here? ########## core/src/main/java/org/apache/accumulo/core/util/tables/TableZooHelper.java: ########## @@ -62,82 +55,81 @@ public TableZooHelper(ClientContext context) { * getCause() of NamespaceNotFoundException */ public TableId getTableId(String tableName) throws TableNotFoundException { - for (AccumuloTable systemTable : AccumuloTable.values()) { - if (systemTable.tableName().equals(tableName)) { - return systemTable.tableId(); - } + Pair<String,String> qualified = TableNameUtil.qualify(tableName); + NamespaceId nid = context.getNamespaces().getNameToIdMap().get(qualified.getFirst()); + if (nid == null) { + throw new TableNotFoundException(tableName, new NamespaceNotFoundException(null, + qualified.getFirst(), "No mapping found for namespace")); } - try { - return _getTableIdDetectNamespaceNotFound(EXISTING_TABLE_NAME.validate(tableName)); - } catch (NamespaceNotFoundException e) { - throw new TableNotFoundException(tableName, e); + TableId tid = context.getTableMapping(nid).getNameToIdMap().get(qualified.getSecond()); + if (tid == null) { + throw new TableNotFoundException(null, tableName, + "No entry for this table found in the given namespace mapping"); } + return tid; } - /** - * Lookup table ID in ZK. If not found, clears cache and tries again. - */ - public TableId _getTableIdDetectNamespaceNotFound(String tableName) - throws NamespaceNotFoundException, TableNotFoundException { - TableId tableId = getTableMap().getNameToIdMap().get(tableName); - if (tableId == null) { - // maybe the table exist, but the cache was not updated yet... - // so try to clear the cache and check again - clearTableListCache(); - tableId = getTableMap().getNameToIdMap().get(tableName); - if (tableId == null) { - String namespace = TableNameUtil.qualify(tableName).getFirst(); - if (Namespaces.getNameToIdMap(context).containsKey(namespace)) { - throw new TableNotFoundException(null, tableName, null); - } else { - throw new NamespaceNotFoundException(null, namespace, null); - } + public String getTableName(TableId tableId) throws TableNotFoundException { + Map<NamespaceId,String> namespaceMapping = context.getNamespaces().getIdToNameMap(); + for (NamespaceId namespaceId : namespaceMapping.keySet()) { + var tableIdToNameMap = context.getTableMapping(namespaceId).getIdToNameMap(); + if (tableIdToNameMap.containsKey(tableId)) { + return TableNameUtil.qualified(tableIdToNameMap.get(tableId), + namespaceMapping.get(namespaceId)); } } - return tableId; + throw new TableNotFoundException(tableId.canonical(), null, + "No entry for this table Id found in table mappings"); } - public String getTableName(TableId tableId) throws TableNotFoundException { - for (AccumuloTable systemTable : AccumuloTable.values()) { - if (systemTable.tableId().equals(tableId)) { - return systemTable.tableName(); + private Map<String,String> loadQualifiedTableMapping(boolean reverse) { + final var builder = ImmutableMap.<String,String>builder(); + for (NamespaceId namespaceId : context.getNamespaces().getIdToNameMap().keySet()) { + for (Map.Entry<TableId,String> entry : context.getTableMapping(namespaceId).getIdToNameMap() + .entrySet()) { + String fullyQualifiedName; + try { + fullyQualifiedName = TableNameUtil.qualified(entry.getValue(), + Namespaces.getNamespaceName(context, namespaceId)); + } catch (NamespaceNotFoundException e) { + throw new RuntimeException( + "getNamespaceName() failed to find namespace for namespaceId: " + namespaceId, e); + } + if (reverse) { + builder.put(fullyQualifiedName, entry.getKey().canonical()); + } else { + builder.put(entry.getKey().canonical(), fullyQualifiedName); + } } } - String tableName = getTableMap().getIdtoNameMap().get(tableId); - if (tableName == null) { - throw new TableNotFoundException(tableId.canonical(), null, null); - } - return tableName; + return builder.build(); } - /** - * Get the TableMap from the cache. A new one will be populated when needed. Cache is cleared - * manually by calling {@link #clearTableListCache()} - */ - public TableMap getTableMap() { - final ZooCache zc = context.getZooCache(); - TableMap map = getCachedTableMap(); - if (!map.isCurrent(zc)) { - instanceToMapCache.invalidateAll(); - map = getCachedTableMap(); - } - return map; + public Map<String,TableId> getQualifiedNameToIdMap() { + return loadQualifiedTableMapping(true).entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, e -> TableId.of(e.getValue()))); } - private TableMap getCachedTableMap() { - return instanceToMapCache.get(this, k -> new TableMap(context)); + public Map<TableId,String> getIdtoQualifiedNameMap() { + return loadQualifiedTableMapping(false).entrySet().stream() + .collect(Collectors.toMap(e -> TableId.of(e.getKey()), Map.Entry::getValue)); } public boolean tableNodeExists(TableId tableId) { - ZooCache zc = context.getZooCache(); - List<String> tableIds = zc.getChildren(Constants.ZTABLES); - return tableIds.contains(tableId.canonical()); + for (NamespaceId namespaceId : context.getNamespaces().getIdToNameMap().keySet()) { + for (Map.Entry<TableId,String> entry : context.getTableMapping(namespaceId).getIdToNameMap() + .entrySet()) { + if (entry.getKey().equals(tableId)) { + return true; + } + } Review Comment: Could the scan of the be avoided by doing the following? ```suggestion if(context.getTableMapping(namespaceId).getIdToNameMap().containsKey(tableId)){ return true; } ``` ########## core/src/main/java/org/apache/accumulo/core/util/tables/TableMapping.java: ########## @@ -0,0 +1,201 @@ +/* + * 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.core.util.tables; + +import static java.util.Collections.emptySortedMap; +import static java.util.Objects.requireNonNull; +import static org.apache.accumulo.core.clientImpl.NamespaceMapping.deserializeMap; +import static org.apache.accumulo.core.clientImpl.NamespaceMapping.serializeMap; + +import java.util.Map; +import java.util.Objects; +import java.util.SortedMap; +import java.util.stream.Stream; + +import org.apache.accumulo.core.Constants; +import org.apache.accumulo.core.clientImpl.AcceptableThriftTableOperationException; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.clientImpl.Namespace; +import org.apache.accumulo.core.clientImpl.thrift.TableOperation; +import org.apache.accumulo.core.clientImpl.thrift.TableOperationExceptionType; +import org.apache.accumulo.core.data.NamespaceId; +import org.apache.accumulo.core.data.TableId; +import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.core.metadata.AccumuloTable; +import org.apache.accumulo.core.zookeeper.ZcStat; +import org.apache.accumulo.core.zookeeper.ZooCache; +import org.apache.zookeeper.KeeperException; + +import com.google.common.collect.ImmutableSortedMap; + +public class TableMapping { + + private final ClientContext context; + private final NamespaceId namespaceId; + private volatile SortedMap<TableId,String> currentTableMap = emptySortedMap(); + private volatile SortedMap<String,TableId> currentTableReverseMap = emptySortedMap(); + private volatile long lastMzxid; + + public TableMapping(ClientContext context, NamespaceId namespaceId) { + this.context = context; + this.namespaceId = namespaceId; + } + + public void put(final ClientContext context, TableId tableId, String tableName, + TableOperation operation) + throws InterruptedException, KeeperException, AcceptableThriftTableOperationException { + var zoo = context.getZooSession().asReaderWriter(); + Stream.of(zoo, tableId, namespaceId, tableName).forEach(Objects::requireNonNull); + String zTableMapPath = getZTableMapPath(namespaceId); + if (!zoo.exists(zTableMapPath)) { + throw new KeeperException.NoNodeException(zTableMapPath + " does not exist in ZooKeeper"); + } + if (isBuiltInZKTable(tableId)) { + throw new AssertionError("Putting built-in tables in map should not be possible after init"); + } + zoo.mutateExisting(zTableMapPath, data -> { + var tables = deserializeMap(data); + final String currentName = tables.get(tableId.canonical()); + if (tableName.equals(currentName)) { + return null; // mapping already exists; operation is idempotent, so no change needed + } + if (currentName != null) { + throw new AcceptableThriftTableOperationException(null, tableId.canonical(), operation, + TableOperationExceptionType.EXISTS, "Table Id already exists"); + } + if (tables.containsValue(tableName)) { + throw new AcceptableThriftTableOperationException(null, tableId.canonical(), operation, + TableOperationExceptionType.EXISTS, "Table name already exists"); + } + tables.put(tableId.canonical(), tableName); + return serializeMap(tables); + }); + } + + public void remove(final ClientContext context, final TableId tableId) + throws InterruptedException, KeeperException, AcceptableThriftTableOperationException { + var zoo = context.getZooSession().asReaderWriter(); + Stream.of(zoo, tableId).forEach(Objects::requireNonNull); + if (isBuiltInZKTable(tableId)) { + throw new AssertionError("Removing built-in tables in map should not be possible"); + } + zoo.mutateExisting(getZTableMapPath(getNamespaceOfTableId(zoo, tableId)), data -> { + var tables = deserializeMap(data); + if (!tables.containsKey(tableId.canonical())) { + throw new AcceptableThriftTableOperationException(null, tableId.canonical(), Review Comment: Why throw an exceptoin if its not there? Seems like if this code runs a 2nd time for some reason that maybe the first run could have removed it and maybe it should not throw an exception. ########## core/src/main/java/org/apache/accumulo/core/util/tables/TableMapping.java: ########## @@ -0,0 +1,201 @@ +/* + * 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.core.util.tables; + +import static java.util.Collections.emptySortedMap; +import static java.util.Objects.requireNonNull; +import static org.apache.accumulo.core.clientImpl.NamespaceMapping.deserializeMap; +import static org.apache.accumulo.core.clientImpl.NamespaceMapping.serializeMap; + +import java.util.Map; +import java.util.Objects; +import java.util.SortedMap; +import java.util.stream.Stream; + +import org.apache.accumulo.core.Constants; +import org.apache.accumulo.core.clientImpl.AcceptableThriftTableOperationException; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.clientImpl.Namespace; +import org.apache.accumulo.core.clientImpl.thrift.TableOperation; +import org.apache.accumulo.core.clientImpl.thrift.TableOperationExceptionType; +import org.apache.accumulo.core.data.NamespaceId; +import org.apache.accumulo.core.data.TableId; +import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.core.metadata.AccumuloTable; +import org.apache.accumulo.core.zookeeper.ZcStat; +import org.apache.accumulo.core.zookeeper.ZooCache; +import org.apache.zookeeper.KeeperException; + +import com.google.common.collect.ImmutableSortedMap; + +public class TableMapping { + + private final ClientContext context; + private final NamespaceId namespaceId; + private volatile SortedMap<TableId,String> currentTableMap = emptySortedMap(); + private volatile SortedMap<String,TableId> currentTableReverseMap = emptySortedMap(); + private volatile long lastMzxid; + + public TableMapping(ClientContext context, NamespaceId namespaceId) { + this.context = context; + this.namespaceId = namespaceId; + } + + public void put(final ClientContext context, TableId tableId, String tableName, + TableOperation operation) + throws InterruptedException, KeeperException, AcceptableThriftTableOperationException { + var zoo = context.getZooSession().asReaderWriter(); + Stream.of(zoo, tableId, namespaceId, tableName).forEach(Objects::requireNonNull); + String zTableMapPath = getZTableMapPath(namespaceId); + if (!zoo.exists(zTableMapPath)) { + throw new KeeperException.NoNodeException(zTableMapPath + " does not exist in ZooKeeper"); + } Review Comment: Wonder if this code could be removed because zoo.mutateExisting may throw this exception if the node does not exists. If that is true then it would be nice to avoid this RPC. Also things could change between the all the exists and mutateExisting, so its better handle everything there if possible. ```suggestion ``` ########## core/src/main/java/org/apache/accumulo/core/util/tables/TableMapping.java: ########## @@ -0,0 +1,201 @@ +/* + * 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.core.util.tables; + +import static java.util.Collections.emptySortedMap; +import static java.util.Objects.requireNonNull; +import static org.apache.accumulo.core.clientImpl.NamespaceMapping.deserializeMap; +import static org.apache.accumulo.core.clientImpl.NamespaceMapping.serializeMap; + +import java.util.Map; +import java.util.Objects; +import java.util.SortedMap; +import java.util.stream.Stream; + +import org.apache.accumulo.core.Constants; +import org.apache.accumulo.core.clientImpl.AcceptableThriftTableOperationException; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.clientImpl.Namespace; +import org.apache.accumulo.core.clientImpl.thrift.TableOperation; +import org.apache.accumulo.core.clientImpl.thrift.TableOperationExceptionType; +import org.apache.accumulo.core.data.NamespaceId; +import org.apache.accumulo.core.data.TableId; +import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.core.metadata.AccumuloTable; +import org.apache.accumulo.core.zookeeper.ZcStat; +import org.apache.accumulo.core.zookeeper.ZooCache; +import org.apache.zookeeper.KeeperException; + +import com.google.common.collect.ImmutableSortedMap; + +public class TableMapping { + + private final ClientContext context; + private final NamespaceId namespaceId; + private volatile SortedMap<TableId,String> currentTableMap = emptySortedMap(); + private volatile SortedMap<String,TableId> currentTableReverseMap = emptySortedMap(); + private volatile long lastMzxid; + + public TableMapping(ClientContext context, NamespaceId namespaceId) { + this.context = context; + this.namespaceId = namespaceId; + } + + public void put(final ClientContext context, TableId tableId, String tableName, + TableOperation operation) + throws InterruptedException, KeeperException, AcceptableThriftTableOperationException { + var zoo = context.getZooSession().asReaderWriter(); + Stream.of(zoo, tableId, namespaceId, tableName).forEach(Objects::requireNonNull); + String zTableMapPath = getZTableMapPath(namespaceId); + if (!zoo.exists(zTableMapPath)) { + throw new KeeperException.NoNodeException(zTableMapPath + " does not exist in ZooKeeper"); + } + if (isBuiltInZKTable(tableId)) { + throw new AssertionError("Putting built-in tables in map should not be possible after init"); + } + zoo.mutateExisting(zTableMapPath, data -> { + var tables = deserializeMap(data); + final String currentName = tables.get(tableId.canonical()); + if (tableName.equals(currentName)) { + return null; // mapping already exists; operation is idempotent, so no change needed + } + if (currentName != null) { + throw new AcceptableThriftTableOperationException(null, tableId.canonical(), operation, + TableOperationExceptionType.EXISTS, "Table Id already exists"); + } + if (tables.containsValue(tableName)) { + throw new AcceptableThriftTableOperationException(null, tableId.canonical(), operation, + TableOperationExceptionType.EXISTS, "Table name already exists"); + } + tables.put(tableId.canonical(), tableName); + return serializeMap(tables); + }); + } + + public void remove(final ClientContext context, final TableId tableId) + throws InterruptedException, KeeperException, AcceptableThriftTableOperationException { + var zoo = context.getZooSession().asReaderWriter(); + Stream.of(zoo, tableId).forEach(Objects::requireNonNull); + if (isBuiltInZKTable(tableId)) { + throw new AssertionError("Removing built-in tables in map should not be possible"); + } + zoo.mutateExisting(getZTableMapPath(getNamespaceOfTableId(zoo, tableId)), data -> { + var tables = deserializeMap(data); + if (!tables.containsKey(tableId.canonical())) { + throw new AcceptableThriftTableOperationException(null, tableId.canonical(), + TableOperation.DELETE, TableOperationExceptionType.NOTFOUND, + "Table already removed while processing"); + } + tables.remove(tableId.canonical()); + return serializeMap(tables); + }); + } + + public void rename(final ClientContext context, final TableId tableId, final String oldName, + final String newName) + throws InterruptedException, KeeperException, AcceptableThriftTableOperationException { + var zoo = context.getZooSession().asReaderWriter(); + Stream.of(zoo, tableId, namespaceId, oldName, newName).forEach(Objects::requireNonNull); + String zTableMapPath = getZTableMapPath(namespaceId); + if (!zoo.exists(zTableMapPath)) { + throw new KeeperException.NoNodeException(zTableMapPath + " does not exist in ZooKeeper"); + } + if (isBuiltInZKTable(tableId)) { + throw new AssertionError("Renaming built-in tables in map should not be possible"); + } + zoo.mutateExisting(zTableMapPath, current -> { + var tables = deserializeMap(current); + final String currentName = tables.get(tableId.canonical()); + if (newName.equals(currentName)) { + return null; // assume in this case the operation is running again, so we are done + } + if (!oldName.equals(currentName)) { + throw new AcceptableThriftTableOperationException(null, oldName, TableOperation.RENAME, + TableOperationExceptionType.NOTFOUND, "Name changed while processing"); + } + if (tables.containsValue(newName)) { + throw new AcceptableThriftTableOperationException(null, newName, TableOperation.RENAME, + TableOperationExceptionType.EXISTS, "Table name already exists"); + } + tables.put(namespaceId.canonical(), newName); + return serializeMap(tables); + }); + } + + private synchronized void update(NamespaceId namespaceId) { + final ZooCache zc = context.getZooCache(); + final ZcStat stat = new ZcStat(); + final String zTableMapPath = getZTableMapPath(namespaceId); + + byte[] data = zc.get(zTableMapPath, stat); + if (stat.getMzxid() > lastMzxid) { + if (data == null) { + throw new IllegalStateException(zTableMapPath + " node should not be null"); Review Comment: When a namespace does not exist in zookeeper, is this the best exception to throw? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
