siddhantsangwan commented on PR #5120: URL: https://github.com/apache/ozone/pull/5120#issuecomment-1652919841
It's important to note the impact of changing `RatisContainerReplicaCount`: 1. We are changing the definition of the "unhealthy" term from meaning replicas with `UNHEALTHY` state to replicas with either `UNHEALTHY` state, or `QUASI_CLOSED` and sequence id not matching the container's sequence id. There's extensive existing documentation where "unhealthy" is meant for only `UNHEALTHY` replicas, so we'll have to change that. Perhaps in a future PR. 2. We have a `RatisUnhealthyReplicationCheckHandler` whose purpose was to detect over/under replication when only `UNHEALTHY` replicas are present in the cluster. Since we're changing the `getUnhealthyReplicaCount()` API in `RatisContainerReplicaCount` such that it also returns `QUASI_CLOSED` replicas with incorrect sequence, this class's logic will now also include these replicas. This also affects `RatisUnderReplicationHandler`. 3. `RatisOverReplicationHandler` already has logic to try and delete unhealthy replicas or replicas whose state doesn't match the container's state first. So in theory, this class should work well with this change. For confidence, we can add unit tests to `TestRatisContainerReplicaCount`, `TestRatisUnderReplicationHandler`, `TestRatisOverReplicationHandler`, and `TestRatisReplicationCheckHandler`. Ideally something in `TestRatisUnhealthyReplicationCheckHandler` too. For integration testing, we have `testContainerReplicationWithLegacyReplicationManagerDisabled` in `TestContainerReplication` (though having `QUASI_CLOSED` replicas in an integration test is better suited for a different PR). -- 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]
