You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ArafatKhan2198 (via GitHub)" <gi...@apache.org> on 2024/08/13 18:25:32 UTC
[PR] HDDS-11309. Increase CONTAINER_STATE Column Length in UNHEALTHY_CONTAINERS Table to Avoid Truncation Errors for Enum States. [ozone]
ArafatKhan2198 opened a new pull request, #7071:
URL: https://github.com/apache/ozone/pull/7071
## What changes were proposed in this pull request?
### Problem
During the execution of the `ContainerHealthTask`, it was discovered that the length of the `CONTAINER_STATE` column in the `UNHEALTHY_CONTAINERS` table is insufficient to store some of the enum values defined in UnHealthyContainerStates. Specifically, the `ALL_REPLICAS_UNHEALTHY` state, which has a length of 21 characters, exceeds the current `VARCHAR(16)` limit set for the `CONTAINER_STATE` column.
When attempting to insert a record with the `ALL_REPLICAS_UNHEALTHY` state into the `UNHEALTHY_CONTAINERS` table, a truncation error is thrown:
```
org.jooq.exception.DataAccessException: SQL [insert into "UNHEALTHY_CONTAINERS" ("container_id", "container_state", "in_state_since", "expected_replica_count", "actual_replica_count", "replica_delta", "reason") values ...]; A truncation error was encountered trying to shrink VARCHAR 'ALL_REPLICAS_UNHEALTHY' to length 16.
```
This issue affects the functionality of the ContainerHealthTask and prevents it from correctly recording certain unhealthy container states.
### Solution
Increase the `CONTAINER_STATE` column length to `VARCHAR(40)` and add a test to ensure all UnHealthyContainerStates enum values insert without truncation errors.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-11309
## How was this patch tested?
UT
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11309. Increase CONTAINER_STATE Column Length in UNHEALTHY_CONTAINERS to Avoid Truncation [ozone]
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on PR #7071:
URL: https://github.com/apache/ozone/pull/7071#issuecomment-2288899573
@devmadhuu @sumitagrawl to avoid breaking backwards compatibility I have changed the name from ALL_REPLICAS_UNHEALTHY to ALL_REPLICAS_BAD to fit the limit of 16 characters.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11309. Increase CONTAINER_STATE Column Length in UNHEALTHY_CONTAINERS Table to Avoid Truncation Errors for Enum States. [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #7071:
URL: https://github.com/apache/ozone/pull/7071#discussion_r1716309414
##########
hadoop-ozone/recon-codegen/src/main/java/org/hadoop/ozone/recon/schema/ContainerSchemaDefinition.java:
##########
@@ -80,7 +80,7 @@ public void initializeSchema() throws SQLException {
private void createUnhealthyContainersTable() {
dslContext.createTableIfNotExists(UNHEALTHY_CONTAINERS_TABLE_NAME)
.column(CONTAINER_ID, SQLDataType.BIGINT.nullable(false))
- .column(CONTAINER_STATE, SQLDataType.VARCHAR(16).nullable(false))
+ .column(CONTAINER_STATE, SQLDataType.VARCHAR(40).nullable(false))
Review Comment:
Please check for compatibility for existing DB, we may need change column length on upgrade.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11309. Increase CONTAINER_STATE Column Length in UNHEALTHY_CONTAINERS to Avoid Truncation [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu merged PR #7071:
URL: https://github.com/apache/ozone/pull/7071
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11309. Increase CONTAINER_STATE Column Length in UNHEALTHY_CONTAINERS Table to Avoid Truncation Errors for Enum States. [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #7071:
URL: https://github.com/apache/ozone/pull/7071#discussion_r1716282406
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java:
##########
@@ -384,6 +385,55 @@ public void testDeletedContainer() throws Exception {
.isGreaterThan(currentTime);
}
+ @Test
+ public void testAllContainerStateInsertions() {
+ // Set up DAOs and Schema Manager
+ 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); // Ensure unique ID for each state
+ unhealthyContainer.setExpectedReplicaCount(3);
+ unhealthyContainer.setActualReplicaCount(1);
+ unhealthyContainer.setReplicaDelta(2);
+ unhealthyContainer.setContainerState(state.name());
Review Comment:
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.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11309. Increase CONTAINER_STATE Column Length in UNHEALTHY_CONTAINERS to Avoid Truncation [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #7071:
URL: https://github.com/apache/ozone/pull/7071#discussion_r1722774991
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java:
##########
@@ -384,6 +386,92 @@ 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:
Review Comment:
Since you are not testing the actual occurrence of `MIS_REPLICATION` and not testing `NEGATIVE_SIZE` as well in this test case, can we combine both the `MIS_REPLICATED` and `NEGATIVE_SIZE` switch case here.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11309. Increase CONTAINER_STATE Column Length in UNHEALTHY_CONTAINERS to Avoid Truncation [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #7071:
URL: https://github.com/apache/ozone/pull/7071#discussion_r1721292254
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java:
##########
@@ -384,6 +385,55 @@ public void testDeletedContainer() throws Exception {
.isGreaterThan(currentTime);
}
+ @Test
+ public void testAllContainerStateInsertions() {
+ // Set up DAOs and Schema Manager
+ 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); // Ensure unique ID for each state
+ unhealthyContainer.setExpectedReplicaCount(3);
+ unhealthyContainer.setActualReplicaCount(1);
+ unhealthyContainer.setReplicaDelta(2);
+ unhealthyContainer.setContainerState(state.name());
Review Comment:
Pls fix this comment, as it creates confusion.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11309. Increase CONTAINER_STATE Column Length in UNHEALTHY_CONTAINERS to Avoid Truncation [ozone]
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #7071:
URL: https://github.com/apache/ozone/pull/7071#discussion_r1722787460
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java:
##########
@@ -384,6 +386,92 @@ 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:
Review Comment:
Thanks implemented
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java:
##########
@@ -384,6 +386,92 @@ 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:
Review Comment:
Thanks implemented
@devmadhuu
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org