Skip to content

Fix IndexMetaData loads after rollover #33394

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
merged 1 commit into from
Sep 6, 2018
Merged

Fix IndexMetaData loads after rollover #33394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
58 changes: 57 additions & 1 deletion qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,62 @@ public void testShrinkAfterUpgrade() throws IOException {
}
}

/**
* Test upgrading after a rollover. Specifically:
* <ol>
* <li>Create an index with a write alias
* <li>Write some documents to the write alias
* <li>Roll over the index
* <li>Make sure the document count is correct
* <li>Upgrade
* <li>Write some more documents to the write alias
* <li>Make sure the document count is correct
* </ol>
*/
public void testRollover() throws IOException {
if (runningAgainstOldCluster) {
Request createIndex = new Request("PUT", "/" + index + "-000001");
createIndex.setJsonEntity("{"
+ " \"aliases\": {"
+ " \"" + index + "_write\": {}"
+ " }"
+ "}");
client().performRequest(createIndex);
}

int bulkCount = 10;
StringBuilder bulk = new StringBuilder();
for (int i = 0; i < bulkCount; i++) {
bulk.append("{\"index\":{}}\n");
bulk.append("{\"test\":\"test\"}\n");
}
Request bulkRequest = new Request("POST", "/" + index + "_write/doc/_bulk");
bulkRequest.setJsonEntity(bulk.toString());
bulkRequest.addParameter("refresh", "");
assertThat(EntityUtils.toString(client().performRequest(bulkRequest).getEntity()), containsString("\"errors\":false"));

if (runningAgainstOldCluster) {
Request rolloverRequest = new Request("POST", "/" + index + "_write/_rollover");
rolloverRequest.setJsonEntity("{"
+ " \"conditions\": {"
+ " \"max_docs\": 5"
+ " }"
+ "}");
client().performRequest(rolloverRequest);

assertThat(EntityUtils.toString(client().performRequest(new Request("GET", "/_cat/indices?v")).getEntity()),
containsString("testrollover-000002"));
}

Request countRequest = new Request("POST", "/" + index + "-*/_search");
countRequest.addParameter("size", "0");
Map<String, Object> count = entityAsMap(client().performRequest(countRequest));
assertNoFailures(count);

int expectedCount = bulkCount + (runningAgainstOldCluster ? 0 : bulkCount);
assertEquals(expectedCount, (int) XContentMapValues.extractValue("hits.total", count));
}

void assertBasicSearchWorks(int count) throws IOException {
logger.info("--> testing basic search");
{
Expand Down Expand Up @@ -947,7 +1003,7 @@ private void checkSnapshot(String snapshotName, int count, Version tookOnVersion
Request writeToRestoredRequest = new Request("POST", "/restored_" + index + "/doc/_bulk");
writeToRestoredRequest.addParameter("refresh", "true");
writeToRestoredRequest.setJsonEntity(bulk.toString());
client().performRequest(writeToRestoredRequest);
assertThat(EntityUtils.toString(client().performRequest(writeToRestoredRequest).getEntity()), containsString("\"errors\":false"));

// And count to make sure the add worked
// Make sure search finds all documents
Expand Down
3 changes: 3 additions & 0 deletions server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;
Expand Down Expand Up @@ -1346,6 +1347,8 @@ public void toXContent(XContentBuilder builder, IndexMetaData state) throws IOEx

@Override
public IndexMetaData fromXContent(XContentParser parser) throws IOException {
assert parser.getXContentRegistry() != NamedXContentRegistry.EMPTY
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

: "loading index metadata requires a working named xcontent registry";
return Builder.fromXContent(parser);
}
};
Expand Down
134 changes: 0 additions & 134 deletions server/src/main/java/org/elasticsearch/common/util/IndexFolderUpgrader.java
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions server/src/main/java/org/elasticsearch/gateway/GatewayMetaState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.IndexFolderUpgrader;
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.index.Index;
import org.elasticsearch.plugins.MetaDataUpgrader;
Expand Down Expand Up @@ -84,7 +83,6 @@ public GatewayMetaState(Settings settings, NodeEnvironment nodeEnv, MetaStateSer
if (DiscoveryNode.isMasterNode(settings) || DiscoveryNode.isDataNode(settings)) {
try {
ensureNoPre019State();
IndexFolderUpgrader.upgradeIndicesIfNeeded(settings, nodeEnv);
final MetaData metaData = metaStateService.loadFullState();
final MetaData upgradedMetaData = upgradeMetaData(metaData, metaDataIndexUpgradeService, metaDataUpgrader);
// We finished global state validation and successfully checked all indices for backward compatibility
Expand Down
9 changes: 6 additions & 3 deletions server/src/main/java/org/elasticsearch/gateway/TransportNodesListGatewayStartedShards.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,18 @@ public class TransportNodesListGatewayStartedShards extends
public static final String ACTION_NAME = "internal:gateway/local/started_shards";
private final NodeEnvironment nodeEnv;
private final IndicesService indicesService;
private final NamedXContentRegistry namedXContentRegistry;

@Inject
public TransportNodesListGatewayStartedShards(Settings settings, ThreadPool threadPool, ClusterService clusterService,
TransportService transportService, ActionFilters actionFilters,
NodeEnvironment env, IndicesService indicesService) {
NodeEnvironment env, IndicesService indicesService,
NamedXContentRegistry namedXContentRegistry) {
super(settings, ACTION_NAME, threadPool, clusterService, transportService, actionFilters,
Request::new, NodeRequest::new, ThreadPool.Names.FETCH_SHARD_STARTED, NodeGatewayStartedShards.class);
this.nodeEnv = env;
this.indicesService = indicesService;
this.namedXContentRegistry = namedXContentRegistry;
}

@Override
Expand Down Expand Up @@ -112,15 +115,15 @@ protected NodeGatewayStartedShards nodeOperation(NodeRequest request) {
try {
final ShardId shardId = request.getShardId();
logger.trace("{} loading local shard state info", shardId);
ShardStateMetaData shardStateMetaData = ShardStateMetaData.FORMAT.loadLatestState(logger, NamedXContentRegistry.EMPTY,
ShardStateMetaData shardStateMetaData = ShardStateMetaData.FORMAT.loadLatestState(logger, namedXContentRegistry,
nodeEnv.availableShardPaths(request.shardId));
if (shardStateMetaData != null) {
IndexMetaData metaData = clusterService.state().metaData().index(shardId.getIndex());
if (metaData == null) {
// we may send this requests while processing the cluster state that recovered the index
// sometimes the request comes in before the local node processed that cluster state
// in such cases we can load it from disk
metaData = IndexMetaData.FORMAT.loadLatestState(logger, NamedXContentRegistry.EMPTY,
metaData = IndexMetaData.FORMAT.loadLatestState(logger, namedXContentRegistry,
Copy link
Member Author

Choose a reason for hiding this comment

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

While I'm 99% sure this change is correct, I don't know of a way to trigger this code locally.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I follow. What do you mean with triggering code locally?

Copy link
Member Author

Choose a reason for hiding this comment

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

This branch looks like the kind of thing that won't kick in much. Do we have a test that'll call it?

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 we only have general IT tests ci runs. This class is not wel tested :(

Copy link
Member Author

Choose a reason for hiding this comment

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

I stuck an exception there and found RecoveryFromGatewayIT.testStartedShardFoundIfStateNotYetProcessed when I ran the test. So it looks like we do cover it. So I feel good!

nodeEnv.indexPaths(shardId.getIndex()));
}
if (metaData == null) {
Expand Down
8 changes: 5 additions & 3 deletions ...r/src/main/java/org/elasticsearch/indices/store/TransportNodesListShardStoreMetaData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,19 @@ public class TransportNodesListShardStoreMetaData extends TransportNodesAction<T
public static final String ACTION_NAME = "internal:cluster/nodes/indices/shard/store";

private final IndicesService indicesService;

private final NodeEnvironment nodeEnv;
private final NamedXContentRegistry namedXContentRegistry;

@Inject
public TransportNodesListShardStoreMetaData(Settings settings, ThreadPool threadPool,
ClusterService clusterService, TransportService transportService,
IndicesService indicesService, NodeEnvironment nodeEnv, ActionFilters actionFilters) {
IndicesService indicesService, NodeEnvironment nodeEnv,
ActionFilters actionFilters, NamedXContentRegistry namedXContentRegistry) {
super(settings, ACTION_NAME, threadPool, clusterService, transportService, actionFilters,
Request::new, NodeRequest::new, ThreadPool.Names.FETCH_SHARD_STORE, NodeStoreFilesMetaData.class);
this.indicesService = indicesService;
this.nodeEnv = nodeEnv;
this.namedXContentRegistry = namedXContentRegistry;
}

@Override
Expand Down Expand Up @@ -129,7 +131,7 @@ private StoreFilesMetaData listStoreMetaData(ShardId shardId) throws IOException
// we may send this requests while processing the cluster state that recovered the index
// sometimes the request comes in before the local node processed that cluster state
// in such cases we can load it from disk
metaData = IndexMetaData.FORMAT.loadLatestState(logger, NamedXContentRegistry.EMPTY,
metaData = IndexMetaData.FORMAT.loadLatestState(logger, namedXContentRegistry,
Copy link
Member Author

Choose a reason for hiding this comment

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

While I'm 99% sure this change is correct, I don't know of a way to trigger this code locally.

nodeEnv.indexPaths(shardId.getIndex()));
}
if (metaData == null) {
Expand Down
Loading