You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "sreejasahithi (via GitHub)" <gi...@apache.org> on 2026/05/18 10:25:43 UTC

[PR] HDDS-15305. ozone admin container list --all returns duplicate containers due to sort/pagination key mismatch [ozone]

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

   ## What changes were proposed in this pull request?
   The ozone admin container list --all command produces duplicate container entries in its output. The duplicates are non-deterministic across runs, which containers are duplicated depends on the order in which the SCM returns them, which can vary.
   
   Root Cause:
   The bug is a mismatch between the server-side sort key and the client-side pagination key.
   Server side (SCMClientProtocolServer.listContainerInternal):The method filters containers by `containerID >= startContainerID`, then calls `.sorted()` which invokes `ContainerInfo.compareTo()`. 
   That comparator is defined as:
   
   > private static final Comparator<ContainerInfo> COMPARATOR = Comparator.comparingLong(info -> info.getLastUsed().toEpochMilli());  
   
   So the batch is returned sorted by lastUsed timestamp, not by containerID.
   
   Client side (ListSubcommand.listAllContainers): The pagination loop advances the cursor using the last returned element's container ID.
   This assumes the last element of the batch has the highest container ID. That is only true if the batch is sorted by container ID. Since it is actually sorted by lastUsed, the last element can have a lower container ID than other elements already in the same batch.
    
   Example: With batch size 40, a batch may return containers with IDs [..., 41, 42, 44, 40] (sorted by lastUsed). The cursor is set to 40 + 1 = 41. The next batch fetches containers with containerID >= 41, returning [41, 42, 43, 44, 45, ...] — re-fetching 41, 42, and 44.
    
   Fix : 
   In SCMClientProtocolServer.listContainerInternal, replace the lastUsed-based sort with a containerID-based sort, aligning the sort key with the pagination key:
   
   > .sorted(Comparator.comparing(ContainerInfo::containerID))
   
   ## What is the link to the Apache JIRA
   [HDDS-15305](https://issues.apache.org/jira/browse/HDDS-15305)
   
   ## How was this patch tested?
   Manually tested in docket cluster.
   Green CI : https://github.com/sreejasahithi/ozone/actions/runs/26017534135


-- 
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


Re: [PR] HDDS-15305. ozone admin container list --all returns duplicate containers due to sort/pagination key mismatch [ozone]

Posted by "sreejasahithi (via GitHub)" <gi...@apache.org>.
sreejasahithi commented on PR #10300:
URL: https://github.com/apache/ozone/pull/10300#issuecomment-4476933857

   @ssulav and @sarvekshayr could you please review this patch.


-- 
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


Re: [PR] HDDS-15305. ozone admin container list --all returns duplicate containers due to sort/pagination key mismatch [ozone]

Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #10300:
URL: https://github.com/apache/ozone/pull/10300#discussion_r3258368104


##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestSCMClientProtocolServer.java:
##########
@@ -155,12 +159,49 @@ public void testScmGetContainerCount() throws IOException {
       scmServer.stop();
     }
   }
+  
+  @Test
+  public void testListContainerPaginationHasNoDuplicates() throws Exception {
+    Instant base = Instant.parse("2026-01-01T00:00:00Z");
+    List<ContainerInfo> infos = new ArrayList<>();
+    infos.add(newContainerWithLastUsedTime(100, base));
+    infos.add(newContainerWithLastUsedTime(5, base.plusMillis(1)));
+    infos.add(newContainerWithLastUsedTime(10, base.plusMillis(2)));
+
+    SCMClientProtocolServer scmServer =
+        new SCMClientProtocolServer(new OzoneConfiguration(),
+            mockStorageContainerManager(infos),
+            mock(ReconfigurationHandler.class));
+    try {
+      List<Long> ids = new ArrayList<>();
+      long start = 0;
+      int batchSize = 2;
+      while (true) {
+        List<ContainerInfo> page =
+            scmServer.listContainer(start, batchSize, null, null, null).getContainerInfoList();
+        if (page.isEmpty()) {
+          break;
+        }
+        for (ContainerInfo c : page) {
+          ids.add(c.getContainerID());
+        }
+        start = page.get(page.size() - 1).getContainerID() + 1;
+      }
+      assertEquals(ids.size(), new HashSet<>(ids).size());

Review Comment:
   should also assert the sort order.



-- 
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


Re: [PR] HDDS-15305. ozone admin container list --all returns duplicate containers due to sort/pagination key mismatch [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #10300:
URL: https://github.com/apache/ozone/pull/10300#issuecomment-4489907579

   Thanks @sreejasahithi for the patch, @priyeshkaratha, @sarvekshayr, @yandrey321 for the review.


-- 
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


Re: [PR] HDDS-15305. ozone admin container list --all returns duplicate containers due to sort/pagination key mismatch [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #10300:
URL: https://github.com/apache/ozone/pull/10300


-- 
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


Re: [PR] HDDS-15305. ozone admin container list --all returns duplicate containers due to sort/pagination key mismatch [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #10300:
URL: https://github.com/apache/ozone/pull/10300#issuecomment-4489811326

   @yandrey321 would you like to take another look?


-- 
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


Re: [PR] HDDS-15305. ozone admin container list --all returns duplicate containers due to sort/pagination key mismatch [ozone]

Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #10300:
URL: https://github.com/apache/ozone/pull/10300#discussion_r3258368104


##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestSCMClientProtocolServer.java:
##########
@@ -155,12 +159,49 @@ public void testScmGetContainerCount() throws IOException {
       scmServer.stop();
     }
   }
+  
+  @Test
+  public void testListContainerPaginationHasNoDuplicates() throws Exception {
+    Instant base = Instant.parse("2026-01-01T00:00:00Z");
+    List<ContainerInfo> infos = new ArrayList<>();
+    infos.add(newContainerWithLastUsedTime(100, base));
+    infos.add(newContainerWithLastUsedTime(5, base.plusMillis(1)));
+    infos.add(newContainerWithLastUsedTime(10, base.plusMillis(2)));
+
+    SCMClientProtocolServer scmServer =
+        new SCMClientProtocolServer(new OzoneConfiguration(),
+            mockStorageContainerManager(infos),
+            mock(ReconfigurationHandler.class));
+    try {
+      List<Long> ids = new ArrayList<>();
+      long start = 0;
+      int batchSize = 2;
+      while (true) {
+        List<ContainerInfo> page =
+            scmServer.listContainer(start, batchSize, null, null, null).getContainerInfoList();
+        if (page.isEmpty()) {
+          break;
+        }
+        for (ContainerInfo c : page) {
+          ids.add(c.getContainerID());
+        }
+        start = page.get(page.size() - 1).getContainerID() + 1;
+      }
+      assertEquals(ids.size(), new HashSet<>(ids).size());

Review Comment:
   should also assert the sort order and expected ids



-- 
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