This is an automated email from the ASF dual-hosted git repository.
erose pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new d1cdf9994e HDDS-7042. Rebuilding tenant cache omits empty tenants
(#3618)
d1cdf9994e is described below
commit d1cdf9994ea99f006ea32ff429d31bcfb6fa74dc
Author: Ethan Rose <[email protected]>
AuthorDate: Mon Jul 25 14:07:11 2022 -0700
HDDS-7042. Rebuilding tenant cache omits empty tenants (#3618)
---
.../hadoop/ozone/om/OMMultiTenantManagerImpl.java | 49 ++++----
.../ozone/om/multitenant/CachedTenantState.java | 41 +++++++
.../ozone/om/TestOMMultiTenantManagerImpl.java | 126 +++++++++++++++++----
3 files changed, 174 insertions(+), 42 deletions(-)
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMultiTenantManagerImpl.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMultiTenantManagerImpl.java
index e8f35f6786..f507734f00 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMultiTenantManagerImpl.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMultiTenantManagerImpl.java
@@ -875,17 +875,34 @@ public class OMMultiTenantManagerImpl implements
OMMultiTenantManager {
return conf;
}
- public void loadTenantCacheFromDB() {
- final Table<String, OmDBAccessIdInfo> tenantAccessIdTable =
- omMetadataManager.getTenantAccessIdTable();
- final TableIterator<String, ? extends KeyValue<String, OmDBAccessIdInfo>>
- accessIdTableIter = tenantAccessIdTable.iterator();
- int userCount = 0;
-
+ private void loadTenantCacheFromDB() {
+ // First load each tenant as a key into the cache.
final Table<String, OmDBTenantState> tenantStateTable =
omMetadataManager.getTenantStateTable();
+ try (TableIterator<String, ? extends KeyValue<String, OmDBTenantState>>
+ tenantStateTableIter = tenantStateTable.iterator()) {
+ while (tenantStateTableIter.hasNext()) {
+ final KeyValue<String, OmDBTenantState> next =
+ tenantStateTableIter.next();
- try {
+ final String tenantId = next.getKey();
+ final OmDBTenantState tenantState = next.getValue();
+
+ tenantCache.put(tenantId, new CachedTenantState(tenantId,
+ tenantState.getUserRoleName(), tenantState.getAdminRoleName()));
+ }
+ } catch (IOException ex) {
+ // Do not allow an inconsistent OM to start up.
+ throw new RuntimeException(
+ "Error while building tenant state cache from DB.", ex);
+ }
+
+ // Next use the access ID table to fill in membership info for each tenant.
+ int userCount = 0;
+ final Table<String, OmDBAccessIdInfo> tenantAccessIdTable =
+ omMetadataManager.getTenantAccessIdTable();
+ try (TableIterator<String, ? extends KeyValue<String, OmDBAccessIdInfo>>
+ accessIdTableIter = tenantAccessIdTable.iterator()) {
while (accessIdTableIter.hasNext()) {
final KeyValue<String, OmDBAccessIdInfo> next =
accessIdTableIter.next();
@@ -897,20 +914,12 @@ public class OMMultiTenantManagerImpl implements
OMMultiTenantManager {
final String userPrincipal = value.getUserPrincipal();
final boolean isAdmin = value.getIsAdmin();
- final OmDBTenantState tenantState = tenantStateTable.get(tenantId);
// If the TenantState doesn't exist, it means the accessId entry is
// orphaned or incorrect, likely metadata inconsistency
- Preconditions.checkNotNull(tenantState,
+ CachedTenantState cachedTenantState = tenantCache.get(tenantId);
+ Preconditions.checkNotNull(cachedTenantState,
"OmDBTenantState should have existed for " + tenantId);
- final String tenantUserRoleName = tenantState.getUserRoleName();
- final String tenantAdminRoleName = tenantState.getAdminRoleName();
-
- // Enter tenant cache entry when it is the first hit for this tenant
- final CachedTenantState cachedTenantState =
tenantCache.computeIfAbsent(
- tenantId, k -> new CachedTenantState(
- tenantId, tenantUserRoleName, tenantAdminRoleName));
-
cachedTenantState.getAccessIdInfoMap().put(accessId,
new CachedAccessIdInfo(userPrincipal, isAdmin));
userCount++;
@@ -918,7 +927,9 @@ public class OMMultiTenantManagerImpl implements
OMMultiTenantManager {
LOG.info("Loaded {} tenants and {} tenant users from the database",
tenantCache.size(), userCount);
} catch (IOException ex) {
- LOG.error("Error while loading user list", ex);
+ // Do not allow an inconsistent OM to start up.
+ throw new RuntimeException(
+ "Error while building tenant user cache from DB.", ex);
}
}
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/multitenant/CachedTenantState.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/multitenant/CachedTenantState.java
index ee19930d69..1f27f9c1fe 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/multitenant/CachedTenantState.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/multitenant/CachedTenantState.java
@@ -18,6 +18,7 @@
package org.apache.hadoop.ozone.om.multitenant;
import java.util.HashMap;
+import java.util.Objects;
/**
* A collection of things that we want to maintain about a tenant in memory.
@@ -64,6 +65,25 @@ public class CachedTenantState {
public boolean getIsAdmin() {
return isAdmin;
}
+
+ @Override
+ public boolean equals(Object object) {
+ if (object == null) {
+ return false;
+ }
+ if (object instanceof CachedAccessIdInfo) {
+ CachedAccessIdInfo other = (CachedAccessIdInfo) object;
+ return isAdmin == other.isAdmin &&
+ userPrincipal.equals(other.userPrincipal);
+ } else {
+ return false;
+ }
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(isAdmin, userPrincipal);
+ }
}
public CachedTenantState(String tenantId,
@@ -85,4 +105,25 @@ public class CachedTenantState {
public boolean isTenantEmpty() {
return accessIdInfoMap.isEmpty();
}
+
+ @Override
+ public boolean equals(Object object) {
+ if (object == null) {
+ return false;
+ }
+ if (object instanceof CachedTenantState) {
+ CachedTenantState other = (CachedTenantState) object;
+ return tenantId.equals(other.tenantId) &&
+ tenantUserRoleName.equals(other.tenantUserRoleName) &&
+ tenantAdminRoleName.equals(other.tenantAdminRoleName) &&
+ accessIdInfoMap.equals(other.accessIdInfoMap);
+ } else {
+ return false;
+ }
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(tenantId, tenantUserRoleName, tenantAdminRoleName);
+ }
}
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMultiTenantManagerImpl.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMultiTenantManagerImpl.java
index d28d1cdc55..432335fac1 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMultiTenantManagerImpl.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMultiTenantManagerImpl.java
@@ -36,6 +36,7 @@ import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.om.helpers.OmDBAccessIdInfo;
import org.apache.hadoop.ozone.om.helpers.OmDBTenantState;
import org.apache.hadoop.ozone.om.helpers.TenantUserList;
+import org.apache.hadoop.ozone.om.multitenant.CachedTenantState;
import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.UserAccessIdInfo;
import org.apache.ozone.test.LambdaTestUtils;
import org.junit.Assert;
@@ -52,37 +53,26 @@ public class TestOMMultiTenantManagerImpl {
private OMMultiTenantManagerImpl tenantManager;
private static final String TENANT_ID = "tenant1";
+ private OMMetadataManager omMetadataManager;
+ private OzoneConfiguration conf;
+ private OzoneManager ozoneManager;
@Rule
public TemporaryFolder folder = new TemporaryFolder();
@Before
public void setUp() throws IOException {
- OzoneConfiguration conf = new OzoneConfiguration();
+ conf = new OzoneConfiguration();
conf.set(OZONE_OM_DB_DIRS,
folder.newFolder().getAbsolutePath());
conf.set(OZONE_OM_TENANT_DEV_SKIP_RANGER, "true");
- OMMetadataManager omMetadataManager = new OmMetadataManagerImpl(conf);
+ omMetadataManager = new OmMetadataManagerImpl(conf);
- final String bucketNamespaceName = TENANT_ID;
- final String bucketNamespacePolicyName =
- OMMultiTenantManager.getDefaultBucketNamespacePolicyName(TENANT_ID);
- final String bucketPolicyName =
- OMMultiTenantManager.getDefaultBucketPolicyName(TENANT_ID);
- final String userRoleName =
- OMMultiTenantManager.getDefaultUserRoleName(TENANT_ID);
- final String adminRoleName =
- OMMultiTenantManager.getDefaultAdminRoleName(TENANT_ID);
- final OmDBTenantState omDBTenantState = new OmDBTenantState(TENANT_ID,
- bucketNamespaceName, userRoleName, adminRoleName,
- bucketNamespacePolicyName, bucketPolicyName);
-
- omMetadataManager.getTenantStateTable().put(TENANT_ID, omDBTenantState);
+ createTenantInDB(TENANT_ID);
+ assignUserToTenantInDB(TENANT_ID, "seed-accessId1", "seed-user1", false,
+ false);
- omMetadataManager.getTenantAccessIdTable().put("seed-accessId1",
- new OmDBAccessIdInfo(TENANT_ID, "seed-user1", false, false));
-
- OzoneManager ozoneManager = Mockito.mock(OzoneManager.class);
+ ozoneManager = Mockito.mock(OzoneManager.class);
Mockito.when(ozoneManager.getMetadataManager())
.thenReturn(omMetadataManager);
@@ -98,9 +88,35 @@ public class TestOMMultiTenantManagerImpl {
.thenReturn(ozoneConfiguration);
tenantManager = new OMMultiTenantManagerImpl(ozoneManager, conf);
- assertEquals(1, tenantManager.getTenantCache().size());
- assertEquals(1, tenantManager.getTenantCache().get(TENANT_ID)
- .getAccessIdInfoMap().size());
+ }
+
+ /**
+ * Tests rebuilding the tenant cache on restart.
+ */
+ @Test
+ public void testReloadCache() throws IOException {
+ // Create a tenant with multiple users.
+ CachedTenantState expectedTenant2State = createTenant("tenant2");
+ assignUserToTenant(expectedTenant2State, "access2", "user2", false, false);
+ assignUserToTenant(expectedTenant2State, "access3", "user2", true, false);
+ assignUserToTenant(expectedTenant2State, "access4", "user2", true, true);
+
+ // Create a tenant with no users.
+ CachedTenantState expectedTenant3State = createTenant("tenant3");
+
+ // Reload the cache as part of new object creation.
+ OMMultiTenantManagerImpl tenantManager2 =
+ new OMMultiTenantManagerImpl(ozoneManager, conf);
+ // Check that the cache was restored correctly.
+ // Setup created a tenant in addition to the ones created for this test.
+ assertEquals(3, tenantManager2.getTenantCache().size());
+ // Check tenant2
+ assertEquals(expectedTenant2State, tenantManager.getTenantCache()
+ .get("tenant2"));
+ // Check tenant3
+ assertEquals(expectedTenant3State, tenantManager.getTenantCache()
+ .get("tenant3"));
+
}
@Test
@@ -156,4 +172,68 @@ public class TestOMMultiTenantManagerImpl {
assertTrue(optionalTenant.isPresent());
assertEquals(TENANT_ID, optionalTenant.get());
}
+
+ /**
+ * @return A new {@link CachedTenantState} object expected to match the one
+ * created by the cache.
+ */
+ private CachedTenantState createTenant(String tenantId) throws IOException {
+ final String userRoleName =
+ OMMultiTenantManager.getDefaultUserRoleName(tenantId);
+ final String adminRoleName =
+ OMMultiTenantManager.getDefaultAdminRoleName(tenantId);
+ createTenantInDB(tenantId, userRoleName, adminRoleName);
+ tenantManager.getCacheOp().createTenant(tenantId, userRoleName,
+ adminRoleName);
+
+ return new CachedTenantState(tenantId, userRoleName, adminRoleName);
+ }
+
+ private void createTenantInDB(String tenantId) throws IOException {
+ final String userRoleName =
+ OMMultiTenantManager.getDefaultUserRoleName(tenantId);
+ final String adminRoleName =
+ OMMultiTenantManager.getDefaultAdminRoleName(tenantId);
+ createTenantInDB(tenantId, userRoleName, adminRoleName);
+ }
+
+ private void createTenantInDB(String tenantId, String userRoleName,
+ String adminRoleName) throws IOException {
+ final String bucketNamespaceName = tenantId;
+ final String bucketNamespacePolicyName =
+ OMMultiTenantManager.getDefaultBucketNamespacePolicyName(tenantId);
+ final String bucketPolicyName =
+ OMMultiTenantManager.getDefaultBucketPolicyName(tenantId);
+ final OmDBTenantState omDBTenantState = new OmDBTenantState(tenantId,
+ bucketNamespaceName, userRoleName, adminRoleName,
+ bucketNamespacePolicyName, bucketPolicyName);
+
+ omMetadataManager.getTenantStateTable().put(tenantId, omDBTenantState);
+ }
+
+ /**
+ * The {@link CachedTenantState} parameter will be updated to match the
+ * expected update performed by the cache.
+ */
+ private void assignUserToTenant(
+ CachedTenantState tenantState, String accessId, String user,
+ boolean isAdmin, boolean isDelegatedAdmin) throws IOException {
+ assignUserToTenantInDB(tenantState.getTenantId(), accessId, user, isAdmin,
+ isDelegatedAdmin);
+ tenantManager.getCacheOp().assignUserToTenant(user,
+ tenantState.getTenantId(), accessId);
+ if (isAdmin) {
+ tenantManager.getCacheOp().assignTenantAdmin(accessId, isDelegatedAdmin);
+ }
+
+ tenantState.getAccessIdInfoMap().put(accessId,
+ new CachedTenantState.CachedAccessIdInfo(user, isAdmin));
+ }
+
+ private void assignUserToTenantInDB(String tenantId, String accessId,
+ String user, boolean isAdmin, boolean isDelegatedAdmin)
+ throws IOException {
+ omMetadataManager.getTenantAccessIdTable().put(accessId,
+ new OmDBAccessIdInfo(tenantId, user, isAdmin, isDelegatedAdmin));
+ }
}
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
