Skip to content

Commit 0ad9642

Browse files
committed
fix loop when updating reposity and creating snapshot
Signed-off-by: kkewwei <[email protected]> Signed-off-by: kkewwei <[email protected]>
1 parent 1acba95 commit 0ad9642

File tree

3 files changed

+74
-0
lines changed

3 files changed

+74
-0
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
3131

3232
### Fixed
3333
- Fix bytes parameter on `_cat/recovery` ([#17598](https://github.com/opensearch-project/OpenSearch/pull/17598))
34+
- Fix simultaneously creating a snapshot and updating the repository can potentially trigger an infinite loop ([#17532](https://github.com/opensearch-project/OpenSearch/pull/17532))
3435

3536
### Security
3637

server/src/internalClusterTest/java/org/opensearch/repositories/RepositoriesServiceIT.java

+66
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,12 @@
4545
import java.util.Collection;
4646
import java.util.Collections;
4747

48+
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
49+
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
50+
import static org.hamcrest.Matchers.containsString;
4851
import static org.hamcrest.Matchers.equalTo;
4952
import static org.hamcrest.Matchers.hasSize;
53+
import static org.hamcrest.Matchers.hasToString;
5054
import static org.hamcrest.Matchers.instanceOf;
5155
import static org.hamcrest.Matchers.not;
5256
import static org.hamcrest.Matchers.sameInstance;
@@ -122,4 +126,66 @@ public void testSystemRepositoryCantBeCreated() {
122126

123127
assertThrows(RepositoryException.class, () -> createRepository(repositoryName, FsRepository.TYPE, repoSettings));
124128
}
129+
130+
public void testCreatSnapAndUpdateReposityCauseInfiniteLoop() throws InterruptedException {
131+
// create index
132+
internalCluster();
133+
String indexName = "test-index";
134+
createIndex(indexName, Settings.builder().put(SETTING_NUMBER_OF_REPLICAS, 0).put(SETTING_NUMBER_OF_SHARDS, 1).build());
135+
index(indexName, "_doc", "1", Collections.singletonMap("user", generateRandomStringArray(1, 10, false, false)));
136+
flush(indexName);
137+
138+
// create repository
139+
final String repositoryName = "test-repo";
140+
Settings.Builder repoSettings = Settings.builder()
141+
.put("location", randomRepoPath())
142+
.put("max_snapshot_bytes_per_sec", "10mb")
143+
.put("max_restore_bytes_per_sec", "10mb");
144+
OpenSearchIntegTestCase.putRepositoryWithNoSettingOverrides(
145+
client().admin().cluster(),
146+
repositoryName,
147+
FsRepository.TYPE,
148+
true,
149+
repoSettings
150+
);
151+
152+
String snapshotName = "test-snapshot";
153+
Runnable createSnapshot = () -> {
154+
logger.info("--> begining snapshot");
155+
client().admin()
156+
.cluster()
157+
.prepareCreateSnapshot(repositoryName, snapshotName)
158+
.setWaitForCompletion(true)
159+
.setIndices(indexName)
160+
.get();
161+
logger.info("--> finishing snapshot");
162+
};
163+
164+
// snapshot mab be failed when updating repository
165+
Thread thread = new Thread(() -> {
166+
try {
167+
createSnapshot.run();
168+
} catch (Exception e) {
169+
assertThat(e, instanceOf(RepositoryException.class));
170+
assertThat(e, hasToString(containsString(("the repository is closed"))));
171+
}
172+
});
173+
thread.start();
174+
175+
logger.info("--> begin to reset repository");
176+
repoSettings = Settings.builder().put("location", randomRepoPath()).put("max_snapshot_bytes_per_sec", "300mb");
177+
OpenSearchIntegTestCase.putRepositoryWithNoSettingOverrides(
178+
client().admin().cluster(),
179+
repositoryName,
180+
FsRepository.TYPE,
181+
true,
182+
repoSettings
183+
);
184+
logger.info("--> finish to reset repository");
185+
186+
// after updating repository, snapshot should be success
187+
createSnapshot.run();
188+
189+
thread.join();
190+
}
125191
}

server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java

+7
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,8 @@ protected static long calculateMaxWithinIntLimit(long defaultThresholdOfHeap, lo
551551
*/
552552
protected volatile int bufferSize;
553553

554+
private volatile boolean closed;
555+
554556
/**
555557
* Constructs new BlobStoreRepository
556558
* @param repositoryMetadata The metadata for this repository including name and settings
@@ -630,6 +632,7 @@ protected void doClose() {
630632
}
631633
if (store != null) {
632634
try {
635+
closed = true;
633636
store.close();
634637
} catch (Exception t) {
635638
logger.warn("cannot close blob store", t);
@@ -643,6 +646,10 @@ public void executeConsistentStateUpdate(
643646
String source,
644647
Consumer<Exception> onFailure
645648
) {
649+
if (this.closed) {
650+
onFailure.accept(new RepositoryException(metadata.name(), "the repository is closed, maybe updated or deleted, please retry"));
651+
return;
652+
}
646653
final RepositoryMetadata repositoryMetadataStart = metadata;
647654
getRepositoryData(ActionListener.wrap(repositoryData -> {
648655
final ClusterStateUpdateTask updateTask = createUpdateTask.apply(repositoryData);

0 commit comments

Comments
 (0)