You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/07/23 01:12:51 UTC

[GitHub] [ozone] errose28 opened a new pull request, #3618: HDDS-7042. Rebuilding tenant cache omits empty tenants

errose28 opened a new pull request, #3618:
URL: https://github.com/apache/ozone/pull/3618

   ## What changes were proposed in this pull request?
   
   On startup, the tenant cache is built from the DB by iterating the access ID table. If the tenant does not have any users, there will be no corresponding entries in the access ID table and the cache will not contain an entry for that tenant, even though there is info for that tenant in the DB's tenant state table. This leads to incorrect behavior while the OM is running.
   
   ## What is the link to the Apache JIRA
   
   HDDS-7042
   
   ## How was this patch tested?
   
   Unit test added.
   


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] errose28 merged pull request #3618: HDDS-7042. Rebuilding tenant cache omits empty tenants

Posted by GitBox <gi...@apache.org>.
errose28 merged PR #3618:
URL: https://github.com/apache/ozone/pull/3618


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] smengcl commented on a diff in pull request #3618: HDDS-7042. Rebuilding tenant cache omits empty tenants

Posted by GitBox <gi...@apache.org>.
smengcl commented on code in PR #3618:
URL: https://github.com/apache/ozone/pull/3618#discussion_r929295815


##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMultiTenantManagerImpl.java:
##########
@@ -98,9 +88,35 @@ public void setUp() throws IOException {
         .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"));

Review Comment:
   nvm. included.



##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMultiTenantManagerImpl.java:
##########
@@ -98,9 +88,35 @@ public void setUp() throws IOException {
         .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"));

Review Comment:
   nvm. included



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] smengcl commented on a diff in pull request #3618: HDDS-7042. Rebuilding tenant cache omits empty tenants

Posted by GitBox <gi...@apache.org>.
smengcl commented on code in PR #3618:
URL: https://github.com/apache/ozone/pull/3618#discussion_r929295184


##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMultiTenantManagerImpl.java:
##########
@@ -98,9 +88,35 @@ public void setUp() throws IOException {
         .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"));

Review Comment:
   nit: check `tenant2` has 3 users



##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMultiTenantManagerImpl.java:
##########
@@ -98,9 +88,35 @@ public void setUp() throws IOException {
         .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"));

Review Comment:
   nit: check `tenant3` has 0 users



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] errose28 commented on pull request #3618: HDDS-7042. Rebuilding tenant cache omits empty tenants

Posted by GitBox <gi...@apache.org>.
errose28 commented on PR #3618:
URL: https://github.com/apache/ozone/pull/3618#issuecomment-1194630452

   Thanks for the review @smengcl. Merging this bug fix.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org