Skip to content

Commit e1a4125

Browse files
authored
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 <[email protected]>
1 parent 1b5e64a commit e1a4125

File tree

2 files changed

+10
-3
lines changed

2 files changed

+10
-3
lines changed

server/src/main/java/org/opensearch/index/shard/IndexShard.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4808,6 +4808,14 @@ private boolean localDirectoryContains(Directory localDirectory, String file, lo
48084808
return true;
48094809
} else {
48104810
logger.warn("Checksum mismatch between local and remote segment file: {}, will override local file", file);
4811+
// If there is a checksum mismatch and we are not serving reads it is safe to go ahead and delete the file now.
4812+
// Outside of engine resets this method will be invoked during recovery so this is safe.
4813+
if (isReadAllowed() == false) {
4814+
localDirectory.deleteFile(file);
4815+
} else {
4816+
// segment conflict with remote store while the shard is serving reads.
4817+
failShard("Local copy of segment " + file + " has a different checksum than the version in remote store", null);
4818+
}
48114819
}
48124820
} catch (NoSuchFileException | FileNotFoundException e) {
48134821
logger.debug("File {} does not exist in local FS, downloading from remote store", file);

server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -457,11 +457,10 @@ private void updateLocalSizeMapAndTracker(Collection<String> segmentFiles) {
457457
private void updateFinalStatusInSegmentTracker(boolean uploadStatus, long bytesBeforeUpload, long startTimeInNS) {
458458
if (uploadStatus) {
459459
long bytesUploaded = segmentTracker.getUploadBytesSucceeded() - bytesBeforeUpload;
460-
long timeTakenInMS = (System.nanoTime() - startTimeInNS) / 1_000_000L;
461-
460+
long timeTakenInMS = TimeValue.nsecToMSec(System.nanoTime() - startTimeInNS);
462461
segmentTracker.incrementTotalUploadsSucceeded();
463462
segmentTracker.addUploadBytes(bytesUploaded);
464-
segmentTracker.addUploadBytesPerSec((bytesUploaded * 1_000L) / timeTakenInMS);
463+
segmentTracker.addUploadBytesPerSec((bytesUploaded * 1_000L) / Math.max(1, timeTakenInMS));
465464
segmentTracker.addUploadTimeMs(timeTakenInMS);
466465
} else {
467466
segmentTracker.incrementTotalUploadsFailed();

0 commit comments

Comments
 (0)