devmadhuu commented on code in PR #11594:
URL: https://github.com/apache/ozone/pull/11594#discussion_r4411935216
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconStorageContainerSyncHelper.java:
##########
@@ -425,6 +426,59 @@ private boolean syncDeletedContainers() {
}
}
+ /**
+ * Copies SCM's suppressed flag to the containers Recon already has. The
+ * passes above compare only the lifecycle state, and Recon's health sweep
+ * skips suppressed containers. SCM is asked only for the suppressed
+ * containers, which are normally few.
+ *
+ * @return {@code true} if all RPC calls completed without error
+ */
+ private boolean syncSuppressedFlags() {
+ try {
+ Set<ContainerID> suppressedInScm = new HashSet<>();
+ // A ContainerInfo is smaller than a ContainerWithPipeline, so that page
size is safe here.
+ int batchSize = safeContainerWithPipelineBatchSize(Integer.MAX_VALUE);
+ ContainerID start = ContainerID.valueOf(1);
+ boolean more = true;
+ while (more) {
+ List<ContainerInfo> page =
scmServiceProvider.getListOfSuppressedContainerInfos(start, batchSize);
+ more = !page.isEmpty();
+ for (ContainerInfo info : page) {
+ if (!info.isSuppressed()) {
+ // The filter came with the flag, so an SCM that ignores it has no
suppressed container.
+ LOG.debug("SCM listed unsuppressed container {}, so it has no
suppressed containers.", info.containerID());
+ more = false;
+ break;
Review Comment:
Recon can mistake an incomplete list for a complete list from SCM. Think of
SCM as keeping a list of containers whose alerts the administrator wants hidden.
Above code follows:
Step 1: Collect all suppressed container IDs from SCM.
Step 2: Visit every container in Recon:
ID is in the collected set → mark suppressed
ID is absent → mark unsuppressed
**Consider this example:**
Container | SCM’s suppression flag initially
-- | --
1 | Suppressed
2 | Suppressed
3 | Suppressed
Suppose SCM returns two containers per page:
First page: 1, 2
Second page: 3
While preparing the first response, SCM selects containers 1 and 2 because
both are suppressed. Before it finishes converting those objects into the RPC
response, an administrator unsuppresses container 1.
SCM can consequently send:
First page: [container 1: false, container 2: true]
This is possible because the selected objects can still change before the
response is serialized.
Now look at this part of the PR:
```
if (!info.isSuppressed()) {
more = false;
break;
}
```
`suppressedInScm.add(info.containerID());`
When Recon sees container 1 with false:
- break skips the rest of that page, including container 2.
- more = false prevents fetching the next page containing container 3.
- The collected suppressedInScm set is still empty.
Recon then executes the second step:
```
boolean suppressed =
suppressedInScm.contains(container.containerID());
```
Because the set is empty, Recon marks all three containers unsuppressed.
But the administrator only unsuppressed container 1. Containers 2 and 3
remain suppressed in SCM. Their alerts can incorrectly reappear in Recon. So we
can say “I stopped collecting the list, therefore everything I did not collect
is unsuppressed.”
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconStorageContainerSyncHelper.java:
##########
@@ -425,6 +426,59 @@ private boolean syncDeletedContainers() {
}
}
+ /**
+ * Copies SCM's suppressed flag to the containers Recon already has. The
+ * passes above compare only the lifecycle state, and Recon's health sweep
+ * skips suppressed containers. SCM is asked only for the suppressed
+ * containers, which are normally few.
+ *
+ * @return {@code true} if all RPC calls completed without error
+ */
+ private boolean syncSuppressedFlags() {
Review Comment:
Although this RPC returns only suppressed containers,
`SCMClientProtocolServer.listContainerInternal()` first materializes the
complete container list, then filters it and applies the page limit. Every
pagination request repeats that full scan, including the final request that
returns an empty page. Recon then also materializes and scans its complete
local container list.
Consequently, even when only a few containers are suppressed, this adds work
proportional to the total container count on both SCM and Recon. At tens of
millions of containers, that could introduce substantial allocation, GC
pressure, and sync latency. The existing 100k-container tests mock the SCM
provider, so they do not validate this RPC cost.
Could we avoid the repeated full scans—for example, through an indexed
suppressed-ID listing—and use bounded iteration on the Recon side?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
