Skip to content

Commit 99009d8

Browse files
nielsbaumannielsbauman
committed
Fix mapping conflicts in clone/split/shrink APIs (elastic#137096)
If an index is in either `logsdb` or `time_series` mode and specifies a non-default `@timestamp` type mapping (e.g. `date_nanos`), using the clone, split, or shrink API will result in shards that are unable to initialize/recover due to a mapping conflict. As of elastic#133954, the `searchable_snapshot` ILM action makes use of the clone API by default - if the index has more than `0` replicas - and will thus also run into this issue.
1 parent f14175a commit 99009d8

File tree

6 files changed

+170
-14
lines changed

6 files changed

+170
-14
lines changed

‎docs/changelog/137096.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 137096
2+
summary: Fix mapping conflicts in clone/split/shrink APIs
3+
area: Indices APIs
4+
type: bug
5+
issues: []

‎server/src/internalClusterTest/java/org/elasticsearch/action/admin/indices/create/CloneIndexIT.java‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.elasticsearch.index.seqno.SeqNoStats;
2121
import org.elasticsearch.test.ESIntegTestCase;
2222
import org.elasticsearch.test.index.IndexVersionUtils;
23+
import org.elasticsearch.xcontent.ObjectPath;
2324
import org.elasticsearch.xcontent.XContentType;
2425

2526
import java.util.List;
@@ -203,4 +204,60 @@ public void testResizeChangeIndexSorts() {
203204
});
204205
assertThat(error.getMessage(), containsString("can't override index sort when resizing an index"));
205206
}
207+
208+
/**
209+
* Test that cloning a logsdb index with a non-default timestamp mapping doesn't result in any mapping conflicts.
210+
*/
211+
public void testCloneLogsdbIndexWithNonDefaultTimestamp() {
212+
// Create a logsdb index with a date_nanos @timestamp field
213+
final int numberOfReplicas = randomInt(internalCluster().numDataNodes() - 1);
214+
final var settings = indexSettings(1, numberOfReplicas).put("index.mode", "logsdb").put("index.blocks.write", true);
215+
prepareCreate("source").setSettings(settings).setMapping("@timestamp", "type=date_nanos").get();
216+
ensureGreen();
217+
218+
// Clone the index
219+
indicesAdmin().prepareResizeIndex("source", "target")
220+
.setResizeType(ResizeType.CLONE)
221+
// We need to explicitly set the number of replicas in case the source has 0 replicas and the cluster has only 1 data node
222+
.setSettings(Settings.builder().put("index.number_of_replicas", numberOfReplicas).build())
223+
.get();
224+
225+
// Verify that the target index has the correct @timestamp mapping
226+
final var targetMappings = indicesAdmin().prepareGetMappings(TEST_REQUEST_TIMEOUT, "target").get();
227+
assertThat(
228+
ObjectPath.eval("[email protected]", targetMappings.mappings().get("target").getSourceAsMap()),
229+
equalTo("date_nanos")
230+
);
231+
ensureGreen();
232+
}
233+
234+
/**
235+
* Test that cloning a time series index with a non-default timestamp mapping doesn't result in any mapping conflicts.
236+
*/
237+
public void testCloneTimeSeriesIndexWithNonDefaultTimestamp() {
238+
// Create a time series index with a date_nanos @timestamp field
239+
final int numberOfReplicas = randomInt(internalCluster().numDataNodes() - 1);
240+
final var settings = indexSettings(1, numberOfReplicas).put("index.mode", "time_series")
241+
.put("index.routing_path", "sensor_id")
242+
.put("index.blocks.write", true);
243+
prepareCreate("source").setSettings(settings)
244+
.setMapping("@timestamp", "type=date_nanos", "sensor_id", "type=keyword,time_series_dimension=true")
245+
.get();
246+
ensureGreen();
247+
248+
// Clone the index
249+
indicesAdmin().prepareResizeIndex("source", "target")
250+
.setResizeType(ResizeType.CLONE)
251+
// We need to explicitly set the number of replicas in case the source has 0 replicas and the cluster has only 1 data node
252+
.setSettings(Settings.builder().put("index.number_of_replicas", numberOfReplicas).build())
253+
.get();
254+
255+
// Verify that the target index has the correct @timestamp mapping
256+
final var targetMappings = indicesAdmin().prepareGetMappings(TEST_REQUEST_TIMEOUT, "target").get();
257+
assertThat(
258+
ObjectPath.eval("[email protected]", targetMappings.mappings().get("target").getSourceAsMap()),
259+
equalTo("date_nanos")
260+
);
261+
ensureGreen();
262+
}
206263
}

‎server/src/internalClusterTest/java/org/elasticsearch/action/admin/indices/create/ShrinkIndexIT.java‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import org.elasticsearch.indices.IndicesService;
5252
import org.elasticsearch.test.ESIntegTestCase;
5353
import org.elasticsearch.test.index.IndexVersionUtils;
54+
import org.elasticsearch.xcontent.ObjectPath;
5455
import org.elasticsearch.xcontent.XContentType;
5556

5657
import java.util.Arrays;
@@ -614,6 +615,63 @@ public void testShrinkThenSplitWithFailedNode() throws Exception {
614615
assertNoResizeSourceIndexSettings("splitagain");
615616
}
616617

618+
/**
619+
* Tests that shrinking a logsdb index with a non-default timestamp mapping doesn't result in any mapping conflicts.
620+
*/
621+
public void testShrinkLogsdbIndexWithNonDefaultTimestamp() {
622+
// Create a logsdb index with a date_nanos @timestamp field
623+
final var settings = indexSettings(2, 0).put("index.mode", "logsdb")
624+
.put("index.blocks.write", true)
625+
.put("index.routing.allocation.require._name", internalCluster().getRandomDataNodeName());
626+
prepareCreate("source").setSettings(settings).setMapping("@timestamp", "type=date_nanos").get();
627+
ensureGreen();
628+
629+
// Shrink the index
630+
indicesAdmin().prepareResizeIndex("source", "target")
631+
.setResizeType(ResizeType.SHRINK)
632+
// We need to explicitly set the number of replicas in case the source has 0 replicas and the cluster has only 1 data node
633+
.setSettings(Settings.builder().put("index.number_of_shards", 1).put("index.number_of_replicas", 0).build())
634+
.get();
635+
636+
// Verify that the target index has the correct @timestamp mapping
637+
final var targetMappings = indicesAdmin().prepareGetMappings(TEST_REQUEST_TIMEOUT, "target").get();
638+
assertThat(
639+
ObjectPath.eval("[email protected]", targetMappings.mappings().get("target").getSourceAsMap()),
640+
equalTo("date_nanos")
641+
);
642+
ensureGreen();
643+
}
644+
645+
/**
646+
* Tests that shrinking a time series index with a non-default timestamp mapping doesn't result in any mapping conflicts.
647+
*/
648+
public void testShrinkTimeSeriesIndexWithNonDefaultTimestamp() {
649+
// Create a time series index with a date_nanos @timestamp field
650+
final var settings = indexSettings(2, 0).put("index.mode", "time_series")
651+
.put("index.routing_path", "sensor_id")
652+
.put("index.routing.allocation.require._name", internalCluster().getRandomDataNodeName())
653+
.put("index.blocks.write", true);
654+
prepareCreate("source").setSettings(settings)
655+
.setMapping("@timestamp", "type=date_nanos", "sensor_id", "type=keyword,time_series_dimension=true")
656+
.get();
657+
ensureGreen();
658+
659+
// Shrink the index
660+
indicesAdmin().prepareResizeIndex("source", "target")
661+
.setResizeType(ResizeType.SHRINK)
662+
// We need to explicitly set the number of replicas in case the source has 0 replicas and the cluster has only 1 data node
663+
.setSettings(Settings.builder().put("index.number_of_shards", 1).put("index.number_of_replicas", 0).build())
664+
.get();
665+
666+
// Verify that the target index has the correct @timestamp mapping
667+
final var targetMappings = indicesAdmin().prepareGetMappings(TEST_REQUEST_TIMEOUT, "target").get();
668+
assertThat(
669+
ObjectPath.eval("[email protected]", targetMappings.mappings().get("target").getSourceAsMap()),
670+
equalTo("date_nanos")
671+
);
672+
ensureGreen();
673+
}
674+
617675
static void assertNoResizeSourceIndexSettings(final String index) {
618676
ClusterStateResponse clusterStateResponse = clusterAdmin().prepareState(TEST_REQUEST_TIMEOUT)
619677
.clear()

‎server/src/internalClusterTest/java/org/elasticsearch/action/admin/indices/create/SplitIndexIT.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import org.elasticsearch.indices.IndicesService;
4747
import org.elasticsearch.test.ESIntegTestCase;
4848
import org.elasticsearch.test.index.IndexVersionUtils;
49+
import org.elasticsearch.xcontent.ObjectPath;
4950
import org.elasticsearch.xcontent.XContentType;
5051

5152
import java.io.IOException;
@@ -496,4 +497,31 @@ public void testCreateSplitWithIndexSort() throws Exception {
496497
assertSortedSegments("target", expectedIndexSort);
497498
assertNoResizeSourceIndexSettings("target");
498499
}
500+
501+
/**
502+
* Tests that splitting a logsdb index with a non-default timestamp mapping doesn't result in any mapping conflicts.
503+
* N.B.: we don't test time_series indices as split is not supported for them.
504+
*/
505+
public void testSplitLogsdbIndexWithNonDefaultTimestamp() {
506+
// Create a logsdb index with a date_nanos @timestamp field
507+
final int numberOfReplicas = randomInt(internalCluster().numDataNodes() - 1);
508+
final var settings = indexSettings(1, numberOfReplicas).put("index.mode", "logsdb").put("index.blocks.write", true);
509+
prepareCreate("source").setSettings(settings).setMapping("@timestamp", "type=date_nanos").get();
510+
ensureGreen();
511+
512+
// Split the index
513+
indicesAdmin().prepareResizeIndex("source", "target")
514+
.setResizeType(ResizeType.SPLIT)
515+
// We need to explicitly set the number of replicas in case the source has 0 replicas and the cluster has only 1 data node
516+
.setSettings(Settings.builder().put("index.number_of_shards", 2).put("index.number_of_replicas", numberOfReplicas).build())
517+
.get();
518+
519+
// Verify that the target index has the correct @timestamp mapping
520+
final var targetMappings = indicesAdmin().prepareGetMappings(TEST_REQUEST_TIMEOUT, "target").get();
521+
assertThat(
522+
ObjectPath.eval("[email protected]", targetMappings.mappings().get("target").getSourceAsMap()),
523+
equalTo("date_nanos")
524+
);
525+
ensureGreen();
526+
}
499527
}

‎server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -543,11 +543,15 @@ private ClusterState applyCreateIndexWithTemporaryService(
543543
assert indicesService.hasIndex(temporaryIndexMeta.getIndex()) == false
544544
: Strings.format("Index [%s] already exists", temporaryIndexMeta.getIndex().getName());
545545
return indicesService.<ClusterState, Exception>withTempIndexService(temporaryIndexMeta, indexService -> {
546-
try {
547-
updateIndexMappingsAndBuildSortOrder(indexService, request, mappings, sourceMetadata);
548-
} catch (Exception e) {
549-
logger.log(silent ? Level.DEBUG : Level.INFO, "failed on parsing mappings on index creation [{}]", request.index(), e);
550-
throw e;
546+
// If we're creating the index from an existing index, we should not provide any mappings, as the new index shards will take
547+
// care of copying the mappings from the source index during recovery. Providing mappings here would cause conflicts.
548+
if (sourceMetadata == null) {
549+
try {
550+
updateIndexMappingsAndBuildSortOrder(indexService, request, mappings);
551+
} catch (Exception e) {
552+
logger.log(silent ? Level.DEBUG : Level.INFO, "failed on parsing mappings on index creation [{}]", request.index(), e);
553+
throw e;
554+
}
551555
}
552556

553557
final List<AliasMetadata> aliases = aliasSupplier.apply(indexService);
@@ -1550,8 +1554,7 @@ private static IndexMetadata.Builder createIndexMetadataBuilder(
15501554
private static void updateIndexMappingsAndBuildSortOrder(
15511555
IndexService indexService,
15521556
CreateIndexClusterStateUpdateRequest request,
1553-
List<CompressedXContent> mappings,
1554-
@Nullable IndexMetadata sourceMetadata
1557+
List<CompressedXContent> mappings
15551558
) throws IOException {
15561559
MapperService mapperService = indexService.mapperService();
15571560
IndexMode indexMode = indexService.getIndexSettings() != null ? indexService.getIndexSettings().getMode() : IndexMode.STANDARD;
@@ -1565,13 +1568,11 @@ private static void updateIndexMappingsAndBuildSortOrder(
15651568

15661569
indexMode.validateTimestampFieldMapping(request.dataStreamName() != null, mapperService.mappingLookup());
15671570

1568-
if (sourceMetadata == null) {
1569-
// now that the mapping is merged we can validate the index sort.
1570-
// we cannot validate for index shrinking since the mapping is empty
1571-
// at this point. The validation will take place later in the process
1572-
// (when all shards are copied in a single place).
1573-
indexService.getIndexSortSupplier().get();
1574-
}
1571+
// now that the mapping is merged we can validate the index sort.
1572+
// we cannot validate for index shrinking since the mapping is empty
1573+
// at this point. The validation will take place later in the process
1574+
// (when all shards are copied in a single place).
1575+
indexService.getIndexSortSupplier().get();
15751576
}
15761577

15771578
private static void validateActiveShardCount(ActiveShardCount waitForActiveShards, IndexMetadata indexMetadata) {

‎test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2114,6 +2114,13 @@ public String getRandomNodeName() {
21142114
return getNodeNameThat(Predicates.always());
21152115
}
21162116

2117+
/**
2118+
* @return the name of a random data node in a cluster
2119+
*/
2120+
public String getRandomDataNodeName() {
2121+
return getNodeNameThat(DiscoveryNode::canContainData);
2122+
}
2123+
21172124
/**
21182125
* @return the name of a random node in a cluster that match the {@code predicate}
21192126
*/

0 commit comments

Comments
 (0)