From 2de97cc224334bb6ed378bab974f54e68c84266c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 26 Jul 2023 08:20:12 +0000 Subject: [PATCH] Fix testReplicaHasDiffFilesThanPrimary. (#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 (cherry picked from commit e1a41255de203bc57c1e29469a9bb5d50fe0d84b) Signed-off-by: github-actions[bot] --- .../main/java/org/opensearch/index/shard/IndexShard.java | 8 ++++++++ .../index/shard/RemoteStoreRefreshListener.java | 5 ++--- 2 files changed, 10 insertions(+), 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 index d0885301c572a..f24484fac86ec 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -4814,6 +4814,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 index 4a402ef397759..a79747038b9ae 100644 --- a/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java +++ b/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java @@ -459,11 +459,10 @@ private void updateLocalSizeMapAndTracker(Collection 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();