Fix testReplicaHasDiffFilesThanPrimary. (opensearch-project#8863)

This test is failing in two ways. First it fails when copying segments from the remote store and there is a cksum mismatch. In this case it is not guaranteed the directory implementation will replace the existing file when copying from the store. This change ensures the mismatched file is cleaned up but only if the shard is not serving reads. In that case we fail the shard so it is re-recovered rather than deleting the segment underneath it. This test also fails with a divide by 0 in RemoteStoreRefreshListener. Signed-off-by: Marc Handalian <[email protected]>
baba-devv · Jul 29, 2023 · ac8f553 · ac8f553
1 parent efebd02
commit ac8f553
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 3 deletions.
diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java
@@ -4808,6 +4808,14 @@ private boolean localDirectoryContains(Directory localDirectory, String file, lo
                 return true;
             } else {
                 logger.warn("Checksum mismatch between local and remote segment file: {}, will override local file", file);
+                // If there is a checksum mismatch and we are not serving reads it is safe to go ahead and delete the file now.
+                // Outside of engine resets this method will be invoked during recovery so this is safe.
+                if (isReadAllowed() == false) {
+                    localDirectory.deleteFile(file);
+                } else {
+                    // segment conflict with remote store while the shard is serving reads.
+                    failShard("Local copy of segment " + file + " has a different checksum than the version in remote store", null);
+                }
             }
         } catch (NoSuchFileException | FileNotFoundException e) {
             logger.debug("File {} does not exist in local FS, downloading from remote store", file);

diff --git a/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java b/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java
@@ -457,11 +457,10 @@ private void updateLocalSizeMapAndTracker(Collection<String> segmentFiles) {
     private void updateFinalStatusInSegmentTracker(boolean uploadStatus, long bytesBeforeUpload, long startTimeInNS) {
         if (uploadStatus) {
             long bytesUploaded = segmentTracker.getUploadBytesSucceeded() - bytesBeforeUpload;
-            long timeTakenInMS = (System.nanoTime() - startTimeInNS) / 1_000_000L;
-
+            long timeTakenInMS = TimeValue.nsecToMSec(System.nanoTime() - startTimeInNS);
             segmentTracker.incrementTotalUploadsSucceeded();
             segmentTracker.addUploadBytes(bytesUploaded);
-            segmentTracker.addUploadBytesPerSec((bytesUploaded * 1_000L) / timeTakenInMS);
+            segmentTracker.addUploadBytesPerSec((bytesUploaded * 1_000L) / Math.max(1, timeTakenInMS));
             segmentTracker.addUploadTimeMs(timeTakenInMS);
         } else {
             segmentTracker.incrementTotalUploadsFailed();