Skip to content

Conversation

@tlrx
Copy link
Member

@tlrx tlrx commented Jun 18, 2021

Now the Mount API overrides the index.shard.check_on_startup
setting value to false, there's no need for MountSnapshotStep
to set it explicitly.

This commit also fixes a failing test where some leftover code
was overriding the setting value.

Relates #74235

Now the Mount API overrides the index.shard.check_on_startup
setting value to `false`, there's no need for MountSnapshotStep
to set it explicitly.
@tlrx tlrx added >non-issue :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs :Data Management/ILM+SLM Index and Snapshot lifecycle management v8.0.0 v7.14.0 labels Jun 18, 2021
@tlrx tlrx requested a review from DaveCTurner June 18, 2021 08:16
@elasticmachine elasticmachine added Team:Data Management Meta label for data/management team Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. labels Jun 18, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (Team:Core/Features)

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

I suggested a different way to fix the test.

} else {
expectedReplicas = 0;
}
final String indexCheckOnStartup;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd be better to keep this code and fix the assertion so that we're covering more things:

diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/FrozenSearchableSnapshotsIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/FrozenSearchableSnapshotsIntegTests.java
index 5c1688ed1e1..17daeef5895 100644
--- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/FrozenSearchableSnapshotsIntegTests.java
+++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/FrozenSearchableSnapshotsIntegTests.java
@@ -291,7 +291,7 @@ public class FrozenSearchableSnapshotsIntegTests extends BaseFrozenSearchableSna
         assertThat(DataTierAllocationDecider.INDEX_ROUTING_PREFER_SETTING.get(settings), equalTo(expectedDataTiersPreference));
         assertTrue(SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING.get(settings));
         assertTrue(DiskThresholdDecider.SETTING_IGNORE_DISK_WATERMARKS.get(settings));
-        assertThat(IndexSettings.INDEX_CHECK_ON_STARTUP.get(settings), equalTo("false"));
+        assertThat(IndexSettings.INDEX_CHECK_ON_STARTUP.get(settings), equalTo(indexCheckOnStartup));

         checkSoftDeletesNotEagerlyLoaded(restoredIndexName);
         assertTotalHits(restoredIndexName, originalAllHits, originalBarHits);

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, as you prefer.

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

LGTM

@tlrx tlrx merged commit b6168e2 into elastic:master Jun 18, 2021
@tlrx tlrx deleted the mountedstep branch June 18, 2021 09:37
@tlrx
Copy link
Member Author

tlrx commented Jun 18, 2021

Thanks David

tlrx added a commit to tlrx/elasticsearch that referenced this pull request Jun 18, 2021
Now the Mount API overrides the index.shard.check_on_startup
setting value to false, there's no need for MountSnapshotStep
to set it explicitly.

Backport of elastic#74283
Relates elastic#74235
tlrx added a commit that referenced this pull request Jun 18, 2021
…#74288)

Now the Mount API overrides the index.shard.check_on_startup
setting value to false, there's no need for MountSnapshotStep
to set it explicitly.

Backport of #74283
Relates #74235
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Data Management/ILM+SLM Index and Snapshot lifecycle management :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >non-issue Team:Data Management Meta label for data/management team Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v7.14.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants