errose28 commented on code in PR #9779:
URL: https://github.com/apache/ozone/pull/9779#discussion_r2848568664
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHddsUpgradeUtils.java:
##########
Review Comment:
This is only called by `TestHDDSUpgrade` which is not run in CI, we should
remove the container state check here for correctness even though the current
CI run won't flag it.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHddsUpgradeUtils.java:
##########
@@ -217,12 +180,9 @@ public static void testPostUpgradeConditionsDataNodes(
dnVersionManager.getMetadataLayoutVersion());
assertThat(dnVersionManager.getMetadataLayoutVersion()).isGreaterThanOrEqualTo(1);
- // Also verify that all the existing containers are closed.
- for (Container<?> container :
+ // Verify containers are in acceptable states (OPEN is now allowed).
+ for (Container<?> ignored :
dsm.getContainer().getController().getContainers()) {
- assertTrue(closeStates.stream().anyMatch(
- state -> container.getContainerState().equals(state)),
- "Container had unexpected state " + container.getContainerState());
countContainers++;
Review Comment:
The comment here is stale as well.
```suggestion
countContainers +=
dsm.getContainer().getContainerSet().containerCount();
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHddsUpgradeUtils.java:
##########
@@ -117,32 +107,13 @@ public static void
testPostUpgradeConditionsSCM(StorageContainerManager scm,
scmVersionManager.getMetadataLayoutVersion());
assertThat(scmVersionManager.getMetadataLayoutVersion()).isGreaterThanOrEqualTo(1);
- // SCM should not return from finalization until there is at least one
- // pipeline to use.
- PipelineManager scmPipelineManager = scm.getPipelineManager();
- try {
- GenericTestUtils.waitFor(
- () -> !scmPipelineManager.getPipelines(RATIS_THREE, OPEN).isEmpty(),
- 500, 60000);
- } catch (TimeoutException | InterruptedException e) {
- fail("Timeout waiting for Upgrade to complete on SCM.");
- }
-
- // SCM will not return from finalization until there is at least one
- // RATIS 3 pipeline. For this to exist, all three of our datanodes must
- // be in the HEALTHY state.
+ // SCM will not return from finalization until all HEALTHY datanodes
+ // have completed their finalization (MLV == SLV). This ensures datanodes
+ // are ready to serve requests even though containers may remain OPEN.
testDataNodesStateOnSCM(scm, numDatanodes, HEALTHY, HEALTHY_READONLY);
int countContainers = 0;
- for (ContainerInfo ci : scm.getContainerManager().getContainers()) {
- HddsProtos.LifeCycleState ciState = ci.getState();
- LOG.info("testPostUpgradeConditionsSCM: container state is {}",
- ciState.name());
- assertTrue((ciState == HddsProtos.LifeCycleState.CLOSED) ||
- (ciState == HddsProtos.LifeCycleState.CLOSING) ||
- (ciState == HddsProtos.LifeCycleState.DELETING) ||
- (ciState == HddsProtos.LifeCycleState.DELETED) ||
- (ciState == HddsProtos.LifeCycleState.QUASI_CLOSED));
+ for (ContainerInfo ignored : scm.getContainerManager().getContainers()) {
countContainers++;
}
Review Comment:
nit.
```suggestion
int countContainers = scm.getContainerManager().getContainers().size();
```
--
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]
