Skip to content
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

Jump to bottom

Always check for archiving broken index settings #41209

Merged
merged 3 commits into from
Apr 17, 2019
Merged

Always check for archiving broken index settings #41209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9737ff7
Always check for archiving broken index settings
jasontedor Apr 15, 2019
2132489
Remove bogus test
jasontedor Apr 15, 2019
856b45d
Fix imports
jasontedor Apr 16, 2019
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
6 changes: 5 additions & 1 deletion server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,11 @@ public MetaDataIndexUpgradeService(Settings settings, NamedXContentRegistry xCon
public IndexMetaData upgradeIndexMetaData(IndexMetaData indexMetaData, Version minimumIndexCompatibilityVersion) {
// Throws an exception if there are too-old segments:
if (isUpgraded(indexMetaData)) {
return indexMetaData;
/*
* We still need to check for broken index settings since it might be that a user removed a plugin that registers a setting
* needed by this index.
*/
return archiveBrokenIndexSettings(indexMetaData);
}
checkSupportedVersion(indexMetaData, minimumIndexCompatibilityVersion);
IndexMetaData newMetaData = indexMetaData;
Expand Down
69 changes: 0 additions & 69 deletions server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

package org.elasticsearch.action.admin.indices.create;

import com.carrotsearch.hppc.cursors.ObjectCursor;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.UnavailableShardsException;
import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse;
Expand All @@ -33,28 +31,18 @@
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.MappingMetaData;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.routing.IndexRoutingTable;
import org.elasticsearch.cluster.routing.IndexShardRoutingTable;
import org.elasticsearch.cluster.routing.RoutingTable;
import org.elasticsearch.cluster.routing.UnassignedInfo;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.gateway.MetaStateService;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.index.query.RangeQueryBuilder;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
import org.elasticsearch.test.ESIntegTestCase.Scope;
import org.elasticsearch.test.InternalTestCluster;

import java.util.HashMap;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.BiFunction;

Expand All @@ -63,12 +51,8 @@
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertBlocked;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.Matchers.hasToString;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.hamcrest.core.IsNull.notNullValue;

Expand Down Expand Up @@ -396,57 +380,4 @@ public void testIndexNameInResponse() {
assertEquals("Should have index name in response", "foo", response.index());
}

public void testIndexWithUnknownSetting() throws Exception {
final int replicas = internalCluster().numDataNodes() - 1;
final Settings settings = Settings.builder().put("index.number_of_shards", 1).put("index.number_of_replicas", replicas).build();
client().admin().indices().prepareCreate("test").setSettings(settings).get();
ensureGreen("test");
final ClusterState state = client().admin().cluster().prepareState().get().getState();

final Set<String> dataOrMasterNodeNames = new HashSet<>();
for (final ObjectCursor<DiscoveryNode> node : state.nodes().getMasterAndDataNodes().values()) {
assertTrue(dataOrMasterNodeNames.add(node.value.getName()));
}

final IndexMetaData metaData = state.getMetaData().index("test");
internalCluster().fullRestart(new InternalTestCluster.RestartCallback() {
@Override
public Settings onNodeStopped(String nodeName) throws Exception {
if (dataOrMasterNodeNames.contains(nodeName)) {
final MetaStateService metaStateService = internalCluster().getInstance(MetaStateService.class, nodeName);
final IndexMetaData brokenMetaData =
IndexMetaData
.builder(metaData)
.settings(Settings.builder().put(metaData.getSettings()).put("index.foo", true))
.build();
// so evil
metaStateService.writeIndexAndUpdateManifest("broken metadata", brokenMetaData);
}
return super.onNodeStopped(nodeName);
}
});

// check that the cluster does not keep reallocating shards
assertBusy(() -> {
final RoutingTable routingTable = client().admin().cluster().prepareState().get().getState().routingTable();
final IndexRoutingTable indexRoutingTable = routingTable.index("test");
assertNotNull(indexRoutingTable);
for (IndexShardRoutingTable shardRoutingTable : indexRoutingTable) {
assertTrue(shardRoutingTable.primaryShard().unassigned());
assertEquals(UnassignedInfo.AllocationStatus.DECIDERS_NO,
shardRoutingTable.primaryShard().unassignedInfo().getLastAllocationStatus());
assertThat(shardRoutingTable.primaryShard().unassignedInfo().getNumFailedAllocations(), greaterThan(0));
}
}, 60, TimeUnit.SECONDS);
client().admin().indices().prepareClose("test").get();

// try to open the index
final ElasticsearchException e =
expectThrows(ElasticsearchException.class, () -> client().admin().indices().prepareOpen("test").get());
assertThat(e, hasToString(containsString("Failed to verify index " + metaData.getIndex())));
assertNotNull(e.getCause());
assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
assertThat(e, hasToString(containsString("unknown setting [index.foo]")));
}

}
40 changes: 28 additions & 12 deletions ...er/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@

import java.util.Collections;

import static org.hamcrest.Matchers.equalTo;

public class MetaDataIndexUpgradeServiceTests extends ESTestCase {

public void testArchiveBrokenIndexSettings() {
MetaDataIndexUpgradeService service = new MetaDataIndexUpgradeService(Settings.EMPTY, xContentRegistry(),
new MapperRegistry(Collections.emptyMap(), Collections.emptyMap(), MapperPlugin.NOOP_FIELD_FILTER),
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, Collections.emptyList());
MetaDataIndexUpgradeService service = getMetaDataIndexUpgradeService();
IndexMetaData src = newIndexMeta("foo", Settings.EMPTY);
IndexMetaData indexMetaData = service.archiveBrokenIndexSettings(src);
assertSame(indexMetaData, src);
Expand All @@ -58,10 +58,20 @@ public void testArchiveBrokenIndexSettings() {
assertSame(indexMetaData, src);
}

public void testAlreadyUpgradedIndexArchivesBrokenIndexSettings() {
final MetaDataIndexUpgradeService service = getMetaDataIndexUpgradeService();
final IndexMetaData initial = newIndexMeta(
"foo",
Settings.builder().put(IndexMetaData.SETTING_VERSION_UPGRADED, Version.CURRENT).put("index.refresh_interval", "-200").build());
assertTrue(service.isUpgraded(initial));
final IndexMetaData after = service.upgradeIndexMetaData(initial, Version.CURRENT.minimumIndexCompatibilityVersion());
// the index does not need to be upgraded, but checking that it does should archive any broken settings
assertThat(after.getSettings().get("archived.index.refresh_interval"), equalTo("-200"));
assertNull(after.getSettings().get("index.refresh_interval"));
}

public void testUpgrade() {
MetaDataIndexUpgradeService service = new MetaDataIndexUpgradeService(Settings.EMPTY, xContentRegistry(),
new MapperRegistry(Collections.emptyMap(), Collections.emptyMap(), MapperPlugin.NOOP_FIELD_FILTER),
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, Collections.emptyList());
MetaDataIndexUpgradeService service = getMetaDataIndexUpgradeService();
IndexMetaData src = newIndexMeta("foo", Settings.builder().put("index.refresh_interval", "-200").build());
assertFalse(service.isUpgraded(src));
src = service.upgradeIndexMetaData(src, Version.CURRENT.minimumIndexCompatibilityVersion());
Expand All @@ -72,9 +82,7 @@ public void testUpgrade() {
}

public void testIsUpgraded() {
MetaDataIndexUpgradeService service = new MetaDataIndexUpgradeService(Settings.EMPTY, xContentRegistry(),
new MapperRegistry(Collections.emptyMap(), Collections.emptyMap(), MapperPlugin.NOOP_FIELD_FILTER),
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, Collections.emptyList());
MetaDataIndexUpgradeService service = getMetaDataIndexUpgradeService();
IndexMetaData src = newIndexMeta("foo", Settings.builder().put("index.refresh_interval", "-200").build());
assertFalse(service.isUpgraded(src));
Version version = VersionUtils.randomVersionBetween(random(), VersionUtils.getFirstVersion(), VersionUtils.getPreviousVersion());
Expand All @@ -85,9 +93,7 @@ public void testIsUpgraded() {
}

public void testFailUpgrade() {
MetaDataIndexUpgradeService service = new MetaDataIndexUpgradeService(Settings.EMPTY, xContentRegistry(),
new MapperRegistry(Collections.emptyMap(), Collections.emptyMap(), MapperPlugin.NOOP_FIELD_FILTER),
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, Collections.emptyList());
MetaDataIndexUpgradeService service = getMetaDataIndexUpgradeService();
Version minCompat = Version.CURRENT.minimumIndexCompatibilityVersion();
Version indexUpgraded = VersionUtils.randomVersionBetween(random(),
minCompat,
Expand Down Expand Up @@ -144,6 +150,15 @@ public void testPluginUpgradeFailure() {
assertEquals(message, "Cannot upgrade index foo");
}

private MetaDataIndexUpgradeService getMetaDataIndexUpgradeService() {
return new MetaDataIndexUpgradeService(
Settings.EMPTY,
xContentRegistry(),
new MapperRegistry(Collections.emptyMap(), Collections.emptyMap(), MapperPlugin.NOOP_FIELD_FILTER),
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
Collections.emptyList());
}

public static IndexMetaData newIndexMeta(String name, Settings indexSettings) {
Settings build = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 1)
Expand All @@ -155,4 +170,5 @@ public static IndexMetaData newIndexMeta(String name, Settings indexSettings) {
.build();
return IndexMetaData.builder(name).settings(build).build();
}

}