Skip to content

Commit 27c539d

Browse files
sachinpkaleSachin Kale
and
Sachin Kale
committed
Make sure IndexOutput of metadata file is always closed (opensearch-project#8653)
--------- Signed-off-by: Sachin Kale <kalsac@amazon.com> Co-authored-by: Sachin Kale <kalsac@amazon.com>
1 parent 54eef1f commit 27c539d

File tree

3 files changed

+46
-24
lines changed

3 files changed

+46
-24
lines changed

server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java

+24-22
Original file line numberDiff line numberDiff line change
@@ -475,30 +475,32 @@ public void uploadMetadata(
475475
RemoteSegmentMetadata.CURRENT_VERSION
476476
);
477477
try {
478-
IndexOutput indexOutput = storeDirectory.createOutput(metadataFilename, IOContext.DEFAULT);
479-
Map<String, String> uploadedSegments = new HashMap<>();
480-
for (String file : segmentFiles) {
481-
if (segmentsUploadedToRemoteStore.containsKey(file)) {
482-
uploadedSegments.put(file, segmentsUploadedToRemoteStore.get(file).toString());
483-
} else {
484-
throw new NoSuchFileException(file);
478+
try (IndexOutput indexOutput = storeDirectory.createOutput(metadataFilename, IOContext.DEFAULT)) {
479+
Map<String, String> uploadedSegments = new HashMap<>();
480+
for (String file : segmentFiles) {
481+
if (segmentsUploadedToRemoteStore.containsKey(file)) {
482+
uploadedSegments.put(file, segmentsUploadedToRemoteStore.get(file).toString());
483+
} else {
484+
throw new NoSuchFileException(file);
485+
}
485486
}
486-
}
487487

488-
ByteBuffersDataOutput byteBuffersIndexOutput = new ByteBuffersDataOutput();
489-
segmentInfosSnapshot.write(new ByteBuffersIndexOutput(byteBuffersIndexOutput, "Snapshot of SegmentInfos", "SegmentInfos"));
490-
byte[] segmentInfoSnapshotByteArray = byteBuffersIndexOutput.toArrayCopy();
491-
492-
metadataStreamWrapper.writeStream(
493-
indexOutput,
494-
new RemoteSegmentMetadata(
495-
RemoteSegmentMetadata.fromMapOfStrings(uploadedSegments),
496-
segmentInfoSnapshotByteArray,
497-
primaryTerm,
498-
segmentInfosSnapshot.getGeneration()
499-
)
500-
);
501-
indexOutput.close();
488+
ByteBuffersDataOutput byteBuffersIndexOutput = new ByteBuffersDataOutput();
489+
segmentInfosSnapshot.write(
490+
new ByteBuffersIndexOutput(byteBuffersIndexOutput, "Snapshot of SegmentInfos", "SegmentInfos")
491+
);
492+
byte[] segmentInfoSnapshotByteArray = byteBuffersIndexOutput.toArrayCopy();
493+
494+
metadataStreamWrapper.writeStream(
495+
indexOutput,
496+
new RemoteSegmentMetadata(
497+
RemoteSegmentMetadata.fromMapOfStrings(uploadedSegments),
498+
segmentInfoSnapshotByteArray,
499+
primaryTerm,
500+
segmentInfosSnapshot.getGeneration()
501+
)
502+
);
503+
}
502504
storeDirectory.sync(Collections.singleton(metadataFilename));
503505
remoteMetadataDirectory.copyFrom(storeDirectory, metadataFilename, metadataFilename, IOContext.DEFAULT);
504506
} finally {

server/src/test/java/org/opensearch/index/shard/RemoteStoreRefreshListenerTests.java

-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import org.apache.lucene.store.Directory;
1414
import org.apache.lucene.store.FilterDirectory;
1515
import org.apache.lucene.tests.store.BaseDirectoryWrapper;
16-
import org.apache.lucene.tests.util.LuceneTestCase;
1716
import org.junit.After;
1817
import org.opensearch.action.ActionListener;
1918
import org.opensearch.cluster.metadata.IndexMetadata;
@@ -48,7 +47,6 @@
4847
import static org.mockito.Mockito.when;
4948
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
5049

51-
@LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/8638")
5250
public class RemoteStoreRefreshListenerTests extends IndexShardTestCase {
5351
private IndexShard indexShard;
5452
private ClusterService clusterService;

server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java

+22
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,28 @@ public void testUploadMetadataNonEmpty() throws IOException {
604604
}
605605
}
606606

607+
public void testUploadMetadataMissingSegment() throws IOException {
608+
populateMetadata();
609+
remoteSegmentStoreDirectory.init();
610+
611+
Directory storeDirectory = mock(Directory.class);
612+
IndexOutput indexOutput = mock(IndexOutput.class);
613+
614+
String generation = RemoteStoreUtils.invertLong(segmentInfos.getGeneration());
615+
String primaryTerm = RemoteStoreUtils.invertLong(12);
616+
when(storeDirectory.createOutput(startsWith("metadata__" + primaryTerm + "__" + generation), eq(IOContext.DEFAULT))).thenReturn(
617+
indexOutput
618+
);
619+
620+
Collection<String> segmentFiles = List.of("_123.si");
621+
assertThrows(
622+
NoSuchFileException.class,
623+
() -> remoteSegmentStoreDirectory.uploadMetadata(segmentFiles, segmentInfos, storeDirectory, 12L, 34L)
624+
);
625+
verify(indexOutput).close();
626+
verify(storeDirectory).deleteFile(startsWith("metadata__" + primaryTerm + "__" + generation));
627+
}
628+
607629
public void testNoMetadataHeaderCorruptIndexException() throws IOException {
608630
List<String> metadataFiles = List.of(metadataFilename);
609631
when(

0 commit comments

Comments
 (0)