Deprecate slicing on _uid. (#29353)

Deprecate slicing on `_uid`.

`_id` should be used instead on 6.x.

modules/reindex/src/main/java/org/elasticsearch/index/reindex/BulkByScrollParallelizationHelper.java

@@ -27,7 +27,7 @@
 import org.elasticsearch.client.Client;
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.index.Index;
-import org.elasticsearch.index.mapper.UidFieldMapper;
+import org.elasticsearch.index.mapper.IdFieldMapper;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
 import org.elasticsearch.search.slice.SliceBuilder;
 import org.elasticsearch.tasks.TaskId;
@@ -127,7 +127,7 @@ private static <Request extends AbstractBulkByScrollRequest<Request>> void sendS
         LeaderBulkByScrollTaskState worker = task.getLeaderState();
         int totalSlices = worker.getSlices();
         TaskId parentTaskId = new TaskId(localNodeId, task.getId());
-        for (final SearchRequest slice : sliceIntoSubRequests(request.getSearchRequest(), UidFieldMapper.NAME, totalSlices)) {
+        for (final SearchRequest slice : sliceIntoSubRequests(request.getSearchRequest(), IdFieldMapper.NAME, totalSlices)) {
             // TODO move the request to the correct node. maybe here or somehow do it as part of startup for reindex in general....
             Request requestForSlice = request.forSlice(parentTaskId, slice, totalSlices);
             ActionListener<BulkByScrollResponse> sliceListener = ActionListener.wrap(

rest-api-spec/src/main/resources/rest-api-spec/test/scroll/12_slices.yml

@@ -42,8 +42,8 @@ setup:
 ---
 "Sliced scroll":
   - skip:
-      version: " - 5.3.0"
-      reason: Prior version uses a random seed per node to compute the hash of the keys.
+      version: " - 6.99.99"
+      reason: Disabled until pr 29353 is backported
 
   - do:
       search:

server/src/main/java/org/elasticsearch/index/mapper/IdFieldMapper.java

@@ -161,7 +161,7 @@ public Query termsQuery(List<?> values, QueryShardContext context) {
         @Override
         public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
             if (indexOptions() == IndexOptions.NONE) {
-                throw new IllegalArgumentException("Fielddata access on the _uid field is disallowed");
+                throw new IllegalArgumentException("Fielddata access on the _id field is disallowed");
             }
             final IndexFieldData.Builder fieldDataBuilder = new PagedBytesIndexFieldData.Builder(
                     TextFieldMapper.Defaults.FIELDDATA_MIN_FREQUENCY,

server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java

@@ -22,11 +22,14 @@
 import org.apache.lucene.search.MatchAllDocsQuery;
 import org.apache.lucene.search.MatchNoDocsQuery;
 import org.apache.lucene.search.Query;
+import org.elasticsearch.Version;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.logging.DeprecationLogger;
+import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.xcontent.ObjectParser;
 import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -53,6 +56,9 @@
  *  {@link org.elasticsearch.search.slice.DocValuesSliceQuery} is used to filter the results.
  */
 public class SliceBuilder implements Writeable, ToXContentObject {
+
+    private static final DeprecationLogger DEPRECATION_LOG = new DeprecationLogger(Loggers.getLogger(SliceBuilder.class));
+
     public static final ParseField FIELD_FIELD = new ParseField("field");
     public static final ParseField ID_FIELD = new ParseField("id");
     public static final ParseField MAX_FIELD = new ParseField("max");
@@ -66,7 +72,7 @@ public class SliceBuilder implements Writeable, ToXContentObject {
     }
 
     /** Name of field to slice against (_uid by default) */
-    private String field = UidFieldMapper.NAME;
+    private String field = IdFieldMapper.NAME;
     /** The id of the slice */
     private int id = -1;
     /** Max number of slices */
@@ -75,7 +81,7 @@ public class SliceBuilder implements Writeable, ToXContentObject {
     private SliceBuilder() {}
 
     public SliceBuilder(int id, int max) {
-        this(UidFieldMapper.NAME, id, max);
+        this(IdFieldMapper.NAME, id, max);
     }
 
     /**
@@ -91,14 +97,23 @@ public SliceBuilder(String field, int id, int max) {
     }
 
     public SliceBuilder(StreamInput in) throws IOException {
-        this.field = in.readString();
+        String field = in.readString();
+        if (UidFieldMapper.NAME.equals(field) && in.getVersion().before(Version.V_6_3_0)) {
+            // This is safe because _id and _uid are handled the same way in #toFilter
+            field = IdFieldMapper.NAME;
+        }
+        this.field = field;
         this.id = in.readVInt();
         this.max = in.readVInt();
     }
 
     @Override
     public void writeTo(StreamOutput out) throws IOException {
-        out.writeString(field);
+        if (IdFieldMapper.NAME.equals(field) && out.getVersion().before(Version.V_6_3_0)) {
+            out.writeString(UidFieldMapper.NAME);
+        } else {
+            out.writeString(field);
+        }
         out.writeVInt(id);
         out.writeVInt(max);
     }
@@ -201,6 +216,14 @@ public Query toFilter(QueryShardContext context, int shardId, int numShards) {
             if (context.getIndexSettings().isSingleType()) {
                 // on new indices, the _id acts as a _uid
                 field = IdFieldMapper.NAME;
+                DEPRECATION_LOG.deprecated("Computing slices on the [_uid] field is deprecated for 6.x indices, use [_id] instead");
+            }
+            useTermQuery = true;
+        } else if (IdFieldMapper.NAME.equals(field)) {
+            if (context.getIndexSettings().isSingleType() == false) {
+                // on old indices, we need _uid. We maintain this so that users
+                // can use _id to slice even if they still have 5.x indices.
+                field = UidFieldMapper.NAME;
             }
             useTermQuery = true;
         } else if (type.hasDocValues() == false) {

server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java

@@ -40,6 +40,7 @@
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.fielddata.IndexNumericFieldData;
+import org.elasticsearch.index.mapper.IdFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.UidFieldMapper;
 import org.elasticsearch.index.query.QueryShardContext;
@@ -158,9 +159,9 @@ public Query existsQuery(QueryShardContext context) {
                     return null;
                 }
             };
-            fieldType.setName(UidFieldMapper.NAME);
+            fieldType.setName(IdFieldMapper.NAME);
             fieldType.setHasDocValues(false);
-            when(context.fieldMapper(UidFieldMapper.NAME)).thenReturn(fieldType);
+            when(context.fieldMapper(IdFieldMapper.NAME)).thenReturn(fieldType);
             when(context.getIndexReader()).thenReturn(reader);
             Settings settings = Settings.builder()
                     .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
@@ -225,7 +226,7 @@ public Query existsQuery(QueryShardContext context) {
             Map<Integer, AtomicInteger> numSliceMap = new HashMap<>();
             for (int i = 0; i < numSlices; i++) {
                 for (int j = 0; j < numShards; j++) {
-                    SliceBuilder slice = new SliceBuilder("_uid", i, numSlices);
+                    SliceBuilder slice = new SliceBuilder("_id", i, numSlices);
                     Query q = slice.toFilter(context, j, numShards);
                     if (q instanceof TermsSliceQuery || q instanceof MatchAllDocsQuery) {
                         AtomicInteger count = numSliceMap.get(j);
@@ -254,7 +255,7 @@ public Query existsQuery(QueryShardContext context) {
             List<Integer> targetShards = new ArrayList<>();
             for (int i = 0; i < numSlices; i++) {
                 for (int j = 0; j < numShards; j++) {
-                    SliceBuilder slice = new SliceBuilder("_uid", i, numSlices);
+                    SliceBuilder slice = new SliceBuilder("_id", i, numSlices);
                     Query q = slice.toFilter(context, j, numShards);
                     if (q instanceof MatchNoDocsQuery == false) {
                         assertThat(q, instanceOf(MatchAllDocsQuery.class));
@@ -270,7 +271,7 @@ public Query existsQuery(QueryShardContext context) {
             numSlices = numShards;
             for (int i = 0; i < numSlices; i++) {
                 for (int j = 0; j < numShards; j++) {
-                    SliceBuilder slice = new SliceBuilder("_uid", i, numSlices);
+                    SliceBuilder slice = new SliceBuilder("_id", i, numSlices);
                     Query q = slice.toFilter(context, j, numShards);
                     if (i == j) {
                         assertThat(q, instanceOf(MatchAllDocsQuery.class));
@@ -311,4 +312,68 @@ public Query existsQuery(QueryShardContext context) {
             assertThat(exc.getMessage(), containsString("cannot load numeric doc values"));
         }
     }
+
+
+    public void testToFilterDeprecationMessage() throws IOException {
+        Directory dir = new RAMDirectory();
+        try (IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(new MockAnalyzer(random())))) {
+            writer.commit();
+        }
+        QueryShardContext context = mock(QueryShardContext.class);
+        try (IndexReader reader = DirectoryReader.open(dir)) {
+            MappedFieldType fieldType = new MappedFieldType() {
+                @Override
+                public MappedFieldType clone() {
+                    return null;
+                }
+
+                @Override
+                public String typeName() {
+                    return null;
+                }
+
+                @Override
+                public Query termQuery(Object value, @Nullable QueryShardContext context) {
+                    return null;
+                }
+
+                public Query existsQuery(QueryShardContext context) {
+                    return null;
+                }
+            };
+            fieldType.setName(UidFieldMapper.NAME);
+            fieldType.setHasDocValues(false);
+            when(context.fieldMapper(UidFieldMapper.NAME)).thenReturn(fieldType);
+            when(context.getIndexReader()).thenReturn(reader);
+            Settings settings = Settings.builder()
+                    .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
+                    .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 2)
+                    .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0)
+                    .build();
+            IndexMetaData indexState = IndexMetaData.builder("index").settings(settings).build();
+            IndexSettings indexSettings = new IndexSettings(indexState, Settings.EMPTY);
+            when(context.getIndexSettings()).thenReturn(indexSettings);
+            SliceBuilder builder = new SliceBuilder("_uid", 5, 10);
+            Query query = builder.toFilter(context, 0, 1);
+            assertThat(query, instanceOf(TermsSliceQuery.class));
+            assertThat(builder.toFilter(context, 0, 1), equalTo(query));
+            assertWarnings("Computing slices on the [_uid] field is deprecated for 6.x indices, use [_id] instead");
+        }
+
+    }
+
+    public void testSerializationBackcompat() throws IOException {
+        SliceBuilder sliceBuilder = new SliceBuilder(1, 5);
+        assertEquals(IdFieldMapper.NAME, sliceBuilder.getField());
+
+        SliceBuilder copy62 = copyWriteable(sliceBuilder,
+                new NamedWriteableRegistry(Collections.emptyList()),
+                SliceBuilder::new, Version.V_6_2_0);
+        assertEquals(sliceBuilder, copy62);
+
+        SliceBuilder copy63 = copyWriteable(copy62,
+                new NamedWriteableRegistry(Collections.emptyList()),
+                SliceBuilder::new, Version.V_6_3_0);
+        assertEquals(sliceBuilder, copy63);
+    }
 }