From 5ba6ec5a6988e0eac1a7e20f631b3023a1d8efcb Mon Sep 17 00:00:00 2001 From: Shay Banon Date: Sat, 17 Sep 2011 00:49:21 +0300 Subject: [PATCH] Versioning: Delete on an already deleted document should still affect versioning, closes #1341. --- .../index/engine/robin/RobinEngine.java | 12 +++++--- .../versioning/SimpleVersioningTests.java | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+), 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 index cda00abf594..8e1b4550162 100644 --- 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 @@ public class RobinEngine extends AbstractIndexShardComponent implements Engine { } 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/modules/test/integration/src/test/java/org/elasticsearch/test/integration/versioning/SimpleVersioningTests.java b/modules/test/integration/src/test/java/org/elasticsearch/test/integration/versioning/SimpleVersioningTests.java index 0bd3df0819b..de9f1ab00f9 100644 --- a/modules/test/integration/src/test/java/org/elasticsearch/test/integration/versioning/SimpleVersioningTests.java +++ b/modules/test/integration/src/test/java/org/elasticsearch/test/integration/versioning/SimpleVersioningTests.java @@ -21,6 +21,7 @@ package org.elasticsearch.test.integration.versioning; 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 {