Skip to content

HDDS-8025. ReplicationManager: Count a container once for missing, under, mis or over replicated #4313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adoroszlai merged 2 commits into from
Feb 24, 2023
Merged

HDDS-8025. ReplicationManager: Count a container once for missing, under, mis or over replicated #4313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4ba0a99
HDDS-8025. ReplicationManager: Count a container once for missing, un…
Feb 24, 2023
435e23e
Fix failing unit tests
Feb 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions ...va/org/apache/hadoop/hdds/scm/container/replication/health/ECReplicationCheckHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,16 @@ public boolean handle(ContainerCheckRequest request) {
// handler so return as unhandled so any further handlers will be tried.
return false;
}
// TODO - should the report have a HEALTHY state, rather than just bad
// states? It would need to be added to legacy RM too.
if (health.getHealthState()
== ContainerHealthResult.HealthState.UNDER_REPLICATED) {
report.incrementAndSample(
ReplicationManagerReport.HealthState.UNDER_REPLICATED, containerID);
ContainerHealthResult.UnderReplicatedHealthResult underHealth
= ((ContainerHealthResult.UnderReplicatedHealthResult) health);
if (underHealth.isUnrecoverable()) {
// TODO - do we need a new health state for unrecoverable EC?
report.incrementAndSample(
ReplicationManagerReport.HealthState.MISSING, containerID);
} else {
report.incrementAndSample(
ReplicationManagerReport.HealthState.UNDER_REPLICATED, containerID);
}
if (!underHealth.isReplicatedOkAfterPending() &&
(!underHealth.isUnrecoverable()
Expand Down
7 changes: 4 additions & 3 deletions ...org/apache/hadoop/hdds/scm/container/replication/health/RatisReplicationCheckHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,6 @@ public boolean handle(ContainerCheckRequest request) {
return false;
}

report.incrementAndSample(
ReplicationManagerReport.HealthState.UNDER_REPLICATED,
container.containerID());
LOG.debug("Container {} is Under Replicated. isReplicatedOkAfterPending" +
" is [{}]. isUnrecoverable is [{}]. hasHealthyReplicas is [{}].",
container,
Expand All @@ -103,6 +100,10 @@ public boolean handle(ContainerCheckRequest request) {
container.containerID());
return true;
}
report.incrementAndSample(
ReplicationManagerReport.HealthState.UNDER_REPLICATED,
container.containerID());

if (!underHealth.isReplicatedOkAfterPending() &&
underHealth.hasHealthyReplicas()) {
request.getReplicationQueue().enqueue(underHealth);
Expand Down
4 changes: 2 additions & 2 deletions ...rc/test/java/org/apache/hadoop/hdds/scm/container/replication/TestReplicationManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ public void testUnderReplicatedAndUnrecoverable()
// replication list. It will be checked again on the next RM run.
Assert.assertEquals(0, repQueue.underReplicatedQueueSize());
Assert.assertEquals(0, repQueue.overReplicatedQueueSize());
Assert.assertEquals(1, repReport.getStat(
Assert.assertEquals(0, repReport.getStat(
ReplicationManagerReport.HealthState.UNDER_REPLICATED));
Assert.assertEquals(1, repReport.getStat(
ReplicationManagerReport.HealthState.MISSING));
Expand Down Expand Up @@ -492,7 +492,7 @@ public void testUnrecoverableClosedContainerWithUnhealthyReplicas()

Assert.assertEquals(0, repQueue.underReplicatedQueueSize());
Assert.assertEquals(0, repQueue.overReplicatedQueueSize());
Assert.assertEquals(1, repReport.getStat(
Assert.assertEquals(0, repReport.getStat(
ReplicationManagerReport.HealthState.UNDER_REPLICATED));
Assert.assertEquals(1, repReport.getStat(
ReplicationManagerReport.HealthState.MISSING));
Expand Down
4 changes: 2 additions & 2 deletions ...rg/apache/hadoop/hdds/scm/container/replication/health/TestECReplicationCheckHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ public void testUnderReplicatedAndUnrecoverable() {
// Unrecoverable so not added to the queue
Assert.assertEquals(0, repQueue.underReplicatedQueueSize());
Assert.assertEquals(0, repQueue.overReplicatedQueueSize());
Assert.assertEquals(1, report.getStat(
Assert.assertEquals(0, report.getStat(
ReplicationManagerReport.HealthState.UNDER_REPLICATED));
Assert.assertEquals(1, report.getStat(
ReplicationManagerReport.HealthState.MISSING));
Expand Down Expand Up @@ -310,7 +310,7 @@ private void testUnderReplicatedAndUnrecoverableWithOffline(
// Unrecoverable so not added to the queue
Assert.assertEquals(1, repQueue.underReplicatedQueueSize());
Assert.assertEquals(0, repQueue.overReplicatedQueueSize());
Assert.assertEquals(1, report.getStat(
Assert.assertEquals(0, report.getStat(
ReplicationManagerReport.HealthState.UNDER_REPLICATED));
Assert.assertEquals(1, report.getStat(
ReplicationManagerReport.HealthState.MISSING));
Expand Down
2 changes: 1 addition & 1 deletion ...apache/hadoop/hdds/scm/container/replication/health/TestRatisReplicationCheckHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ public void testUnderReplicatedAndUnrecoverable() {
// Unrecoverable, so not added to the queue.
Assert.assertEquals(0, repQueue.underReplicatedQueueSize());
Assert.assertEquals(0, repQueue.overReplicatedQueueSize());
Assert.assertEquals(1, report.getStat(
Assert.assertEquals(0, report.getStat(
ReplicationManagerReport.HealthState.UNDER_REPLICATED));
Assert.assertEquals(1, report.getStat(
ReplicationManagerReport.HealthState.MISSING));
Expand Down