sumitagrawl commented on code in PR #10326:
URL: https://github.com/apache/ozone/pull/10326#discussion_r3360509338
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java:
##########
@@ -210,28 +214,102 @@ void transitionOpenToClosing(ContainerID containerID,
ContainerInfo containerInf
pipelineToOpenContainer.put(pipelineID, curCnt - 1);
}
}
- updateContainerState(containerID, FINALIZE); // OPEN → CLOSING
+ updateContainerState(containerID, FINALIZE);
}
/**
- * Check if an OPEN container should move to CLOSING based on a healthy
- * non-OPEN DN replica report.
+ * Check if Recon's container lifecycle state can be corrected based on a DN
+ * replica report and SCM's authoritative state.
+ *
+ * <p>OPEN uses the DN replica state as the signal: only a healthy non-OPEN
+ * replica can move Recon to CLOSING. For CLOSING, the DN report is only a
+ * wake-up signal and Recon queries SCM before advancing. For DELETED, Recon
+ * recovers only on a live terminal replica report and only when SCM still
+ * reports the container as live.
+ *
+ * @param containerID containerID to check
+ * @param state replica state reported by a DataNode
*/
private void checkContainerStateAndUpdate(ContainerID containerID,
- ContainerReplicaProto.State
replicaState)
+ ContainerReplicaProto.State state)
throws IOException, InvalidStateTransitionException {
ContainerInfo containerInfo = getContainer(containerID);
HddsProtos.LifeCycleState reconState = containerInfo.getState();
- if (reconState != HddsProtos.LifeCycleState.OPEN
- || replicaState == ContainerReplicaProto.State.OPEN
- || !isHealthy(replicaState)) {
+ if (reconState == HddsProtos.LifeCycleState.OPEN) {
+ if (state.equals(ContainerReplicaProto.State.OPEN) || !isHealthy(state))
{
+ return;
+ }
+ LOG.info("Container {} has state OPEN, but given state is {}.",
+ containerID, state);
+ transitionOpenToClosing(containerID, containerInfo);
+ return;
+ }
+
+ if (reconState == HddsProtos.LifeCycleState.CLOSING) {
+ reconcileClosingContainerFromScm(containerID);
+ return;
+ }
+
+ if (reconState == HddsProtos.LifeCycleState.DELETED) {
Review Comment:
we need consider replica state as not deleted and not deleting case only
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java:
##########
@@ -210,28 +214,102 @@ void transitionOpenToClosing(ContainerID containerID,
ContainerInfo containerInf
pipelineToOpenContainer.put(pipelineID, curCnt - 1);
}
}
- updateContainerState(containerID, FINALIZE); // OPEN → CLOSING
+ updateContainerState(containerID, FINALIZE);
}
/**
- * Check if an OPEN container should move to CLOSING based on a healthy
- * non-OPEN DN replica report.
+ * Check if Recon's container lifecycle state can be corrected based on a DN
+ * replica report and SCM's authoritative state.
+ *
+ * <p>OPEN uses the DN replica state as the signal: only a healthy non-OPEN
+ * replica can move Recon to CLOSING. For CLOSING, the DN report is only a
+ * wake-up signal and Recon queries SCM before advancing. For DELETED, Recon
+ * recovers only on a live terminal replica report and only when SCM still
+ * reports the container as live.
+ *
+ * @param containerID containerID to check
+ * @param state replica state reported by a DataNode
*/
private void checkContainerStateAndUpdate(ContainerID containerID,
- ContainerReplicaProto.State
replicaState)
+ ContainerReplicaProto.State state)
throws IOException, InvalidStateTransitionException {
ContainerInfo containerInfo = getContainer(containerID);
HddsProtos.LifeCycleState reconState = containerInfo.getState();
- if (reconState != HddsProtos.LifeCycleState.OPEN
- || replicaState == ContainerReplicaProto.State.OPEN
- || !isHealthy(replicaState)) {
+ if (reconState == HddsProtos.LifeCycleState.OPEN) {
+ if (state.equals(ContainerReplicaProto.State.OPEN) || !isHealthy(state))
{
+ return;
+ }
+ LOG.info("Container {} has state OPEN, but given state is {}.",
+ containerID, state);
+ transitionOpenToClosing(containerID, containerInfo);
+ return;
+ }
+
+ if (reconState == HddsProtos.LifeCycleState.CLOSING) {
Review Comment:
why do we need handling closing here based on recon state ? it can be
handled as part of DN state if closed as reported.
--
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]
