Deleting a document from a non-existing index creates the should not auto create it, unless using EXTERNAL* versioning (#24518)

Currently a `delete document` request against a non-existing index actually **creates** this index.

With this change the `delete document` no longer creates the previously non-existing index and throws an `index_not_found` exception instead.

However as discussed in https://github.com/elastic/elasticsearch/pull/15451#issuecomment-165772026, if an external version is explicitly used, the current behavior is preserved and the index is still created and the document is marked for deletion.

Fixes #15425

client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java

@@ -55,7 +55,6 @@ import org.elasticsearch.threadpool.ThreadPool;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.Map;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 
 import static java.util.Collections.singletonMap;
@@ -63,16 +62,6 @@ import static java.util.Collections.singletonMap;
 public class CrudIT extends ESRestHighLevelClientTestCase {
 
     public void testDelete() throws IOException {
-        {
-            // Testing non existing document
-            String docId = "does_not_exist";
-            DeleteRequest deleteRequest = new DeleteRequest("index", "type", docId);
-            DeleteResponse deleteResponse = execute(deleteRequest, highLevelClient()::delete, highLevelClient()::deleteAsync);
-            assertEquals("index", deleteResponse.getIndex());
-            assertEquals("type", deleteResponse.getType());
-            assertEquals(docId, deleteResponse.getId());
-            assertEquals(DocWriteResponse.Result.NOT_FOUND, deleteResponse.getResult());
-        }
         {
             // Testing deletion
             String docId = "id";
@@ -87,6 +76,16 @@ public class CrudIT extends ESRestHighLevelClientTestCase {
             assertEquals(docId, deleteResponse.getId());
             assertEquals(DocWriteResponse.Result.DELETED, deleteResponse.getResult());
         }
+        {
+            // Testing non existing document
+            String docId = "does_not_exist";
+            DeleteRequest deleteRequest = new DeleteRequest("index", "type", docId);
+            DeleteResponse deleteResponse = execute(deleteRequest, highLevelClient()::delete, highLevelClient()::deleteAsync);
+            assertEquals("index", deleteResponse.getIndex());
+            assertEquals("type", deleteResponse.getType());
+            assertEquals(docId, deleteResponse.getId());
+            assertEquals(DocWriteResponse.Result.NOT_FOUND, deleteResponse.getResult());
+        }
         {
             // Testing version conflict
             String docId = "version_conflict";

core/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java

@@ -54,6 +54,7 @@ import org.elasticsearch.common.util.concurrent.AbstractRunnable;
 import org.elasticsearch.common.util.concurrent.AtomicArray;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexNotFoundException;
+import org.elasticsearch.index.VersionType;
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.indices.IndexClosedException;
 import org.elasticsearch.ingest.IngestService;
@@ -144,6 +145,11 @@ public class TransportBulkAction extends HandledTransportAction<BulkRequest, Bul
             // Attempt to create all the indices that we're going to need during the bulk before we start.
             // Step 1: collect all the indices in the request
             final Set<String> indices = bulkRequest.requests.stream()
+                    // delete requests should not attempt to create the index (if the index does not
+                    // exists), unless an external versioning is used
+                .filter(request -> request.opType() != DocWriteRequest.OpType.DELETE 
+                        || request.versionType() == VersionType.EXTERNAL 
+                        || request.versionType() == VersionType.EXTERNAL_GTE)
                 .map(DocWriteRequest::index)
                 .collect(Collectors.toSet());
             /* Step 2: filter that to indices that don't exist and we can create. At the same time build a map of indices we can't create

core/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIndicesThatCannotBeCreatedTests.java

@@ -31,6 +31,7 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.util.concurrent.AtomicArray;
 import org.elasticsearch.index.IndexNotFoundException;
+import org.elasticsearch.index.VersionType;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.transport.TransportService;
@@ -66,7 +67,7 @@ public class TransportBulkActionIndicesThatCannotBeCreatedTests extends ESTestCa
         BulkRequest bulkRequest = new BulkRequest();
         bulkRequest.add(new IndexRequest("no"));
         bulkRequest.add(new IndexRequest("can't"));
-        bulkRequest.add(new DeleteRequest("do"));
+        bulkRequest.add(new DeleteRequest("do").version(0).versionType(VersionType.EXTERNAL));
         bulkRequest.add(new UpdateRequest("nothin", randomAlphaOfLength(5), randomAlphaOfLength(5)));
         indicesThatCannotBeCreatedTestCase(new HashSet<>(Arrays.asList("no", "can't", "do", "nothin")), bulkRequest, index -> {
             throw new IndexNotFoundException("Can't make it because I say so");

core/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java

@@ -0,0 +1,135 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.action.bulk;
+
+import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
+import org.elasticsearch.action.bulk.TransportBulkActionTookTests.Resolver;
+import org.elasticsearch.action.delete.DeleteRequest;
+import org.elasticsearch.action.support.ActionFilters;
+import org.elasticsearch.action.support.AutoCreateIndex;
+import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.unit.TimeValue;
+import org.elasticsearch.index.IndexNotFoundException;
+import org.elasticsearch.index.VersionType;
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.transport.CapturingTransport;
+import org.elasticsearch.threadpool.TestThreadPool;
+import org.elasticsearch.threadpool.ThreadPool;
+import org.elasticsearch.transport.TransportService;
+import org.junit.After;
+import org.junit.Before;
+
+import java.util.Collections;
+import java.util.concurrent.TimeUnit;
+
+import static org.elasticsearch.test.ClusterServiceUtils.createClusterService;
+
+public class TransportBulkActionTests extends ESTestCase {
+
+    /** Services needed by bulk action */
+    private TransportService transportService;
+    private ClusterService clusterService;
+    private ThreadPool threadPool;
+    
+    private TestTransportBulkAction bulkAction;
+
+    class TestTransportBulkAction extends TransportBulkAction {
+
+        boolean indexCreated = false; // set when the "real" index is created
+
+        TestTransportBulkAction() {
+            super(Settings.EMPTY, TransportBulkActionTests.this.threadPool, transportService, clusterService, null, null,
+                    null, new ActionFilters(Collections.emptySet()), new Resolver(Settings.EMPTY),
+                    new AutoCreateIndex(Settings.EMPTY, clusterService.getClusterSettings(), new Resolver(Settings.EMPTY)));
+        }
+
+        @Override
+        protected boolean needToCheck() {
+            return true;
+        }
+
+        @Override
+        void createIndex(String index, TimeValue timeout, ActionListener<CreateIndexResponse> listener) {
+            indexCreated = true;
+            listener.onResponse(null);
+        }
+    }
+
+    @Before
+    public void setUp() throws Exception {
+        super.setUp();
+        threadPool = new TestThreadPool("TransportBulkActionTookTests");
+        clusterService = createClusterService(threadPool);
+        CapturingTransport capturingTransport = new CapturingTransport();
+        transportService = new TransportService(clusterService.getSettings(), capturingTransport, threadPool,
+                TransportService.NOOP_TRANSPORT_INTERCEPTOR,
+            boundAddress -> clusterService.localNode(), null);
+        transportService.start();
+        transportService.acceptIncomingRequests();
+        bulkAction = new TestTransportBulkAction();
+    }
+
+    @After
+    public void tearDown() throws Exception {
+        ThreadPool.terminate(threadPool, 30, TimeUnit.SECONDS);
+        threadPool = null;
+        clusterService.close();
+        super.tearDown();
+    }
+
+    public void testDeleteNonExistingDocDoesNotCreateIndex() throws Exception {
+        BulkRequest bulkRequest = new BulkRequest().add(new DeleteRequest("index", "type", "id"));
+
+        bulkAction.execute(null, bulkRequest, ActionListener.wrap(response -> {
+            assertFalse(bulkAction.indexCreated);
+            BulkItemResponse[] bulkResponses = ((BulkResponse) response).getItems();
+            assertEquals(bulkResponses.length, 1);
+            assertTrue(bulkResponses[0].isFailed());
+            assertTrue(bulkResponses[0].getFailure().getCause() instanceof IndexNotFoundException);
+            assertEquals("index", bulkResponses[0].getFailure().getIndex());
+        }, exception -> {
+            throw new AssertionError(exception);
+        }));
+    }
+
+    public void testDeleteNonExistingDocExternalVersionCreatesIndex() throws Exception {
+        BulkRequest bulkRequest = new BulkRequest()
+                .add(new DeleteRequest("index", "type", "id").versionType(VersionType.EXTERNAL).version(0));
+
+        bulkAction.execute(null, bulkRequest, ActionListener.wrap(response -> {
+            assertTrue(bulkAction.indexCreated);
+        }, exception -> {
+            throw new AssertionError(exception);
+        }));
+    }
+
+    public void testDeleteNonExistingDocExternalGteVersionCreatesIndex() throws Exception {
+        BulkRequest bulkRequest = new BulkRequest()
+                .add(new DeleteRequest("index2", "type", "id").versionType(VersionType.EXTERNAL_GTE).version(0));
+
+        bulkAction.execute(null, bulkRequest, ActionListener.wrap(response -> {
+            assertTrue(bulkAction.indexCreated);
+        }, exception -> {
+            throw new AssertionError(exception);
+        }));
+    }
+}

docs/reference/docs/delete.asciidoc

@@ -105,7 +105,8 @@ thrown instead.
 [[delete-index-creation]]
 === Automatic index creation
 
-The delete operation automatically creates an index if it has not been
+If an <<docs-index_,external versioning variant>> is used,
+the delete operation automatically creates an index if it has not been
 created before (check out the <<indices-create-index,create index API>>
 for manually creating an index), and also automatically creates a
 dynamic type mapping for the specific type if it has not been created

docs/reference/migration/migrate_6_0/indices.asciidoc

@@ -44,3 +44,9 @@ The default value of the `allow_no_indices` option for the Open/Close index API
 has been changed from `false` to `true` so it is aligned with the behaviour of the
 Delete index API. As a result, Open/Close index API don't return an error by
 default when a provided wildcard expression doesn't match any closed/open index.
+
+==== Delete a document
+
+Delete a document from non-existing index has been modified to not create the index.
+However if an external versioning is used the index will be created and the document
+will be marked for deletion.