devmadhuu commented on code in PR #7372:
URL: https://github.com/apache/ozone/pull/7372#discussion_r1832168823
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/upgrade/InitialConstraintUpgradeAction.java:
##########
@@ -0,0 +1,89 @@
+package org.apache.hadoop.ozone.recon.upgrade;
+
+import com.google.inject.Inject;
+import org.hadoop.ozone.recon.schema.ContainerSchemaDefinition;
+import org.jooq.DSLContext;
+import org.jooq.impl.DSL;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.Arrays;
+
+import static
org.apache.hadoop.ozone.recon.upgrade.ReconLayoutFeature.INITIAL_VERSION;
+import static
org.apache.hadoop.ozone.recon.upgrade.ReconUpgradeAction.UpgradeActionType.FINALIZE;
+import static org.hadoop.ozone.recon.codegen.SqlDbUtils.TABLE_EXISTS_CHECK;
+import static
org.hadoop.ozone.recon.schema.ContainerSchemaDefinition.UNHEALTHY_CONTAINERS_TABLE_NAME;
+import static org.jooq.impl.DSL.field;
+import static org.jooq.impl.DSL.name;
+
+/**
+ * Upgrade action for the INITIAL schema version, which manages constraints
+ * for the UNHEALTHY_CONTAINERS table.
+ */
+@UpgradeActionRecon(feature = INITIAL_VERSION, type = FINALIZE)
+public class InitialConstraintUpgradeAction implements ReconUpgradeAction {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(InitialConstraintUpgradeAction.class);
+
+ private final DataSource dataSource;
+ private DSLContext dslContext;
+
+ @Inject
+ public InitialConstraintUpgradeAction(DataSource dataSource) {
+ this.dataSource = dataSource;
+ }
+
+ @Override
+ public void execute() throws SQLException {
+ try (Connection conn = dataSource.getConnection()) {
+ if (!TABLE_EXISTS_CHECK.test(conn, UNHEALTHY_CONTAINERS_TABLE_NAME)) {
+ return;
+ }
+ dslContext = DSL.using(conn);
+ // Drop the existing constraint if it exists
+ dropConstraint();
+ // Add the updated constraint with all enum states
+ addUpdatedConstraint();
+ } catch (SQLException e) {
+ throw new SQLException("Failed to execute
InitialConstraintUpgradeAction", e);
+ }
+ }
+
+ /**
+ * Drops the existing constraint from the UNHEALTHY_CONTAINERS table.
+ */
+ private void dropConstraint() {
+ String constraintName = UNHEALTHY_CONTAINERS_TABLE_NAME + "ck1";
+ dslContext.alterTable(UNHEALTHY_CONTAINERS_TABLE_NAME)
+ .dropConstraint(constraintName)
+ .execute();
+ LOG.debug("Dropped the existing constraint: {}", constraintName);
+ }
+
+ /**
+ * Adds the updated constraint directly within this class.
+ */
+ private void addUpdatedConstraint() {
Review Comment:
You can remove the unused method `addUpdatedConstraint` from
`ContainerSchemaDefinition` class.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/upgrade/ReconLayoutVersionManager.java:
##########
@@ -47,12 +48,12 @@ public class ReconLayoutVersionManager {
private int currentMLV;
public ReconLayoutVersionManager(ReconSchemaVersionTableManager
schemaVersionTableManager,
Review Comment:
Also in this class while you finalize layout features here in
`finalizeLayoutFeatures`, I found some edge case in old code when this layout
framework was developed. After calling `execute` method of each action, we are
updating the schema version in Recon SQL table. So both are in same try catch
block and either of it can fail. If `execute` is success, but updating schema
version fails, then also we are throwing the exception with log , but not
reverting the action done by `execute`. So this is half baked inconsitency
state of Recon. Pls check that part to make it robust.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/upgrade/ReconLayoutVersionManager.java:
##########
@@ -47,12 +48,12 @@ public class ReconLayoutVersionManager {
private int currentMLV;
public ReconLayoutVersionManager(ReconSchemaVersionTableManager
schemaVersionTableManager,
- ReconContext reconContext)
+ ReconContext reconContext, Injector
injector)
Review Comment:
You need not to pass Injector reference here, as injector is only used to
get instance once which you have binded in guice framework as Singleton. And
those all `ReconUpgradeAction` classes will not be used in code flow anywhere.
So impl classes of `ReconUpgradeAction` will be scanned using annotation and
you can create instance of it there itself in your `registerUpgradeActions`
scan code.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconUpgradeActionsModule.java:
##########
@@ -0,0 +1,17 @@
+package org.apache.hadoop.ozone.recon;
+
+import com.google.inject.AbstractModule;
+import com.google.inject.Singleton;
+import org.apache.hadoop.ozone.recon.upgrade.InitialConstraintUpgradeAction;
+
+/**
+ * Guice module for binding upgrade actions as singletons in Recon.
+ */
+public class ReconUpgradeActionsModule extends AbstractModule {
Review Comment:
You don't need this class because whatever action classes you will bind
here, they are already being scanned using annotation.
--
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]
