devmadhuu commented on code in PR #11560:
URL: https://github.com/apache/ozone/pull/11560#discussion_r4297099685
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/recovery/ReconOmMetadataManagerImpl.java:
##########
@@ -121,13 +125,8 @@ public void start(OzoneConfiguration configuration) throws
IOException {
* @param addCacheMetrics
*/
private void initializeNewRdbStore(File dbFile, boolean addCacheMetrics)
throws IOException {
- try {
- setStore(DBStoreBuilder.newBuilder(ozoneConfiguration,
OMDBDefinition.get(), dbFile).build());
- LOG.info("Created OM DB handle from snapshot at {}.",
- dbFile.getAbsolutePath());
- } catch (IOException ioEx) {
- LOG.error("Unable to initialize Recon OM DB snapshot store.", ioEx);
- }
+ setStore(DBStoreBuilder.newBuilder(ozoneConfiguration,
OMDBDefinition.get(), dbFile).build());
+ LOG.info("Created OM DB handle from snapshot at {}.",
dbFile.getAbsolutePath());
if (getStore() != null) {
initializeOmTables(TableCache.CacheType.FULL_CACHE, addCacheMetrics);
Review Comment:
Failure can happen here or in `setStore` call, its not differentiated. So
lets say if `setStore` call is succeeded, but `initializeOmTables` throw error,
then `current == getStore()` condition will be false and recon has switched to
new OM DB snapshot, but since initialize table call failed, some API calls for
recon will continue to read stale data silently without knowing. Also dangerous
part is that subsequent all delta operations will continue to process from that
last updated sequence number and will update further events and updates to OM
tables pointing to old DB tables as `initializeOmTables` call was failed. This
also will happen silently.
If build() succeeds and initializeOmTables then throws:
- store already points to the new DB, so current == getStore() is false and
the new folder is not deleted.
- Only some table fields were reassigned before the throw, so tableMap holds
a mix of new-DB and old-DB tables.
- finally closes the old store, so the table fields that still point to it
now fail with "Rocks Database is closed".
- omTablesInitialized stays true.
- The refresh is reported as failed, so no reinit is queued. The task tables
stay at the old sequence number while the OM DB copy is at the new one. Later
deltas move both forward together, so the startup sequence-number check does
not catch the gap either.
--
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]
