Skip to content
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.

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

HDDS-11309. Increase CONTAINER_STATE Column Length in UNHEALTHY_CONTAINERS to Avoid Truncation #7071

Merged
merged 5 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public enum UnHealthyContainerStates {
UNDER_REPLICATED,
OVER_REPLICATED,
MIS_REPLICATED,
ALL_REPLICAS_UNHEALTHY,
ALL_REPLICAS_BAD,
NEGATIVE_SIZE // Added new state to track containers with negative sizes
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
package org.apache.hadoop.ozone.recon.persistence;

import static org.hadoop.ozone.recon.schema.ContainerSchemaDefinition.UnHealthyContainerStates.UNDER_REPLICATED;
import static org.hadoop.ozone.recon.schema.ContainerSchemaDefinition.UnHealthyContainerStates.ALL_REPLICAS_UNHEALTHY;
import static org.hadoop.ozone.recon.schema.ContainerSchemaDefinition.UnHealthyContainerStates.ALL_REPLICAS_BAD;
import static org.hadoop.ozone.recon.schema.tables.UnhealthyContainersTable.UNHEALTHY_CONTAINERS;
import static org.jooq.impl.DSL.count;

Expand Down Expand Up @@ -76,7 +76,7 @@ public List<UnhealthyContainers> getUnhealthyContainers(
SelectQuery<Record> query = dslContext.selectQuery();
query.addFrom(UNHEALTHY_CONTAINERS);
if (state != null) {
if (state.equals(ALL_REPLICAS_UNHEALTHY)) {
if (state.equals(ALL_REPLICAS_BAD)) {
query.addConditions(UNHEALTHY_CONTAINERS.CONTAINER_STATE
.eq(UNDER_REPLICATED.toString()));
query.addConditions(UNHEALTHY_CONTAINERS.ACTUAL_REPLICA_COUNT.eq(0));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@

import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE;
import static org.assertj.core.api.Assertions.assertThat;
import static org.hadoop.ozone.recon.schema.ContainerSchemaDefinition.UnHealthyContainerStates.ALL_REPLICAS_UNHEALTHY;
import static org.hadoop.ozone.recon.schema.ContainerSchemaDefinition.UnHealthyContainerStates.ALL_REPLICAS_BAD;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyInt;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -199,7 +202,7 @@ public void testRun() throws Exception {

List<UnhealthyContainers> unhealthyContainers =
containerHealthSchemaManager.getUnhealthyContainers(
ALL_REPLICAS_UNHEALTHY, 0, Integer.MAX_VALUE);
ALL_REPLICAS_BAD, 0, Integer.MAX_VALUE);
assertEquals(1, unhealthyContainers.size());
assertEquals(2L,
unhealthyContainers.get(0).getContainerId().longValue());
Expand Down Expand Up @@ -384,6 +387,91 @@ public void testDeletedContainer() throws Exception {
.isGreaterThan(currentTime);
}

@Test
public void testAllContainerStateInsertions() {
UnhealthyContainersDao unHealthyContainersTableHandle =
getDao(UnhealthyContainersDao.class);

ContainerHealthSchemaManager containerHealthSchemaManager =
new ContainerHealthSchemaManager(
getSchemaDefinition(ContainerSchemaDefinition.class),
unHealthyContainersTableHandle);

// Iterate through each state in the UnHealthyContainerStates enum
for (ContainerSchemaDefinition.UnHealthyContainerStates state :
ContainerSchemaDefinition.UnHealthyContainerStates.values()) {

// Create a dummy UnhealthyContainer record with the current state
UnhealthyContainers unhealthyContainer = new UnhealthyContainers();
unhealthyContainer.setContainerId(state.ordinal() + 1L);

// Set replica counts based on the state
switch (state) {
case MISSING:
case EMPTY_MISSING:
unhealthyContainer.setExpectedReplicaCount(3);
unhealthyContainer.setActualReplicaCount(0);
unhealthyContainer.setReplicaDelta(3);
break;

case UNDER_REPLICATED:
unhealthyContainer.setExpectedReplicaCount(3);
unhealthyContainer.setActualReplicaCount(1);
unhealthyContainer.setReplicaDelta(2);
break;

case OVER_REPLICATED:
unhealthyContainer.setExpectedReplicaCount(3);
unhealthyContainer.setActualReplicaCount(4);
unhealthyContainer.setReplicaDelta(-1);
break;

case MIS_REPLICATED:
ArafatKhan2198 marked this conversation as resolved.
Show resolved Hide resolved
case NEGATIVE_SIZE:
unhealthyContainer.setExpectedReplicaCount(3);
unhealthyContainer.setActualReplicaCount(3);
unhealthyContainer.setReplicaDelta(0);
break;

case ALL_REPLICAS_BAD:
unhealthyContainer.setExpectedReplicaCount(3);
unhealthyContainer.setActualReplicaCount(0);
unhealthyContainer.setReplicaDelta(3);
break;

default:
fail("Unhandled state: " + state.name() + ". Please add this state to the switch case.");
}

unhealthyContainer.setContainerState(state.name());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though it will not impact test case purpose, but for each UnhealthyContainerState, expected replica count and actual replica count will differ. Currently what you have set in test is a Under replicated container , but in a loop setting container state same for all iterations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls fix this comment, as it creates confusion.

unhealthyContainer.setInStateSince(System.currentTimeMillis());

// Try inserting the record and catch any exception that occurs
Exception exception = null;
try {
containerHealthSchemaManager.insertUnhealthyContainerRecords(
Collections.singletonList(unhealthyContainer));
} catch (Exception e) {
exception = e;
}

// Assert no exception should be thrown for each state
assertNull(exception,
"Exception was thrown during insertion for state " + state.name() +
": " + exception);

// Optionally, verify the record was inserted correctly
List<UnhealthyContainers> insertedRecords =
unHealthyContainersTableHandle.fetchByContainerId(
state.ordinal() + 1L);
assertFalse(insertedRecords.isEmpty(),
"Record was not inserted for state " + state.name() + ".");
assertEquals(insertedRecords.get(0).getContainerState(), state.name(),
"The inserted container state does not match for state " +
state.name() + ".");
}
}

@Test
public void testNegativeSizeContainers() throws Exception {
// Setup mock objects and test environment
Expand Down