smengcl commented on code in PR #3821:
URL: https://github.com/apache/ozone/pull/3821#discussion_r999993033


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/OzoneManagerServiceProviderImpl.java:
##########
@@ -442,64 +468,75 @@ void innerGetAndApplyDeltaUpdatesFromOM(long 
fromSequenceNumber,
    * full snapshot from Ozone Manager.
    */
   @VisibleForTesting
-  public void syncDataFromOM() {
-    LOG.info("Syncing data from Ozone Manager.");
-    long currentSequenceNumber = getCurrentOMDBSequenceNumber();
-    LOG.debug("Seq number of Recon's OM DB : {}", currentSequenceNumber);
-    boolean fullSnapshot = false;
-
-    if (currentSequenceNumber <= 0) {
-      fullSnapshot = true;
-    } else {
-      try (OMDBUpdatesHandler omdbUpdatesHandler =
-               new OMDBUpdatesHandler(omMetadataManager)) {
-        LOG.info("Obtaining delta updates from Ozone Manager");
-        // Get updates from OM and apply to local Recon OM DB.
-        getAndApplyDeltaUpdatesFromOM(currentSequenceNumber,
-            omdbUpdatesHandler);
-        // Update timestamp of successful delta updates query.
-        ReconTaskStatus reconTaskStatusRecord = new ReconTaskStatus(
-            OmSnapshotTaskName.OmDeltaRequest.name(),
-                System.currentTimeMillis(), getCurrentOMDBSequenceNumber());
-        reconTaskStatusDao.update(reconTaskStatusRecord);
-
-        // Pass on DB update events to tasks that are listening.
-        reconTaskController.consumeOMEvents(new OMUpdateEventBatch(
-            omdbUpdatesHandler.getEvents()), omMetadataManager);
-      } catch (InterruptedException intEx) {
-        Thread.currentThread().interrupt();
-      } catch (Exception e) {
-        metrics.incrNumDeltaRequestsFailed();
-        LOG.warn("Unable to get and apply delta updates from OM.", e);
-        fullSnapshot = true;
-      }
+  public boolean syncDataFromOM() {
+    if (isSyncDataFromOMRunning) {
+      LOG.debug("syncDataFromOM() is already running");
+      return false;
     }
+    isSyncDataFromOMRunning = true;

Review Comment:
   uh sorry I just realized my suggested approach isn't atomic. If two threads 
are calling `syncDataFromOM()` at the same time, there is a chance that both 
threads pass the condition check and keep running the core logic.
   
   We will need to use atomic swap for `isSyncDataFromOMRunning` instead. -- Or 
just use a Java lock.



-- 
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]

Reply via email to