From 69aad541b321dd0f914a775c022aae6ac12ecddd Mon Sep 17 00:00:00 2001 From: Tsz-Wo Nicholas Sze Date: Thu, 24 Apr 2025 13:08:45 -0700 Subject: [PATCH 1/2] HDDS-12900. Use OMDBDefinition in OmMetadataManagerImpl. --- .../utils/db/DBColumnFamilyDefinition.java | 28 ++- .../apache/hadoop/hdds/utils/db/DBStore.java | 6 + .../ha/TestInterSCMGrpcProtocolService.java | 9 +- .../ozone/om/OmMetadataManagerImpl.java | 234 +++++++----------- .../hadoop/ozone/om/codec/OMDBDefinition.java | 4 +- .../ozone/om/TestOmMetadataManager.java | 55 +++- .../ozone/om/TestOmSnapshotManager.java | 17 +- 7 files changed, 179 insertions(+), 174 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBColumnFamilyDefinition.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBColumnFamilyDefinition.java index 89326bb4223a..3d0c2fe196c1 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBColumnFamilyDefinition.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBColumnFamilyDefinition.java @@ -17,12 +17,16 @@ package org.apache.hadoop.hdds.utils.db; +import static org.apache.ratis.util.JavaUtils.getClassSimpleName; + import java.io.IOException; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import org.apache.hadoop.hdds.utils.CollectionUtils; +import org.apache.hadoop.hdds.utils.db.cache.TableCache.CacheType; import org.apache.hadoop.hdds.utils.db.managed.ManagedColumnFamilyOptions; /** @@ -34,17 +38,18 @@ public class DBColumnFamilyDefinition { private final String tableName; - private final Codec keyCodec; - private final Codec valueCodec; + private final String name; private volatile ManagedColumnFamilyOptions cfOptions; public DBColumnFamilyDefinition(String tableName, Codec keyCodec, Codec valueCodec) { - this.tableName = tableName; - this.keyCodec = keyCodec; - this.valueCodec = valueCodec; + this.tableName = Objects.requireNonNull(tableName, "tableName == null"); + this.keyCodec = Objects.requireNonNull(keyCodec, "keyCodec == null"); + this.valueCodec = Objects.requireNonNull(valueCodec, "valueCodec == null"); + this.name = tableName + "-def: " + getClassSimpleName(getKeyType()) + + " -> " + getClassSimpleName(getValueType()); this.cfOptions = null; } @@ -67,8 +72,12 @@ public DBColumnFamilyDefinition(String tableName, Codec keyCodec, Codec getTable(DBStore db) throws IOException { - return db.getTable(tableName, getKeyType(), getValueType()); + public TypedTable getTable(DBStore db) throws IOException { + return db.getTable(tableName, keyCodec, valueCodec); + } + + public TypedTable getTable(DBStore db, CacheType cacheType) throws IOException { + return db.getTable(tableName, keyCodec, valueCodec, cacheType); } public String getName() { @@ -98,4 +107,9 @@ public ManagedColumnFamilyOptions getCfOptions() { public void setCfOptions(ManagedColumnFamilyOptions cfOptions) { this.cfOptions = cfOptions; } + + @Override + public String toString() { + return name; + } } diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStore.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStore.java index 4d83acba39ef..eae3148ae04b 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStore.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStore.java @@ -24,6 +24,7 @@ import java.util.Map; import org.apache.hadoop.hdds.annotation.InterfaceStability; import org.apache.hadoop.hdds.utils.db.cache.TableCache; +import org.apache.hadoop.hdds.utils.db.cache.TableCache.CacheType; import org.apache.ozone.rocksdiff.RocksDBCheckpointDiffer; /** @@ -45,6 +46,11 @@ public interface DBStore extends Closeable, BatchOperationHandler { */ Table getTable(String name) throws IOException; + /** The same as getTable(name, keyCodec, valueCodec, CacheType.PARTIAL_CACHE). */ + default TypedTable getTable(String name, Codec keyCodec, Codec valueCodec) + throws IOException { + return getTable(name, keyCodec, valueCodec, CacheType.PARTIAL_CACHE); + } /** * Gets an existing TableStore with implicit key/value conversion and diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestInterSCMGrpcProtocolService.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestInterSCMGrpcProtocolService.java index 92416ca22f36..8697521f0752 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestInterSCMGrpcProtocolService.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestInterSCMGrpcProtocolService.java @@ -53,9 +53,10 @@ import org.apache.hadoop.hdds.security.x509.CertificateTestUtils; import org.apache.hadoop.hdds.security.x509.certificate.client.SCMCertificateClient; import org.apache.hadoop.hdds.utils.TransactionInfo; +import org.apache.hadoop.hdds.utils.db.Codec; import org.apache.hadoop.hdds.utils.db.DBCheckpoint; import org.apache.hadoop.hdds.utils.db.DBStore; -import org.apache.hadoop.hdds.utils.db.Table; +import org.apache.hadoop.hdds.utils.db.TypedTable; import org.apache.hadoop.ozone.OzoneConfigKeys; import org.apache.ozone.test.GenericTestUtils.PortAllocator; import org.junit.jupiter.api.Test; @@ -172,7 +173,7 @@ private SCMMetadataStore metadataStore() throws IOException { private DBStore dbStore() throws IOException { DBStore dbStoreMock = mock(DBStore.class); - doReturn(trInfoTable()).when(dbStoreMock).getTable(any(), any(), any()); + doReturn(trInfoTable()).when(dbStoreMock).getTable(any(), (Codec) any(), any()); doReturn(checkPoint()).when(dbStoreMock).getCheckpoint(anyBoolean()); return dbStoreMock; } @@ -186,9 +187,9 @@ private DBCheckpoint checkPoint() throws IOException { return checkpoint; } - private Table trInfoTable() + private TypedTable trInfoTable() throws IOException { - Table tableMock = mock(Table.class); + final TypedTable tableMock = mock(TypedTable.class); doReturn(mock(TransactionInfo.class)).when(tableMock).get(any()); return tableMock; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java index 01d187fffd69..0ff82b2c5824 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java @@ -69,6 +69,7 @@ import org.apache.hadoop.hdds.utils.TransactionInfo; import org.apache.hadoop.hdds.utils.db.BatchOperation; import org.apache.hadoop.hdds.utils.db.DBCheckpoint; +import org.apache.hadoop.hdds.utils.db.DBColumnFamilyDefinition; import org.apache.hadoop.hdds.utils.db.DBStore; import org.apache.hadoop.hdds.utils.db.DBStoreBuilder; import org.apache.hadoop.hdds.utils.db.RDBCheckpointUtils; @@ -76,6 +77,7 @@ import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.Table.KeyValue; import org.apache.hadoop.hdds.utils.db.TableIterator; +import org.apache.hadoop.hdds.utils.db.TypedTable; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.hdds.utils.db.cache.TableCache.CacheType; @@ -83,6 +85,7 @@ import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.common.BlockGroup; +import org.apache.hadoop.ozone.om.codec.OMDBDefinition; import org.apache.hadoop.ozone.om.codec.TokenIdentifierCodec; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes; @@ -257,61 +260,73 @@ public class OmMetadataManagerImpl implements OMMetadataManager, public static final String COMPACTION_LOG_TABLE = "compactionLogTable"; - static final String[] ALL_TABLES = new String[] { - USER_TABLE, - VOLUME_TABLE, - BUCKET_TABLE, - KEY_TABLE, - DELETED_TABLE, - OPEN_KEY_TABLE, - MULTIPARTINFO_TABLE, - S3_SECRET_TABLE, - DELEGATION_TOKEN_TABLE, - PREFIX_TABLE, - TRANSACTION_INFO_TABLE, - DIRECTORY_TABLE, - FILE_TABLE, - DELETED_DIR_TABLE, - OPEN_FILE_TABLE, - META_TABLE, - TENANT_ACCESS_ID_TABLE, - PRINCIPAL_TO_ACCESS_IDS_TABLE, - TENANT_STATE_TABLE, - SNAPSHOT_INFO_TABLE, - SNAPSHOT_RENAMED_TABLE, - COMPACTION_LOG_TABLE - }; + private class TableInitializer { + private final boolean addCacheMetrics; + + TableInitializer(boolean addCacheMetrics) { + this.addCacheMetrics = addCacheMetrics; + } + + TypedTable get(DBColumnFamilyDefinition definition) + throws IOException { + return get(definition.getTable(store)); + } + + TypedTable get(DBColumnFamilyDefinition definition, CacheType cacheType) + throws IOException { + return get(definition.getTable(store, cacheType)); + } + + private TypedTable get(TypedTable table) { + Objects.requireNonNull(table, "table == null"); + final String name = table.getName(); + final Table previousTable = tableMap.put(name, table); + if (previousTable != null) { + LOG.warn("Overwriting an existing table {}: {}", name, previousTable); + } + + if (addCacheMetrics) { + final TableCacheMetrics previous = tableCacheMetricsMap.put(name, table.createCacheMetrics()); + if (previous != null) { + previous.unregister(); + } + } + return table; + } + } private DBStore store; private final IOzoneManagerLock lock; - private Table userTable; - private Table volumeTable; - private Table bucketTable; - private Table keyTable; - private Table deletedTable; - private Table openKeyTable; - private Table multipartInfoTable; - private Table s3SecretTable; - private Table dTokenTable; - private Table prefixTable; - private Table dirTable; - private Table fileTable; - private Table openFileTable; - private Table transactionInfoTable; - private Table metaTable; + private TypedTable userTable; + private TypedTable volumeTable; + private TypedTable bucketTable; + private TypedTable keyTable; - // Tables required for multi-tenancy - private Table tenantAccessIdTable; - private Table principalToAccessIdsTable; - private Table tenantStateTable; + private TypedTable openKeyTable; + private TypedTable multipartInfoTable; + private TypedTable deletedTable; + + private TypedTable dirTable; + private TypedTable fileTable; + private TypedTable openFileTable; + private TypedTable deletedDirTable; - private Table snapshotInfoTable; - private Table snapshotRenamedTable; - private Table compactionLogTable; + private TypedTable s3SecretTable; + private TypedTable dTokenTable; + private TypedTable prefixTable; + private TypedTable transactionInfoTable; + private TypedTable metaTable; - private Table deletedDirTable; + // Tables required for multi-tenancy + private TypedTable tenantAccessIdTable; + private TypedTable principalToAccessIdsTable; + private TypedTable tenantStateTable; + + private TypedTable snapshotInfoTable; + private TypedTable snapshotRenamedTable; + private TypedTable compactionLogTable; private OzoneManager ozoneManager; @@ -506,25 +521,6 @@ public Table getMultipartInfoTable() { return multipartInfoTable; } - private void checkTableStatus(Table table, String name, - boolean addCacheMetrics) throws IOException { - String logMessage = "Unable to get a reference to %s table. Cannot " + - "continue."; - String errMsg = "Inconsistent DB state, Table - %s. Please check the logs" + - "for more info."; - if (table == null) { - LOG.error(String.format(logMessage, name)); - throw new IOException(String.format(errMsg, name)); - } - this.tableMap.put(name, table); - if (addCacheMetrics) { - if (tableCacheMetricsMap.containsKey(name)) { - tableCacheMetricsMap.get(name).unregister(); - } - tableCacheMetricsMap.put(name, table.createCacheMetrics()); - } - } - /** * Start metadata manager. */ @@ -648,110 +644,50 @@ public static DBStoreBuilder addOMTablesAndCodecs(DBStoreBuilder builder) { protected void initializeOmTables(CacheType cacheType, boolean addCacheMetrics) throws IOException { - userTable = - this.store.getTable(USER_TABLE, String.class, - PersistedUserVolumeInfo.class); - checkTableStatus(userTable, USER_TABLE, addCacheMetrics); - - volumeTable = - this.store.getTable(VOLUME_TABLE, String.class, OmVolumeArgs.class, - cacheType); - checkTableStatus(volumeTable, VOLUME_TABLE, addCacheMetrics); - - bucketTable = - this.store.getTable(BUCKET_TABLE, String.class, OmBucketInfo.class, - cacheType); - - checkTableStatus(bucketTable, BUCKET_TABLE, addCacheMetrics); - - keyTable = this.store.getTable(KEY_TABLE, String.class, OmKeyInfo.class); - checkTableStatus(keyTable, KEY_TABLE, addCacheMetrics); - - deletedTable = this.store.getTable(DELETED_TABLE, String.class, - RepeatedOmKeyInfo.class); - checkTableStatus(deletedTable, DELETED_TABLE, addCacheMetrics); - - openKeyTable = - this.store.getTable(OPEN_KEY_TABLE, String.class, - OmKeyInfo.class); - checkTableStatus(openKeyTable, OPEN_KEY_TABLE, addCacheMetrics); - - multipartInfoTable = this.store.getTable(MULTIPARTINFO_TABLE, - String.class, OmMultipartKeyInfo.class); - checkTableStatus(multipartInfoTable, MULTIPARTINFO_TABLE, addCacheMetrics); - - dTokenTable = this.store.getTable(DELEGATION_TOKEN_TABLE, - OzoneTokenIdentifier.class, Long.class); - checkTableStatus(dTokenTable, DELEGATION_TOKEN_TABLE, addCacheMetrics); - - s3SecretTable = this.store.getTable(S3_SECRET_TABLE, String.class, - S3SecretValue.class); - checkTableStatus(s3SecretTable, S3_SECRET_TABLE, addCacheMetrics); + final TableInitializer initializer = new TableInitializer(addCacheMetrics); - prefixTable = this.store.getTable(PREFIX_TABLE, String.class, - OmPrefixInfo.class); - checkTableStatus(prefixTable, PREFIX_TABLE, addCacheMetrics); + userTable = initializer.get(OMDBDefinition.USER_TABLE); - dirTable = this.store.getTable(DIRECTORY_TABLE, String.class, - OmDirectoryInfo.class); - checkTableStatus(dirTable, DIRECTORY_TABLE, addCacheMetrics); + volumeTable = initializer.get(OMDBDefinition.VOLUME_TABLE, cacheType); + bucketTable = initializer.get(OMDBDefinition.BUCKET_TABLE, cacheType); + keyTable = initializer.get(OMDBDefinition.KEY_TABLE); - fileTable = this.store.getTable(FILE_TABLE, String.class, - OmKeyInfo.class); - checkTableStatus(fileTable, FILE_TABLE, addCacheMetrics); + openKeyTable = initializer.get(OMDBDefinition.OPEN_KEY_TABLE); + multipartInfoTable = initializer.get(OMDBDefinition.MULTIPART_INFO_TABLE); + deletedTable = initializer.get(OMDBDefinition.DELETED_TABLE); - openFileTable = this.store.getTable(OPEN_FILE_TABLE, String.class, - OmKeyInfo.class); - checkTableStatus(openFileTable, OPEN_FILE_TABLE, addCacheMetrics); + dirTable = initializer.get(OMDBDefinition.DIRECTORY_TABLE); + fileTable = initializer.get(OMDBDefinition.FILE_TABLE); + openFileTable = initializer.get(OMDBDefinition.OPEN_FILE_TABLE); + deletedDirTable = initializer.get(OMDBDefinition.DELETED_DIR_TABLE); - deletedDirTable = this.store.getTable(DELETED_DIR_TABLE, String.class, - OmKeyInfo.class); - checkTableStatus(deletedDirTable, DELETED_DIR_TABLE, addCacheMetrics); + dTokenTable = initializer.get(OMDBDefinition.DELEGATION_TOKEN_TABLE); + s3SecretTable = initializer.get(OMDBDefinition.S3_SECRET_TABLE); + prefixTable = initializer.get(OMDBDefinition.PREFIX_TABLE); - transactionInfoTable = this.store.getTable(TRANSACTION_INFO_TABLE, - String.class, TransactionInfo.class); - checkTableStatus(transactionInfoTable, TRANSACTION_INFO_TABLE, - addCacheMetrics); + transactionInfoTable = initializer.get(OMDBDefinition.TRANSACTION_INFO_TABLE); - metaTable = this.store.getTable(META_TABLE, String.class, String.class); - checkTableStatus(metaTable, META_TABLE, addCacheMetrics); + metaTable = initializer.get(OMDBDefinition.META_TABLE); // accessId -> OmDBAccessIdInfo (tenantId, secret, Kerberos principal) - tenantAccessIdTable = this.store.getTable(TENANT_ACCESS_ID_TABLE, - String.class, OmDBAccessIdInfo.class); - checkTableStatus(tenantAccessIdTable, TENANT_ACCESS_ID_TABLE, - addCacheMetrics); + tenantAccessIdTable = initializer.get(OMDBDefinition.TENANT_ACCESS_ID_TABLE); // User principal -> OmDBUserPrincipalInfo (a list of accessIds) - principalToAccessIdsTable = this.store.getTable( - PRINCIPAL_TO_ACCESS_IDS_TABLE, - String.class, OmDBUserPrincipalInfo.class); - checkTableStatus(principalToAccessIdsTable, PRINCIPAL_TO_ACCESS_IDS_TABLE, - addCacheMetrics); + principalToAccessIdsTable = initializer.get(OMDBDefinition.PRINCIPAL_TO_ACCESS_IDS_TABLE); // tenant name -> tenant (tenant states) - tenantStateTable = this.store.getTable(TENANT_STATE_TABLE, - String.class, OmDBTenantState.class); - checkTableStatus(tenantStateTable, TENANT_STATE_TABLE, addCacheMetrics); + tenantStateTable = initializer.get(OMDBDefinition.TENANT_STATE_TABLE); // TODO: [SNAPSHOT] Consider FULL_CACHE for snapshotInfoTable since // exclusiveSize in SnapshotInfo can be frequently updated. // path -> snapshotInfo (snapshot info for snapshot) - snapshotInfoTable = this.store.getTable(SNAPSHOT_INFO_TABLE, - String.class, SnapshotInfo.class); - checkTableStatus(snapshotInfoTable, SNAPSHOT_INFO_TABLE, addCacheMetrics); + snapshotInfoTable = initializer.get(OMDBDefinition.SNAPSHOT_INFO_TABLE); // volumeName/bucketName/objectID -> renamedKey or renamedDir - snapshotRenamedTable = this.store.getTable(SNAPSHOT_RENAMED_TABLE, - String.class, String.class); - checkTableStatus(snapshotRenamedTable, SNAPSHOT_RENAMED_TABLE, - addCacheMetrics); + snapshotRenamedTable = initializer.get(OMDBDefinition.SNAPSHOT_RENAMED_TABLE); // TODO: [SNAPSHOT] Initialize table lock for snapshotRenamedTable. - compactionLogTable = this.store.getTable(COMPACTION_LOG_TABLE, - String.class, CompactionLogEntry.class); - checkTableStatus(compactionLogTable, COMPACTION_LOG_TABLE, - addCacheMetrics); + compactionLogTable = initializer.get(OMDBDefinition.COMPACTION_LOG_TABLE); } /** diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java index a529aa38e40e..b0a6f79b9489 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java @@ -105,7 +105,7 @@ public final class OMDBDefinition extends DBDefinition.WithMap { OmPrefixInfo.getCodec()); public static final DBColumnFamilyDefinition - DTOKEN_TABLE = + DELEGATION_TOKEN_TABLE = new DBColumnFamilyDefinition<>( OmMetadataManagerImpl.DELEGATION_TOKEN_TABLE, TokenIdentifierCodec.get(), @@ -228,7 +228,7 @@ public final class OMDBDefinition extends DBDefinition.WithMap { DELETED_DIR_TABLE, DELETED_TABLE, DIRECTORY_TABLE, - DTOKEN_TABLE, + DELEGATION_TOKEN_TABLE, FILE_TABLE, KEY_TABLE, META_TABLE, diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java index 0f7401fb8728..c2e8ee894d13 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java @@ -24,6 +24,28 @@ import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_MPU_EXPIRE_THRESHOLD_DEFAULT; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_OPEN_KEY_EXPIRE_THRESHOLD; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_OPEN_KEY_EXPIRE_THRESHOLD_DEFAULT; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.BUCKET_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.COMPACTION_LOG_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.DELEGATION_TOKEN_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.DELETED_DIR_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.DELETED_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.DIRECTORY_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.FILE_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.KEY_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.META_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.MULTIPARTINFO_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.OPEN_FILE_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.OPEN_KEY_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.PREFIX_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.PRINCIPAL_TO_ACCESS_IDS_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.S3_SECRET_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.SNAPSHOT_INFO_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.SNAPSHOT_RENAMED_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.TENANT_ACCESS_ID_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.TENANT_STATE_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.TRANSACTION_INFO_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.USER_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.VOLUME_TABLE; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.BUCKET_NOT_FOUND; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_NOT_FOUND; import static org.assertj.core.api.Assertions.assertThat; @@ -57,6 +79,7 @@ import org.apache.hadoop.hdds.utils.TransactionInfo; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; +import org.apache.hadoop.ozone.om.codec.OMDBDefinition; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes; import org.apache.hadoop.ozone.om.helpers.BucketLayout; @@ -954,12 +977,38 @@ private void addKeysToOM(String volumeName, String bucketName, } } + static final String[] ALL_TABLES = new String[] { + USER_TABLE, + VOLUME_TABLE, + BUCKET_TABLE, + KEY_TABLE, + DELETED_TABLE, + OPEN_KEY_TABLE, + MULTIPARTINFO_TABLE, + S3_SECRET_TABLE, + DELEGATION_TOKEN_TABLE, + PREFIX_TABLE, + TRANSACTION_INFO_TABLE, + DIRECTORY_TABLE, + FILE_TABLE, + DELETED_DIR_TABLE, + OPEN_FILE_TABLE, + META_TABLE, + TENANT_ACCESS_ID_TABLE, + PRINCIPAL_TO_ACCESS_IDS_TABLE, + TENANT_STATE_TABLE, + SNAPSHOT_INFO_TABLE, + SNAPSHOT_RENAMED_TABLE, + COMPACTION_LOG_TABLE + }; + @Test public void testAllTablesAreProperInOMMetadataManagerImpl() { - Set tablesByDefinition = - new HashSet<>(Arrays.asList(OmMetadataManagerImpl.ALL_TABLES)); - Set tablesInManager = omMetadataManager.listTableNames(); + final Set expected = new HashSet<>(Arrays.asList(ALL_TABLES)); + final Set tablesByDefinition = new HashSet<>(OMDBDefinition.get().getColumnFamilyNames()); + assertEquals(expected, tablesByDefinition); + final Set tablesInManager = omMetadataManager.listTableNames(); assertEquals(tablesByDefinition, tablesInManager); } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java index 7cb1e2e38ab8..0c061ba2aac9 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java @@ -66,7 +66,7 @@ import org.apache.hadoop.hdds.utils.db.DBStore; import org.apache.hadoop.hdds.utils.db.RDBBatchOperation; import org.apache.hadoop.hdds.utils.db.RDBStore; -import org.apache.hadoop.hdds.utils.db.Table; +import org.apache.hadoop.hdds.utils.db.TypedTable; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; @@ -128,8 +128,7 @@ void stop() { public void testSnapshotFeatureFlagSafetyCheck() throws IOException { // Verify that the snapshot feature config safety check method // is returning the expected value. - - Table snapshotInfoTable = mock(Table.class); + final TypedTable snapshotInfoTable = mock(TypedTable.class); HddsWhiteboxTestUtils.setInternalState( om.getMetadataManager(), SNAPSHOT_INFO_TABLE, snapshotInfoTable); @@ -147,9 +146,9 @@ public void testCloseOnEviction() throws IOException, GenericTestUtils.setLogLevel(RDBStore.class, Level.DEBUG); LogCapturer logCapture = LogCapturer.captureLogs(RDBStore.class); // set up db tables - Table volumeTable = mock(Table.class); - Table bucketTable = mock(Table.class); - Table snapshotInfoTable = mock(Table.class); + final TypedTable volumeTable = mock(TypedTable.class); + final TypedTable bucketTable = mock(TypedTable.class); + final TypedTable snapshotInfoTable = mock(TypedTable.class); HddsWhiteboxTestUtils.setInternalState( om.getMetadataManager(), VOLUME_TABLE, volumeTable); HddsWhiteboxTestUtils.setInternalState( @@ -655,9 +654,9 @@ void testProcessFileWithDestDirParameter(@TempDir File testDir) throws IOExcepti public void testCreateSnapshotIdempotent() throws Exception { // set up db tables LogCapturer logCapturer = LogCapturer.captureLogs(OmSnapshotManager.class); - Table volumeTable = mock(Table.class); - Table bucketTable = mock(Table.class); - Table snapshotInfoTable = mock(Table.class); + final TypedTable volumeTable = mock(TypedTable.class); + final TypedTable bucketTable = mock(TypedTable.class); + final TypedTable snapshotInfoTable = mock(TypedTable.class); HddsWhiteboxTestUtils.setInternalState( om.getMetadataManager(), VOLUME_TABLE, volumeTable); HddsWhiteboxTestUtils.setInternalState( From 9b1c8735cfc487ed2d01229db6658e18a8c818d2 Mon Sep 17 00:00:00 2001 From: Tsz-Wo Nicholas Sze Date: Sun, 27 Apr 2025 14:35:33 -0700 Subject: [PATCH 2/2] Move TableInitializer to the end of the file. --- .../ozone/om/OmMetadataManagerImpl.java | 70 +++++++++---------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java index 0ff82b2c5824..219fa080c6c5 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java @@ -260,41 +260,6 @@ public class OmMetadataManagerImpl implements OMMetadataManager, public static final String COMPACTION_LOG_TABLE = "compactionLogTable"; - private class TableInitializer { - private final boolean addCacheMetrics; - - TableInitializer(boolean addCacheMetrics) { - this.addCacheMetrics = addCacheMetrics; - } - - TypedTable get(DBColumnFamilyDefinition definition) - throws IOException { - return get(definition.getTable(store)); - } - - TypedTable get(DBColumnFamilyDefinition definition, CacheType cacheType) - throws IOException { - return get(definition.getTable(store, cacheType)); - } - - private TypedTable get(TypedTable table) { - Objects.requireNonNull(table, "table == null"); - final String name = table.getName(); - final Table previousTable = tableMap.put(name, table); - if (previousTable != null) { - LOG.warn("Overwriting an existing table {}: {}", name, previousTable); - } - - if (addCacheMetrics) { - final TableCacheMetrics previous = tableCacheMetricsMap.put(name, table.createCacheMetrics()); - if (previous != null) { - previous.unregister(); - } - } - return table; - } - } - private DBStore store; private final IOzoneManagerLock lock; @@ -2186,4 +2151,39 @@ public void deleteWithBatch(AutoCloseable batchOperator, String id) } } } + + private class TableInitializer { + private final boolean addCacheMetrics; + + TableInitializer(boolean addCacheMetrics) { + this.addCacheMetrics = addCacheMetrics; + } + + TypedTable get(DBColumnFamilyDefinition definition) + throws IOException { + return get(definition.getTable(store)); + } + + TypedTable get(DBColumnFamilyDefinition definition, CacheType cacheType) + throws IOException { + return get(definition.getTable(store, cacheType)); + } + + private TypedTable get(TypedTable table) { + Objects.requireNonNull(table, "table == null"); + final String name = table.getName(); + final Table previousTable = tableMap.put(name, table); + if (previousTable != null) { + LOG.warn("Overwriting an existing table {}: {}", name, previousTable); + } + + if (addCacheMetrics) { + final TableCacheMetrics previous = tableCacheMetricsMap.put(name, table.createCacheMetrics()); + if (previous != null) { + previous.unregister(); + } + } + return table; + } + } }