-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Rolling upgrade test for synthetic id #141525
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.
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
Changes from all commits
e4d21f5
90dbf43
e3e000c
b03878a
b9d2ea1
6e5737f
e0832bd
af1ed58
727c911
c5e6371
4f0df9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| area: TSDB | ||
| issues: [] | ||
| pr: 141525 | ||
| summary: Rolling upgrade test for synthetic id | ||
| type: enhancement |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,190 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the "Elastic License | ||
| * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side | ||
| * Public License v 1"; you may not use this file except in compliance with, at | ||
| * your election, the "Elastic License 2.0", the "GNU Affero General Public | ||
| * License v3.0 only", or the "Server Side Public License, v 1". | ||
| */ | ||
|
|
||
| package org.elasticsearch.upgrades; | ||
|
|
||
| import com.carrotsearch.randomizedtesting.annotations.Name; | ||
|
|
||
| import org.elasticsearch.action.admin.indices.create.CreateIndexResponse; | ||
| import org.elasticsearch.client.Request; | ||
| import org.elasticsearch.client.Response; | ||
| import org.elasticsearch.client.ResponseException; | ||
| import org.elasticsearch.cluster.metadata.IndexMetadata; | ||
| import org.elasticsearch.common.settings.Settings; | ||
| import org.elasticsearch.index.IndexMode; | ||
| import org.elasticsearch.index.IndexSettings; | ||
| import org.elasticsearch.index.IndexVersion; | ||
| import org.elasticsearch.index.IndexVersions; | ||
| import org.elasticsearch.test.rest.ObjectPath; | ||
| import org.hamcrest.Matchers; | ||
|
|
||
| import java.io.IOException; | ||
| import java.time.Instant; | ||
| import java.time.temporal.ChronoUnit; | ||
| import java.util.Locale; | ||
| import java.util.Map; | ||
| import java.util.StringJoiner; | ||
|
|
||
| public class TSDBSyntheticIdUpgradeIT extends AbstractRollingUpgradeTestCase { | ||
| private static final int DOC_COUNT = 10; | ||
|
|
||
| public TSDBSyntheticIdUpgradeIT(@Name("upgradedNodes") int upgradedNodes) { | ||
| super(upgradedNodes); | ||
| } | ||
|
|
||
| public void testRollingUpgrade() throws IOException { | ||
| IndexVersion oldClusterIndexVersion = getOldClusterIndexVersion(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that we need to guard this test with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The feature flag(s) that control if the setting can be used or not is in the nodes different processes and might not align with what we see in the test-process. So I don't think we should check it here. |
||
|
|
||
| if (hasSupportForSyntheticId(oldClusterIndexVersion)) { | ||
| if (isOldCluster()) { | ||
| assertWriteIndex("old-cluster-index"); | ||
| assertIndexRead("old-cluster-index"); | ||
| } | ||
|
|
||
| if (isFirstMixedCluster()) { | ||
| assertWriteIndex("first-mixed-cluster-index"); | ||
| assertIndexRead("old-cluster-index"); | ||
| assertIndexRead("first-mixed-cluster-index"); | ||
| } | ||
|
|
||
| if (isFirstMixedCluster() == false && isMixedCluster()) { | ||
| assertWriteIndex("second-mixed-cluster-index"); | ||
| assertIndexRead("old-cluster-index"); | ||
| assertIndexRead("first-mixed-cluster-index"); | ||
| assertIndexRead("second-mixed-cluster-index"); | ||
| } | ||
|
|
||
| if (isUpgradedCluster()) { | ||
| assertWriteIndex("upgraded-cluster-index"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we check that once we've upgraded the cluster the disk usage for the inverted index is 0? |
||
| assertIndexRead("old-cluster-index"); | ||
| assertIndexRead("first-mixed-cluster-index"); | ||
| assertIndexRead("second-mixed-cluster-index"); | ||
| assertIndexRead("upgraded-cluster-index"); | ||
| } | ||
| } else { | ||
|
|
||
| if (isOldCluster()) { | ||
| assertNoWriteIndex("old-cluster-index"); | ||
| } | ||
|
|
||
| if (isMixedCluster()) { | ||
| assertNoWriteIndex("mixed-cluster-index"); | ||
| } | ||
|
|
||
| if (isUpgradedCluster()) { | ||
| assertWriteIndex("upgraded-cluster-index"); | ||
| assertIndexRead("upgraded-cluster-index"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static void assertWriteIndex(String indexName) throws IOException { | ||
| assertIndexCanBeCreated(indexName); | ||
| assertCanAddDocuments(indexName); | ||
| } | ||
|
|
||
| private static void assertIndexRead(String indexName) throws IOException { | ||
| assertTrue("Expected index [" + indexName + "] to exist, but did not", indexExists(indexName)); | ||
| Map<String, Object> indexSettingsAsMap = getIndexSettingsAsMap(indexName); | ||
| assertThat(indexSettingsAsMap.get(IndexSettings.SYNTHETIC_ID.getKey()), Matchers.equalTo("true")); | ||
| assertDocCount(client(), indexName, DOC_COUNT); | ||
| assertThat(invertedIndexSize(indexName), Matchers.equalTo(0)); | ||
| } | ||
|
|
||
| private static int invertedIndexSize(String indexName) throws IOException { | ||
| var diskUsage = new Request("POST", "/" + indexName + "/_disk_usage?run_expensive_tasks=true"); | ||
| Response response = client().performRequest(diskUsage); | ||
| ObjectPath objectPath = ObjectPath.createFromResponse(response); | ||
| return objectPath.evaluate(indexName + ".all_fields.inverted_index.total_in_bytes"); | ||
| } | ||
|
|
||
| private static void assertIndexCanBeCreated(String indexName) throws IOException { | ||
| CreateIndexResponse response = null; | ||
| try { | ||
| response = createSyntheticIdIndex(indexName); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we assert that the index settings include the synthetic id setting? |
||
| assertTrue("Expected index [" + indexName + "] to be created successfully, but was not", response.isAcknowledged()); | ||
| assertTrue( | ||
| "Expected shards of index [" + indexName + "] to be created successfully, but was not", | ||
| response.isShardsAcknowledged() | ||
| ); | ||
| } finally { | ||
| if (response != null) { | ||
| response.decRef(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static void assertCanAddDocuments(String indexName) throws IOException { | ||
| StringJoiner joiner = new StringJoiner("\n", "", "\n"); | ||
| Instant now = Instant.now(); | ||
| for (int i = 0; i < DOC_COUNT; i++) { | ||
| addDocument(joiner, now.plus(i, ChronoUnit.SECONDS)); | ||
| } | ||
| var request = new Request("PUT", "/" + indexName + "/_bulk"); | ||
| request.setJsonEntity(joiner.toString()); | ||
| request.addParameter("refresh", "true"); | ||
| Response response = client().performRequest(request); | ||
| assertOK(response); | ||
| } | ||
|
|
||
| private static void addDocument(StringJoiner joiner, Instant timestamp) { | ||
| joiner.add("{\"create\": {}}"); | ||
| joiner.add(String.format(Locale.ROOT, """ | ||
| {"@timestamp": "%s", "hostname": "host", "metric": {"field": "cpu-load", "value": %d}} | ||
| """, timestamp, randomByte())); | ||
| } | ||
|
|
||
| private static void assertNoWriteIndex(String indexName) { | ||
| String setting = IndexSettings.SYNTHETIC_ID.getKey(); | ||
| String unknownSetting = "unknown setting [" + setting + "]"; | ||
| String versionTooLow = "The setting [" + setting + "] is set to [true] but index metadata has a different value [false]"; | ||
|
|
||
| ResponseException e = assertThrows(ResponseException.class, () -> createSyntheticIdIndex(indexName)); | ||
| assertThat(e.getMessage(), Matchers.either(Matchers.containsString(unknownSetting)).or(Matchers.containsString(versionTooLow))); | ||
| assertThat(e.getMessage(), Matchers.containsString("illegal_argument_exception")); | ||
| } | ||
|
|
||
| private static CreateIndexResponse createSyntheticIdIndex(String indexName) throws IOException { | ||
| Settings settings = Settings.builder() | ||
| .put(IndexSettings.SYNTHETIC_ID.getKey(), true) | ||
| .put(IndexSettings.MODE.getKey(), IndexMode.TIME_SERIES) | ||
| .put(IndexMetadata.INDEX_ROUTING_PATH.getKey(), "hostname") | ||
| .build(); | ||
| final var mapping = """ | ||
| { | ||
| "properties": { | ||
| "@timestamp": { | ||
| "type": "date" | ||
| }, | ||
| "hostname": { | ||
| "type": "keyword", | ||
| "time_series_dimension": true | ||
| }, | ||
| "metric": { | ||
| "properties": { | ||
| "field": { | ||
| "type": "keyword", | ||
| "time_series_dimension": true | ||
| }, | ||
| "value": { | ||
| "type": "integer", | ||
| "time_series_metric": "counter" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| """; | ||
| return createIndex(indexName, settings, mapping); | ||
| } | ||
|
|
||
| private boolean hasSupportForSyntheticId(IndexVersion indexVersion) { | ||
| return indexVersion.onOrAfter(IndexVersions.TIME_SERIES_USE_SYNTHETIC_ID_94); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we test against the live commits in the serverless environments for test suites in this module. I think we should have that additional test coverage. Many logsdb en tsdb tests are now in x-pack/plugin/logsdb/qa/rolling-upgrade module. This test against stateless versions like the current module , but this module is also mirrored in serverless project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! I would like to address this in a later PR because it will probably take quite a bit of time for me to dig into this (so much to learn), if you are ok with that? Do you have any further pointers to where I can read more about how we test against live commits in serverless?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is good with me. I think it is a matter of moving this test suite to the
x-pack/plugin/logsdb/qa/rolling-upgrademodule and adjusting how the tests upgrades nodes. See other test suites in that module.