ArafatKhan2198 commented on code in PR #6987:
URL: https://github.com/apache/ozone/pull/6987#discussion_r1694603318
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/OzoneManagerServiceProviderImpl.java:
##########
@@ -623,6 +639,34 @@ public boolean syncDataFromOM() {
return true;
}
+ private void printFileAndKeyTableCount() throws IOException {
+ Table fileTable = omMetadataManager.getTable("fileTable");
+ Table keyTable = omMetadataManager.getTable("keyTable");
+ if (keyTable == null) {
+ LOG.error("Table keyTable not found in OM Metadata.");
+ }
+
+ if (LOG.isDebugEnabled() && null != keyTable) {
+ try (TableIterator<String, ? extends Table.KeyValue<String, ?>> iterator
+ = keyTable.iterator()) {
+ long count = Iterators.size(iterator);
+ LOG.debug("keyTable Table count: {}", count);
Review Comment:
We can combine null checks and debug log conditions to avoid repetitive
code, and also write it in such a way that we can add any other table in it in
the future if necessary.
```
private void printFileAndKeyTableCount() throws IOException {
printTableCount("fileTable");
printTableCount("keyTable");
}
private void printTableCount(String tableName) throws IOException {
Table table = omMetadataManager.getTable(tableName);
if (table == null) {
LOG.error("Table {} not found in OM Metadata.", tableName);
return;
}
if (LOG.isDebugEnabled()) {
try (TableIterator<String, ? extends Table.KeyValue<String, ?>>
iterator = table.iterator()) {
long count = Iterators.size(iterator);
LOG.debug("{} Table count: {}", tableName, count);
}
}
}
```
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/OzoneManagerServiceProviderImpl.java:
##########
@@ -539,7 +554,7 @@ boolean innerGetAndApplyDeltaUpdatesFromOM(long
fromSequenceNumber,
* full snapshot from Ozone Manager.
*/
@VisibleForTesting
- public boolean syncDataFromOM() {
+ public boolean syncDataFromOM() throws IOException {
Review Comment:
Mention in the JavaDoc that it throws an `IOException`.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/OzoneManagerServiceProviderImpl.java:
##########
@@ -260,6 +263,18 @@ public void start() {
omMetadataManager.start(configuration);
} catch (IOException ioEx) {
LOG.error("Error starting Recon OM Metadata Manager.", ioEx);
+ } catch (RuntimeException runtimeException) {
+ LOG.warn("Unexpected runtime error starting Recon OM Metadata Manager.",
runtimeException);
+ LOG.warn("Trying to delete existing recon OM snapshot DB and fetch new
one.");
+ metrics.incrNumSnapshotRequests();
+ LOG.info("Fetching full snapshot from Ozone Manager");
+ // Update local Recon OM DB to new snapshot.
+ try {
+ boolean success = updateReconOmDBWithNewSnapshot();
+ LOG.info("Fetched full new snapshot from Ozone Manager: {}", success);
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
}
reconTaskController.start();
Review Comment:
Can we add return statements or flags to indicate whether the start was
successful or if it needs further attention.
```
if (startSuccessful) {
reconTaskController.start();
long initialDelay = configuration.getTimeDuration(
OZONE_RECON_OM_SNAPSHOT_TASK_INITIAL_DELAY,
configuration.get(
ReconServerConfigKeys.RECON_OM_SNAPSHOT_TASK_INITIAL_DELAY,
OZONE_RECON_OM_SNAPSHOT_TASK_INITIAL_DELAY_DEFAULT),
TimeUnit.MILLISECONDS);
startSyncDataFromOM(initialDelay);
} else {
LOG.warn("Recon start was not fully successful. Check logs for
details.");
}
```
--
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]