F64116045 commented on PR #11199: URL: https://github.com/apache/ozone/pull/11199#issuecomment-5779474154
Thanks @sodonnel and @devmadhuu. I agree with treating checksum mismatch as a normal health state. The combined states @devmadhuu suggested seem like a good way, I’ll also check the other possible overlaps before deciding which ones we need. > On Recon: I think, making mismatch first-class also removes a whole parallel detection path this PR currently adds in Recon. Right now ReconReplicationManager.processAll() does its normal processContainer(..., readOnly=true) pass and then it runs very recon specific - hasDataChecksumMismatch(container, replicas) check, feeding matches into a new ReconReplicationManagerReport.replicaMismatchContainers list. The Javadoc also for replicaMismatchContainers field says that it has to be tracked separately. Once mismatch is a real ContainerHealthState value, SCM's DataChecksumMismatchCheckHandler runs during Recon's read-only pass exactly like every other handler, stamps the state onto ContainerInfo.healthState, and Recon's downstream bucketing reads it back the same way it reads MISSING, UNDER_REPLICATED, etc. So hasDataChecksumMismatch, replicaMismatchContainers, and addReplicaMismatchContainer, all can be removed. Recon has one detection lane instead of two. Simplify the code of Rec on also in separate PR. Thanks for the suggestion for Recon. Also I think the follow up should move its EC comparison into the shared handler too, so we can remove Recon’s separate check entirely. This would preserve the behavior fixed in #11215 but move it to SCM -- 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]
