F64116045 commented on PR #11199: URL: https://github.com/apache/ozone/pull/11199#issuecomment-5681749457
Thanks @sodonnel for taking the time to review this and for all your help. > We should deal with just the RM change and the report initially and leave the ContainerInfo command change as a second task. Agreed on splitting out the CLI changes. I was also concerned about the size and as I [mentioned earlier](https://github.com/apache/ozone/pull/11199#issuecomment-5545278423). > Also, we are adding a new health state, but we are treating it different to other health states (under replicated, over replicated, unhealthy). For all the other states, the state is stored into the container object in memory after it is processed. Why should this one be different? Does it make sense to have this like any other? Quasi_Closed_Stuck, for example, is an "unfixable state" right now. Containers in this state cannot get out of it, and this BCSID+checksum mismatch is nearly the same - its informational, but nothing for an operator to do. In the future perhaps they do take action or run a manual reconcile command, so it feels as valid a health state as QC_Stuck. If we treat it as a normal health state, then we do need to be aware that a container can only have a single health state. That means all replication and health related checks have passed and their problems fixed via replication, as they are more important than the mis-match. I originally kept checksum mismatch separate so it could be reported alongside replication issues. How about moving the checksum check after the existing health checks and recording it as a normal health state only when no other health issue was found? But that would mean a mismatch is not reported until the higher-priority issues are resolved. As you pointed out, this is informational for now, like QC_Stuck. I think it’s reasonable to let the repair checks run first and then report the mismatch as a normal health state. > Is it possible for a container to be QC_Stuck and have this checksum mis-match? If so, which would win - BCSID+checksum mis-match or QC_Stuck? Yes, the two conditions can coexist. But the current check only covers CLOSED RATIS containers, so QC_Stuck containers are not checked. Do you think we need to cover QUASI_CLOSED containers as well? If so, I’m not sure which state should take priority if both are present. -- 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]
