Skip to content

Allow freezing searchable snapshots #52653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaveCTurner merged 8 commits into from
Mar 2, 2020
Merged

Allow freezing searchable snapshots #52653

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2ca67a8
Allow freezing searchable snapshots
DaveCTurner Feb 21, 2020
2774b7f
Missed by git add
DaveCTurner Feb 21, 2020
206856a
Move to REST tests
DaveCTurner Feb 27, 2020
8e0a73a
Merge branch 'feature/searchable-snapshots' into 2020-02-21-freeze-se…
DaveCTurner Feb 27, 2020
3f0cca1
Revert
DaveCTurner Feb 27, 2020
9e3d4bf
Merge branch 'feature/searchable-snapshots' into 2020-02-21-freeze-se…
DaveCTurner Feb 27, 2020
b10dd74
AssertOK
DaveCTurner Mar 2, 2020
bc85c25
Merge branch 'feature/searchable-snapshots' into 2020-02-21-freeze-se…
DaveCTurner Mar 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ...pshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ public Map<String, DirectoryFactory> getDirectoryFactories() {

@Override
public Optional<EngineFactory> getEngineFactory(IndexSettings indexSettings) {
if (SearchableSnapshotRepository.SNAPSHOT_DIRECTORY_FACTORY_KEY.equals(INDEX_STORE_TYPE_SETTING.get(indexSettings.getSettings()))) {
if (SearchableSnapshotRepository.SNAPSHOT_DIRECTORY_FACTORY_KEY.equals(INDEX_STORE_TYPE_SETTING.get(indexSettings.getSettings()))
&& indexSettings.getSettings().getAsBoolean("index.frozen", false) == false) {
Copy link
Contributor Author

@DaveCTurner DaveCTurner Feb 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about introducing this string literal here, vs adding a dependency on the frozen-indices module and referring to the setting directly.

Copy link
Contributor

@ywelsch ywelsch Feb 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to avoid the dependency and just use the String literal (we have other such cases in our code base).

I also wonder if we need to handle closed replicated snapshot indices (i.e. use NoOpEngine for those).

Copy link
Contributor Author

@DaveCTurner DaveCTurner Feb 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already bypass this mechanism for closed indices:

elasticsearch/server/src/main/java/org/elasticsearch/indices/IndicesService.java

Lines 622 to 627 in 0d1e67d

private EngineFactory getEngineFactory(final IndexSettings idxSettings) {
final IndexMetaData indexMetaData = idxSettings.getIndexMetaData();
if (indexMetaData != null && indexMetaData.getState() == IndexMetaData.State.CLOSE) {
// NoOpEngine takes precedence as long as the index is closed
return NoOpEngine::new;
}

I added a test case showing that closing and reopening a searchable snapshot index does work.

return Optional.of(engineConfig -> new ReadOnlyEngine(engineConfig, null, new TranslogStats(), false, Function.identity()));
}
return Optional.empty();
Expand Down
109 changes: 79 additions & 30 deletions .../org/elasticsearch/xpack/searchablesnapshots/AbstractSearchableSnapshotsRestTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public abstract class AbstractSearchableSnapshotsRestTestCase extends ESRestTest

protected abstract Settings repositorySettings();

public void testSearchableSnapshots() throws Exception {
private void runSearchableSnapshotsTest(SearchableSnapshotsTestCaseBody testCaseBody) throws Exception {
final String repositoryType = repositoryType();
final Settings repositorySettings = repositorySettings();

Expand Down Expand Up @@ -117,34 +117,7 @@ public void testSearchableSnapshots() throws Exception {
final Number count = count(restoredIndexName);
assertThat("Wrong index count for index " + restoredIndexName, count.intValue(), equalTo(numDocs));

for (int i = 0; i < 10; i++) {
final int randomTieBreaker = randomIntBetween(1, numDocs - 1);
Map<String, Object> searchResults;
switch (randomInt(3)) {
case 0:
searchResults = search(restoredIndexName, QueryBuilders.termQuery("field", String.valueOf(randomTieBreaker)));
assertThat(extractValue(searchResults, "hits.total.value"), equalTo(1));
@SuppressWarnings("unchecked")
Map<String, Object> searchHit = (Map<String, Object>) ((List<?>) extractValue(searchResults, "hits.hits")).get(0);
assertThat(extractValue(searchHit, "_index"), equalTo(restoredIndexName));
assertThat(extractValue(searchHit, "_source.field"), equalTo(randomTieBreaker));
break;
case 1:
searchResults = search(restoredIndexName, QueryBuilders.rangeQuery("field").lt(randomTieBreaker));
assertThat(extractValue(searchResults, "hits.total.value"), equalTo(randomTieBreaker));
break;
case 2:
searchResults = search(restoredIndexName, QueryBuilders.rangeQuery("field").gte(randomTieBreaker));
assertThat(extractValue(searchResults, "hits.total.value"), equalTo(numDocs - randomTieBreaker));
break;
case 3:
searchResults = search(restoredIndexName, QueryBuilders.matchQuery("text", "document"));
assertThat(extractValue(searchResults, "hits.total.value"), equalTo(numDocs));
break;
default:
fail("Unsupported randomized search query");
}
}
testCaseBody.runTest(restoredIndexName, numDocs);

logger.info("deleting snapshot [{}]", snapshot);
deleteSnapshot(repository, snapshot, false);
Expand All @@ -154,6 +127,71 @@ public void testSearchableSnapshots() throws Exception {
searchableSnapshotStats.size(), equalTo(numberOfShards));
}

public void testSearchResults() throws Exception {
runSearchableSnapshotsTest((restoredIndexName, numDocs) -> {
for (int i = 0; i < 10; i++) {
assertSearchResults(restoredIndexName, numDocs, randomFrom(Boolean.TRUE, Boolean.FALSE, null));
}
});
}

public void testSearchResultsWhenFrozen() throws Exception {
runSearchableSnapshotsTest((restoredIndexName, numDocs) -> {
final Request freezeRequest = new Request(HttpPost.METHOD_NAME, restoredIndexName + "/_freeze");
client().performRequest(freezeRequest);
Copy link
Member

@tlrx tlrx Mar 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you assertOK() this?

ensureGreen(restoredIndexName);
for (int i = 0; i < 10; i++) {
assertSearchResults(restoredIndexName, numDocs, Boolean.FALSE);
}
});
}

public void testCloseAndReopen() throws Exception {
runSearchableSnapshotsTest((restoredIndexName, numDocs) -> {
final Request closeRequest = new Request(HttpPost.METHOD_NAME, restoredIndexName + "/_close");
client().performRequest(closeRequest);
Copy link
Member

@tlrx tlrx Mar 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you assertOK() this?

ensureGreen(restoredIndexName);

final Request openRequest = new Request(HttpPost.METHOD_NAME, restoredIndexName + "/_open");
client().performRequest(openRequest);
Copy link
Member

@tlrx tlrx Mar 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you assertOK() this?

ensureGreen(restoredIndexName);

for (int i = 0; i < 10; i++) {
assertSearchResults(restoredIndexName, numDocs, randomFrom(Boolean.TRUE, Boolean.FALSE, null));
}
});
}

public void assertSearchResults(String indexName, int numDocs, Boolean ignoreThrottled) throws IOException {
final int randomTieBreaker = randomIntBetween(1, numDocs - 1);
Map<String, Object> searchResults;
switch (randomInt(3)) {
case 0:
searchResults
= search(indexName, QueryBuilders.termQuery("field", String.valueOf(randomTieBreaker)), ignoreThrottled);
assertThat(extractValue(searchResults, "hits.total.value"), equalTo(1));
@SuppressWarnings("unchecked")
Map<String, Object> searchHit = (Map<String, Object>) ((List<?>) extractValue(searchResults, "hits.hits")).get(0);
assertThat(extractValue(searchHit, "_index"), equalTo(indexName));
assertThat(extractValue(searchHit, "_source.field"), equalTo(randomTieBreaker));
break;
case 1:
searchResults = search(indexName, QueryBuilders.rangeQuery("field").lt(randomTieBreaker), ignoreThrottled);
assertThat(extractValue(searchResults, "hits.total.value"), equalTo(randomTieBreaker));
break;
case 2:
searchResults = search(indexName, QueryBuilders.rangeQuery("field").gte(randomTieBreaker), ignoreThrottled);
assertThat(extractValue(searchResults, "hits.total.value"), equalTo(numDocs - randomTieBreaker));
break;
case 3:
searchResults = search(indexName, QueryBuilders.matchQuery("text", "document"), ignoreThrottled);
assertThat(extractValue(searchResults, "hits.total.value"), equalTo(numDocs));
break;
default:
fail("Unsupported randomized search query");
}
}

protected static void registerRepository(String repository, String type, boolean verify, Settings settings) throws IOException {
final Request request = new Request(HttpPut.METHOD_NAME, "_snapshot/" + repository);
request.setJsonEntity(Strings.toString(new PutRepositoryRequest(repository).type(type).verify(verify).settings(settings)));
Expand Down Expand Up @@ -218,9 +256,12 @@ protected static Number count(String index) throws IOException {
return (Number) extractValue(responseAsMap, "count");
}

protected static Map<String, Object> search(String index, QueryBuilder query) throws IOException {
protected static Map<String, Object> search(String index, QueryBuilder query, Boolean ignoreThrottled) throws IOException {
final Request request = new Request(HttpPost.METHOD_NAME, '/' + index + "/_search");
request.setJsonEntity(new SearchSourceBuilder().trackTotalHits(true).query(query).toString());
if (ignoreThrottled != null) {
request.addParameter("ignore_throttled", ignoreThrottled.toString());
}

final Response response = client().performRequest(request);
assertThat("Failed to execute search request on index [" + index + "]: " + response,
Expand Down Expand Up @@ -260,4 +301,12 @@ protected static Map<String, Object> responseAsMap(Response response) throws IOE
protected static <T> T extractValue(Map<String, Object> map, String path) {
return (T) XContentMapValues.extractValue(path, map);
}

/**
* The body of a test case, which runs after the searchable snapshot has been created and restored.
*/
@FunctionalInterface
interface SearchableSnapshotsTestCaseBody {
void runTest(String indexName, int numDocs) throws IOException;
}
}