devmadhuu commented on code in PR #7213:
URL: https://github.com/apache/ozone/pull/7213#discussion_r1770826428
##########
hadoop-ozone/recon-codegen/src/main/java/org/hadoop/ozone/recon/schema/ContainerSchemaDefinition.java:
##########
@@ -71,25 +76,35 @@ public void initializeSchema() throws SQLException {
Connection conn = dataSource.getConnection();
dslContext = DSL.using(conn);
- if (TABLE_EXISTS_CHECK.test(conn, UNHEALTHY_CONTAINERS_TABLE_NAME)) {
- // Drop the existing constraint if it exists
- String constraintName = UNHEALTHY_CONTAINERS_TABLE_NAME + "ck1";
- dslContext.alterTable(UNHEALTHY_CONTAINERS_TABLE_NAME)
- .dropConstraint(constraintName)
- .execute();
-
- // Add the updated constraint with all enum states
- addUpdatedConstraint();
- } else {
- // Create the table if it does not exist
+ if (!TABLE_EXISTS_CHECK.test(conn, UNHEALTHY_CONTAINERS_TABLE_NAME)) {
createUnhealthyContainersTable();
}
}
+ @Override
+ public void upgradeSchema(String fromVersion, String toVersion)
+ throws SQLException {
+ Connection conn = dataSource.getConnection();
+ if (!TABLE_EXISTS_CHECK.test(conn, UNHEALTHY_CONTAINERS_TABLE_NAME)) {
+ return;
+ }
+ // Example upgrade script
+ if (fromVersion.equals("1.0") && toVersion.equals("2.0")) {
+ runMigrationToVersion2(conn);
+ LOG.info("Upgraded schema from version 1.0 to 2.0.");
+ }
+ }
+
/**
- * Add the updated constraint to the table.
+ * Run the upgrade to version 2.0.
*/
- private void addUpdatedConstraint() {
+ private void runMigrationToVersion2(Connection conn) throws SQLException {
Review Comment:
IMO, we should keep this method name more generic because versions will keep
on increasing.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconSchemaManager.java:
##########
@@ -29,29 +30,128 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.inject.Inject;
+import static org.hadoop.ozone.recon.codegen.SqlDbUtils.TABLE_EXISTS_CHECK;
+
/**
- * Class used to create Recon SQL tables.
+ * Manages the creation and upgrade of Recon SQL tables with schema versioning.
+ *
+ * This class handles the following scenarios:
+ *
+ * 1. Fresh Installation:
+ * - No tables, including `schemaVersionTable`, exist.
+ * - All tables are created with the latest schema version.
+ *
+ * 2. Upgrade from Older Version:
+ * - All Existing tables (e.g., `UNHEALTHY_CONTAINERS`) are present, but
`schemaVersionTable` is missing.
+ * - Indicates an upgrade from a version without schema tracking which was
introduced in version 2.0.
+ * - The `schemaVersionTable` is created and all tables are upgraded to the
latest version.
+ *
+ * 3. Upgrade with SchemaVersionTable:
+ * - `schemaVersionTable` exists but is outdated.
+ * - Migrations are applied to upgrade all tables to the latest schema.
+ *
+ * 4. Schema Already Up to Date:
+ * - All tables and the schema version match the latest version; no action
is needed.
*/
public class ReconSchemaManager {
private static final Logger LOG =
LoggerFactory.getLogger(ReconSchemaManager.class);
private Set<ReconSchemaDefinition> reconSchemaDefinitions = new HashSet<>();
+ private final ReconSchemaVersionTableManager schemaVersionTableManager;
@Inject
- public ReconSchemaManager(Set<ReconSchemaDefinition> reconSchemaDefinitions)
{
+ public ReconSchemaManager(Set<ReconSchemaDefinition> reconSchemaDefinitions,
+ ReconSchemaVersionTableManager
schemaVersionTableManager) {
+ this.schemaVersionTableManager = schemaVersionTableManager;
this.reconSchemaDefinitions.addAll(reconSchemaDefinitions);
}
@VisibleForTesting
- public void createReconSchema() {
- reconSchemaDefinitions.forEach(reconSchemaDefinition -> {
+ public void createAndUpgradeReconSchema() {
+
+ boolean isUpgrade = areOtherTablesExisting();
+
+ // Initialize all tables
+ initializeAllSchemas();
+
+ // Fetch current version from the SchemaVersionTable
+ String currentVersion =
schemaVersionTableManager.getCurrentSchemaVersion();
+ String latestVersion = ReconConstants.LATEST_SCHEMA_VERSION;
+
+ // Handle cases where currentVersion is null, indicating
schemaVersionTable just got created
+ if (currentVersion == null) {
+ if (isUpgrade) {
+ // Case 1: Upgrade from older version where schemaVersionTable was not
present
+ LOG.info("Upgrade from older version detected. Setting current schema
version to 1.0.");
+ currentVersion = "1.0"; // Set current version to the previous version
before schemaVersionTable was introduced
+ } else {
+ // Case 2: Fresh install
+ LOG.info("Fresh installation detected. Setting schema version to
latest.");
+ currentVersion = latestVersion;
+ }
+ }
+
+ // Upgrade schema if necessary
+ if (!currentVersion.equals(latestVersion)) {
Review Comment:
So as of now in this version, we are handling 2 cases:
1. Fresh install (version will be set as `2.0`)
2. Old version with no schema version table (current version set as `1.0`)
and in new version `v2.0`, we are only updating the schema of
`UNHEALTHY_CONTAINERS` table schema and not the schema of other derby tables ,
so isn't good if we also maintain the list of tables and details in
`schemaversion` table whose schema was updated in that respective version.
##########
hadoop-ozone/recon-codegen/src/main/java/org/hadoop/ozone/recon/schema/ContainerSchemaDefinition.java:
##########
@@ -71,25 +76,35 @@ public void initializeSchema() throws SQLException {
Connection conn = dataSource.getConnection();
dslContext = DSL.using(conn);
- if (TABLE_EXISTS_CHECK.test(conn, UNHEALTHY_CONTAINERS_TABLE_NAME)) {
- // Drop the existing constraint if it exists
- String constraintName = UNHEALTHY_CONTAINERS_TABLE_NAME + "ck1";
- dslContext.alterTable(UNHEALTHY_CONTAINERS_TABLE_NAME)
- .dropConstraint(constraintName)
- .execute();
-
- // Add the updated constraint with all enum states
- addUpdatedConstraint();
- } else {
- // Create the table if it does not exist
+ if (!TABLE_EXISTS_CHECK.test(conn, UNHEALTHY_CONTAINERS_TABLE_NAME)) {
createUnhealthyContainersTable();
}
}
+ @Override
+ public void upgradeSchema(String fromVersion, String toVersion)
+ throws SQLException {
+ Connection conn = dataSource.getConnection();
+ if (!TABLE_EXISTS_CHECK.test(conn, UNHEALTHY_CONTAINERS_TABLE_NAME)) {
+ return;
+ }
+ // Example upgrade script
+ if (fromVersion.equals("1.0") && toVersion.equals("2.0")) {
Review Comment:
If I have understood correctly, this is just a version upgrade condition for
current version when we are on 1.0 and from current to this new v2.0 will be
upgraded and we know what schema and other things are getting changed. Is that
correct ?
##########
hadoop-ozone/recon-codegen/src/main/java/org/hadoop/ozone/recon/schema/StatsSchemaDefinition.java:
##########
@@ -54,6 +54,12 @@ public void initializeSchema() throws SQLException {
}
}
+ @Override
+ public void upgradeSchema(String fromVersion, String toVersion)
+ throws SQLException {
+ // No schema upgrades needed for the stats table.
Review Comment:
Can we put a log that no schema version upgraded for this XYZ table in this
version ?
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/persistence/AbstractReconSqlDBTest.java:
##########
@@ -115,7 +115,7 @@ protected void configure() {
public void createSchema(Injector inj) {
ReconSchemaManager reconSchemaManager =
inj.getInstance(ReconSchemaManager.class);
- reconSchemaManager.createReconSchema();
+ reconSchemaManager.createAndUpgradeReconSchema();
Review Comment:
Can we add some more test cases related to error and failure and assert when
and all schema version gets inserted/ updated in schema version table ?
##########
hadoop-ozone/recon-codegen/src/main/java/org/hadoop/ozone/recon/schema/ContainerSchemaDefinition.java:
##########
@@ -71,25 +76,35 @@ public void initializeSchema() throws SQLException {
Connection conn = dataSource.getConnection();
dslContext = DSL.using(conn);
- if (TABLE_EXISTS_CHECK.test(conn, UNHEALTHY_CONTAINERS_TABLE_NAME)) {
- // Drop the existing constraint if it exists
- String constraintName = UNHEALTHY_CONTAINERS_TABLE_NAME + "ck1";
- dslContext.alterTable(UNHEALTHY_CONTAINERS_TABLE_NAME)
- .dropConstraint(constraintName)
- .execute();
-
- // Add the updated constraint with all enum states
- addUpdatedConstraint();
- } else {
- // Create the table if it does not exist
+ if (!TABLE_EXISTS_CHECK.test(conn, UNHEALTHY_CONTAINERS_TABLE_NAME)) {
createUnhealthyContainersTable();
}
}
+ @Override
+ public void upgradeSchema(String fromVersion, String toVersion)
+ throws SQLException {
+ Connection conn = dataSource.getConnection();
+ if (!TABLE_EXISTS_CHECK.test(conn, UNHEALTHY_CONTAINERS_TABLE_NAME)) {
Review Comment:
In what use case, this condition can happen ? As I can see that we are
calling `upgradeSchema` only after initialization of all schemas ?
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconSchemaVersionTableManager.java:
##########
@@ -0,0 +1,68 @@
+package org.apache.hadoop.ozone.recon;
+
+import com.google.inject.Inject;
+import org.jooq.DSLContext;
+import org.jooq.impl.DSL;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import javax.sql.DataSource;
+import java.sql.SQLException;
+
+import static org.jooq.impl.DSL.name;
+
+public class ReconSchemaVersionTableManager {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(ReconSchemaVersionTableManager.class);
+ public static final String RECON_SCHEMA_VERSION_TABLE_NAME =
"RECON_SCHEMA_VERSION";
+ private final DSLContext dslContext;
+ private final DataSource dataSource;
+
+ @Inject
+ public ReconSchemaVersionTableManager(DataSource src) throws
+ SQLException {
+ this.dataSource = src;
+ this.dslContext = DSL.using(dataSource.getConnection());
+ }
+
+ /**
+ * Get the current schema version stored in the RECON_SCHEMA_VERSION_TABLE.
+ *
+ * @return The current schema version as a String, or null if no entry
exists.
+ * @throws SQLException if any SQL error occurs.
+ */
+ public String getCurrentSchemaVersion() {
+ return dslContext.select(DSL.field(name("version_number")))
+ .from(RECON_SCHEMA_VERSION_TABLE_NAME)
+ .fetchOneInto(String.class); // Return the version number or null if
no entry exists
+ }
+
+ /**
+ * Update the schema version in the RECON_SCHEMA_VERSION_TABLE after all
tables are upgraded.
Review Comment:
This comment doesn't justify the error scenarios if any schema upgrade fails.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconSchemaManager.java:
##########
@@ -29,29 +30,128 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.inject.Inject;
+import static org.hadoop.ozone.recon.codegen.SqlDbUtils.TABLE_EXISTS_CHECK;
+
/**
- * Class used to create Recon SQL tables.
+ * Manages the creation and upgrade of Recon SQL tables with schema versioning.
+ *
+ * This class handles the following scenarios:
+ *
+ * 1. Fresh Installation:
+ * - No tables, including `schemaVersionTable`, exist.
+ * - All tables are created with the latest schema version.
+ *
+ * 2. Upgrade from Older Version:
+ * - All Existing tables (e.g., `UNHEALTHY_CONTAINERS`) are present, but
`schemaVersionTable` is missing.
+ * - Indicates an upgrade from a version without schema tracking which was
introduced in version 2.0.
+ * - The `schemaVersionTable` is created and all tables are upgraded to the
latest version.
+ *
+ * 3. Upgrade with SchemaVersionTable:
+ * - `schemaVersionTable` exists but is outdated.
+ * - Migrations are applied to upgrade all tables to the latest schema.
+ *
+ * 4. Schema Already Up to Date:
+ * - All tables and the schema version match the latest version; no action
is needed.
*/
public class ReconSchemaManager {
private static final Logger LOG =
LoggerFactory.getLogger(ReconSchemaManager.class);
private Set<ReconSchemaDefinition> reconSchemaDefinitions = new HashSet<>();
+ private final ReconSchemaVersionTableManager schemaVersionTableManager;
@Inject
- public ReconSchemaManager(Set<ReconSchemaDefinition> reconSchemaDefinitions)
{
+ public ReconSchemaManager(Set<ReconSchemaDefinition> reconSchemaDefinitions,
+ ReconSchemaVersionTableManager
schemaVersionTableManager) {
+ this.schemaVersionTableManager = schemaVersionTableManager;
this.reconSchemaDefinitions.addAll(reconSchemaDefinitions);
}
@VisibleForTesting
- public void createReconSchema() {
- reconSchemaDefinitions.forEach(reconSchemaDefinition -> {
+ public void createAndUpgradeReconSchema() {
+
+ boolean isUpgrade = areOtherTablesExisting();
+
+ // Initialize all tables
+ initializeAllSchemas();
+
+ // Fetch current version from the SchemaVersionTable
+ String currentVersion =
schemaVersionTableManager.getCurrentSchemaVersion();
+ String latestVersion = ReconConstants.LATEST_SCHEMA_VERSION;
+
+ // Handle cases where currentVersion is null, indicating
schemaVersionTable just got created
+ if (currentVersion == null) {
+ if (isUpgrade) {
+ // Case 1: Upgrade from older version where schemaVersionTable was not
present
+ LOG.info("Upgrade from older version detected. Setting current schema
version to 1.0.");
+ currentVersion = "1.0"; // Set current version to the previous version
before schemaVersionTable was introduced
+ } else {
+ // Case 2: Fresh install
+ LOG.info("Fresh installation detected. Setting schema version to
latest.");
+ currentVersion = latestVersion;
+ }
+ }
+
+ // Upgrade schema if necessary
+ if (!currentVersion.equals(latestVersion)) {
+ upgradeAllSchemas(currentVersion, latestVersion);
+ // Update the schema version in the version table after migration
+ schemaVersionTableManager.updateSchemaVersion(latestVersion);
Review Comment:
This seems incorrect here, we are updating the schema version in
schemaVersionTable even if we get some error while upgrading the schema of a
required table ? How are we handling failure cases.
--
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]
