Skip to content

Commit

Permalink
Reject remounting snapshot of a searchable snapshot (elastic#68816)
Browse files Browse the repository at this point in the history
Today you can mount a snapshot of a searchable snapshot index, but the
shard fails to allocate since the underlying snapshot is devoid of
content. Doing this is a mistake, you probably meant to restore the
index instead, so this commit rejects it earlier with a more helpful
message.

Closes elastic#68792
  • Loading branch information
DaveCTurner committed Feb 10, 2021
1 parent ffe9d6c commit ab9f97b
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -1376,7 +1376,7 @@ public void markStoreCorrupted(IOException exception) throws IOException {
BytesRef ref = bytes.toBytesRef();
output.writeBytes(ref.bytes, ref.offset, ref.length);
CodecUtil.writeFooter(output);
} catch (IOException ex) {
} catch (IOException | ImmutableDirectoryException ex) {
logger.warn("Can't mark store as corrupted", ex);
}
directory().sync(Collections.singleton(corruptionMarkerName));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.carrotsearch.hppc.cursors.ObjectCursor;
import org.apache.lucene.index.IndexFileNames;
import org.apache.lucene.search.TotalHits;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.ResourceNotFoundException;
import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse;
import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotIndexShardStatus;
Expand Down Expand Up @@ -98,7 +99,9 @@
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.getDataTiersPreference;
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.SNAPSHOT_DIRECTORY_FACTORY_KEY;
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.SNAPSHOT_RECOVERY_STATE_FACTORY_KEY;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
Expand Down Expand Up @@ -1292,6 +1295,34 @@ public void testSnapshotOfSearchableSnapshotIncludesNoDataButCanBeRestored() thr
logger.info("--> finished restoring snapshot-2");

assertTotalHits(restoredIndexName, originalAllHits, originalBarHits);

final IllegalArgumentException remountException = expectThrows(IllegalArgumentException.class, () -> {
try {
mountSnapshot(
restoreRepositoryName,
snapshotTwo.getName(),
restoredIndexName,
randomAlphaOfLength(10).toLowerCase(Locale.ROOT),
Settings.EMPTY
);
} catch (Exception e) {
final Throwable cause = ExceptionsHelper.unwrap(e, IllegalArgumentException.class);
throw cause == null ? e : cause;
}
});
assertThat(
remountException.getMessage(),
allOf(
containsString("is a snapshot of a searchable snapshot index backed by index"),
containsString(repositoryName),
containsString(snapshotOne.getName()),
containsString(indexName),
containsString(restoreRepositoryName),
containsString(snapshotTwo.getName()),
containsString(restoredIndexName),
containsString("cannot be mounted; did you mean to restore it instead?")
)
);
}

private void assertTotalHits(String indexName, TotalHits originalAllHits, TotalHits originalBarHits) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,15 @@
import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants;

import java.util.Arrays;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;

import static org.elasticsearch.index.IndexModule.INDEX_RECOVERY_TYPE_SETTING;
import static org.elasticsearch.index.IndexModule.INDEX_STORE_TYPE_SETTING;
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.getDataTiersPreference;
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.isSearchableSnapshotStore;

/**
* Action that mounts a snapshot as a searchable snapshot, by converting the mount request into a restore request with specific settings
Expand Down Expand Up @@ -177,6 +179,25 @@ protected void masterOperation(
final String[] ignoreIndexSettings = Arrays.copyOf(request.ignoreIndexSettings(), request.ignoreIndexSettings().length + 1);
ignoreIndexSettings[ignoreIndexSettings.length - 1] = IndexMetadata.SETTING_DATA_PATH;

final IndexMetadata indexMetadata = repository.getSnapshotIndexMetaData(repoData, snapshotId, indexId);
if (isSearchableSnapshotStore(indexMetadata.getSettings())) {
throw new IllegalArgumentException(
String.format(
Locale.ROOT,
"index [%s] in snapshot [%s/%s:%s] is a snapshot of a searchable snapshot index "
+ "backed by index [%s] in snapshot [%s/%s:%s] and cannot be mounted; did you mean to restore it instead?",
indexName,
repoName,
repository.getMetadata().uuid(),
snapName,
SearchableSnapshots.SNAPSHOT_INDEX_NAME_SETTING.get(indexMetadata.getSettings()),
SearchableSnapshots.SNAPSHOT_REPOSITORY_NAME_SETTING.get(indexMetadata.getSettings()),
SearchableSnapshots.SNAPSHOT_REPOSITORY_UUID_SETTING.get(indexMetadata.getSettings()),
SearchableSnapshots.SNAPSHOT_SNAPSHOT_NAME_SETTING.get(indexMetadata.getSettings())
)
);
}

client.admin()
.cluster()
.restoreSnapshot(
Expand Down

0 comments on commit ab9f97b

Please sign in to comment.