Versioning: Delete on an already deleted document should still affect…

… versioning, closes elastic#1341.
amckinley · Sep 16, 2011 · 5ba6ec5 · 5ba6ec5
1 parent bdfa079
commit 5ba6ec5
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 4 deletions.
diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/engine/robin/RobinEngine.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/engine/robin/RobinEngine.java
@@ -654,11 +654,15 @@ private void innerDelete(Delete delete, IndexWriter writer) throws IOException {
                 }
 
                 if (currentVersion == -1) {
-                    // if the doc does not exists, just update with doc 0
-                    delete.version(0).notFound(true);
+                    // doc does not exists and no prior deletes
+                    delete.version(updatedVersion).notFound(true);
+                    Translog.Location translogLocation = translog.add(new Translog.Delete(delete));
+                    versionMap.put(delete.uid().text(), new VersionValue(updatedVersion, true, threadPool.estimatedTimeInMillis(), translogLocation));
                 } else if (versionValue != null && versionValue.delete()) {
-                    // if its a delete on delete and we have the current delete version, return it
-                    delete.version(versionValue.version()).notFound(true);
+                    // a "delete on delete", in this case, we still increment the version, log it, and return that version
+                    delete.version(updatedVersion).notFound(true);
+                    Translog.Location translogLocation = translog.add(new Translog.Delete(delete));
+                    versionMap.put(delete.uid().text(), new VersionValue(updatedVersion, true, threadPool.estimatedTimeInMillis(), translogLocation));
                 } else {
                     delete.version(updatedVersion);
                     writer.deleteDocuments(delete.uid());

diff --git a/...on/src/test/java/org/elasticsearch/test/integration/versioning/SimpleVersioningTests.java b/...on/src/test/java/org/elasticsearch/test/integration/versioning/SimpleVersioningTests.java
@@ -21,6 +21,7 @@
 
 import org.elasticsearch.ElasticSearchException;
 import org.elasticsearch.action.bulk.BulkResponse;
+import org.elasticsearch.action.delete.DeleteResponse;
 import org.elasticsearch.action.index.IndexResponse;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.client.Client;
@@ -85,6 +86,20 @@ public class SimpleVersioningTests extends AbstractNodesTests {
         for (int i = 0; i < 10; i++) {
             assertThat(client.prepareGet("test", "type", "1").execute().actionGet().version(), equalTo(14l));
         }
+
+        DeleteResponse deleteResponse = client2.prepareDelete("test", "type", "1").setVersion(17).setVersionType(VersionType.EXTERNAL).execute().actionGet();
+        assertThat(deleteResponse.notFound(), equalTo(false));
+        assertThat(deleteResponse.version(), equalTo(17l));
+
+        try {
+            client2.prepareDelete("test", "type", "1").setVersion(2).setVersionType(VersionType.EXTERNAL).execute().actionGet();
+        } catch (ElasticSearchException e) {
+            assertThat(e.unwrapCause(), instanceOf(VersionConflictEngineException.class));
+        }
+
+        deleteResponse = client2.prepareDelete("test", "type", "1").setVersion(18).setVersionType(VersionType.EXTERNAL).execute().actionGet();
+        assertThat(deleteResponse.notFound(), equalTo(true));
+        assertThat(deleteResponse.version(), equalTo(18l));
     }
 
     @Test public void testSimpleVersioning() throws Exception {
@@ -154,6 +169,20 @@ public class SimpleVersioningTests extends AbstractNodesTests {
             SearchResponse searchResponse = client.prepareSearch().setQuery(matchAllQuery()).execute().actionGet();
             assertThat(searchResponse.hits().getAt(0).version(), equalTo(-1l));
         }
+
+        DeleteResponse deleteResponse = client2.prepareDelete("test", "type", "1").setVersion(2).execute().actionGet();
+        assertThat(deleteResponse.notFound(), equalTo(false));
+        assertThat(deleteResponse.version(), equalTo(3l));
+
+        try {
+            client2.prepareDelete("test", "type", "1").setVersion(2).execute().actionGet();
+        } catch (ElasticSearchException e) {
+            assertThat(e.unwrapCause(), instanceOf(VersionConflictEngineException.class));
+        }
+
+        deleteResponse = client2.prepareDelete("test", "type", "1").setVersion(3).execute().actionGet();
+        assertThat(deleteResponse.notFound(), equalTo(true));
+        assertThat(deleteResponse.version(), equalTo(4l));
     }
 
     @Test public void testSimpleVersioningWithFlush() throws Exception {