keith-turner commented on code in PR #5451: URL: https://github.com/apache/accumulo/pull/5451#discussion_r2031679006
########## 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 looping over the map be avoided by doing the following? ```suggestion if(context.getTableMapping(namespaceId).getIdToNameMap().containsKey(tableId)){ return true; } ``` -- 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