From 82a1d83aaad072ddb7fe226fe3b3d7ffceedad96 Mon Sep 17 00:00:00 2001
From: Ashish <ssashish@amazon.com>
Date: Wed, 12 Jul 2023 06:41:04 +0530
Subject: [PATCH] Fix remote segments sync retry regression introduced in PR
 #7119 (#8632)

---------

Signed-off-by: Ashish Singh <ssashish@amazon.com>
Signed-off-by: Sachin Kale <kalsac@amazon.com>
Co-authored-by: Sachin Kale <kalsac@amazon.com>
Signed-off-by: Ashish Singh <ssashish@amazon.com>
---
 .../shard/RemoteStoreRefreshListener.java      | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java b/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java
index 6b15ce36266b9..e37303ad3e323 100644
--- a/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java
+++ b/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java
@@ -201,9 +201,9 @@ private synchronized void syncSegments(boolean isRetry) {
         long refreshTimeMs = segmentTracker.getLocalRefreshTimeMs(), refreshClockTimeMs = segmentTracker.getLocalRefreshClockTimeMs();
         long refreshSeqNo = segmentTracker.getLocalRefreshSeqNo();
         long bytesBeforeUpload = segmentTracker.getUploadBytesSucceeded(), startTimeInNS = System.nanoTime();
+        final AtomicBoolean shouldRetry = new AtomicBoolean(true);
 
         try {
-
             if (this.primaryTerm != indexShard.getOperationPrimaryTerm()) {
                 this.primaryTerm = indexShard.getOperationPrimaryTerm();
                 this.remoteDirectory.init();
@@ -247,7 +247,6 @@ private synchronized void syncSegments(boolean isRetry) {
                         ActionListener<Void> segmentUploadsCompletedListener = new LatchedActionListener<>(new ActionListener<>() {
                             @Override
                             public void onResponse(Void unused) {
-                                boolean shouldRetry = true;
                                 try {
                                     // Start metadata file upload
                                     uploadMetadata(localSegmentsPostRefresh, segmentInfos);
@@ -261,27 +260,18 @@ public void onResponse(Void unused) {
                                     );
                                     // At this point since we have uploaded new segments, segment infos and segment metadata file,
                                     // along with marking minSeqNoToKeep, upload has succeeded completely.
-                                    shouldRetry = false;
+                                    shouldRetry.set(false);
                                 } catch (Exception e) {
                                     // We don't want to fail refresh if upload of new segments fails. The missed segments will be re-tried
                                     // in the next refresh. This should not affect durability of the indexed data after remote trans-log
                                     // integration.
                                     logger.warn("Exception in post new segment upload actions", e);
-                                } finally {
-                                    doComplete(shouldRetry);
                                 }
                             }
 
                             @Override
                             public void onFailure(Exception e) {
                                 logger.warn("Exception while uploading new segments to the remote segment store", e);
-                                doComplete(true);
-                            }
-
-                            private void doComplete(boolean shouldRetry) {
-                                // Update the segment tracker with the final upload status as seen at the end
-                                updateFinalUploadStatusInSegmentTracker(shouldRetry == false, bytesBeforeUpload, startTimeInNS);
-                                afterSegmentsSync(isRetry, shouldRetry);
                             }
                         }, latch);
 
@@ -300,6 +290,8 @@ private void doComplete(boolean shouldRetry) {
         } catch (Throwable t) {
             logger.error("Exception in RemoteStoreRefreshListener.afterRefresh()", t);
         }
+        updateFinalStatusInSegmentTracker(shouldRetry.get() == false, bytesBeforeUpload, startTimeInNS);
+        afterSegmentsSync(isRetry, shouldRetry.get());
     }
 
     /**
@@ -514,7 +506,7 @@ private void updateLocalSizeMapAndTracker(Collection<String> segmentFiles) {
         segmentTracker.setLatestLocalFileNameLengthMap(latestFileNameSizeOnLocalMap);
     }
 
-    private void updateFinalUploadStatusInSegmentTracker(boolean uploadStatus, long bytesBeforeUpload, long startTimeInNS) {
+    private void updateFinalStatusInSegmentTracker(boolean uploadStatus, long bytesBeforeUpload, long startTimeInNS) {
         if (uploadStatus) {
             long bytesUploaded = segmentTracker.getUploadBytesSucceeded() - bytesBeforeUpload;
             long timeTakenInMS = (System.nanoTime() - startTimeInNS) / 1_000_000L;