ArafatKhan2198 commented on code in PR #7554:
URL: https://github.com/apache/ozone/pull/7554#discussion_r1883538694
##########
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:
The code was originally written without throwing exceptions, so I followed
the same flow. However, if we decide to throw exceptions now, we would need to
define a new `ErrorCode` for the Recon context, pass it to the
`ReconSchemaManager`, and set the error context accordingly.
That said, throwing exceptions could stop the Recon server entirely. Is it
appropriate to halt the server just because a single table isn't created?
We could address the table initialization issue later without stopping
Recon. Shouldn't we allow the server to continue running despite the issue with
one table?
@sumitagrawl @devmadhuu
##########
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:
Done!
--
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]
