sumitagrawl commented on code in PR #7554:
URL: https://github.com/apache/ozone/pull/7554#discussion_r1881865396
##########
hadoop-ozone/recon-codegen/src/main/java/org/hadoop/ozone/recon/schema/SchemaVersionTableDefinition.java:
##########
@@ -37,9 +39,12 @@
@Singleton
public class SchemaVersionTableDefinition implements ReconSchemaDefinition {
+ private static final Logger LOG =
LoggerFactory.getLogger(SchemaVersionTableDefinition.class);
+
public static final String SCHEMA_VERSION_TABLE_NAME =
"RECON_SCHEMA_VERSION";
private final DataSource dataSource;
private DSLContext dslContext;
+ private int latestSLV;
Review Comment:
This field not requried, can be obtained by calling ReconLayoutFeature
method.
##########
hadoop-ozone/recon-codegen/src/main/java/org/hadoop/ozone/recon/schema/SchemaVersionTableDefinition.java:
##########
@@ -48,11 +53,20 @@ public SchemaVersionTableDefinition(DataSource dataSource) {
@Override
public void initializeSchema() throws SQLException {
- Connection conn = dataSource.getConnection();
- dslContext = DSL.using(conn);
+ try (Connection conn = dataSource.getConnection()) {
+ dslContext = DSL.using(conn);
Review Comment:
dslContext outside of try may be invalid, as conn will be closed. So having
as member variable may be risky,
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconSchemaManager.java:
##########
@@ -45,13 +49,46 @@ public ReconSchemaManager(Set<ReconSchemaDefinition>
reconSchemaDefinitions) {
@VisibleForTesting
public void createReconSchema() {
- reconSchemaDefinitions.forEach(reconSchemaDefinition -> {
- try {
- reconSchemaDefinition.initializeSchema();
- } catch (SQLException e) {
- LOG.error("Error creating Recon schema {}.",
- reconSchemaDefinition.getClass().getSimpleName(), e);
- }
- });
+ // Calculate the latest SLV from ReconLayoutFeature
+ int latestSLV = calculateLatestSLV();
+
+ try {
+ // Initialize the schema version table first
+ reconSchemaDefinitions.stream()
+ .filter(SchemaVersionTableDefinition.class::isInstance)
+ .findFirst()
+ .ifPresent(schemaDefinition -> {
+ SchemaVersionTableDefinition schemaVersionTable =
(SchemaVersionTableDefinition) schemaDefinition;
+ schemaVersionTable.setLatestSLV(latestSLV);
+ try {
+ schemaVersionTable.initializeSchema();
+ } catch (SQLException e) {
+ LOG.error("Error initializing SchemaVersionTableDefinition.", e);
+ }
+ });
+
+ // Initialize all other tables
+ reconSchemaDefinitions.stream()
+ .filter(definition -> !(definition instanceof
SchemaVersionTableDefinition))
+ .forEach(definition -> {
+ try {
+ definition.initializeSchema();
+ } catch (SQLException e) {
+ LOG.error("Error initializing schema: {}.",
definition.getClass().getSimpleName(), e);
+ }
+ });
+
+ } catch (Exception e) {
+ LOG.error("Error creating Recon schema.", e);
Review Comment:
any specific reason that exception is not thrown?
--
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]
