Use soft-update to maintain document history #29458
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,8 @@ | |
| import org.apache.lucene.document.StoredField; | ||
| import org.apache.lucene.document.TextField; | ||
| import org.apache.lucene.index.DirectoryReader; | ||
| import org.apache.lucene.index.FilterDirectoryReader; | ||
| import org.apache.lucene.index.FilterLeafReader; | ||
| import org.apache.lucene.index.IndexCommit; | ||
| import org.apache.lucene.index.IndexReader; | ||
| import org.apache.lucene.index.IndexWriter; | ||
|
|
@@ -119,6 +121,7 @@ | |
| import org.elasticsearch.index.translog.Translog; | ||
| import org.elasticsearch.index.translog.TranslogConfig; | ||
| import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; | ||
| import org.elasticsearch.test.IndexSettingsModule; | ||
| import org.hamcrest.MatcherAssert; | ||
| import org.hamcrest.Matchers; | ||
|
|
||
|
|
@@ -504,7 +507,7 @@ public void testSegmentsWithMergeFlag() throws Exception { | |
|
|
||
| if (flush) { | ||
| // we should have had just 1 merge, so last generation should be exact | ||
| assertEquals(gen2 + 1, store.readLastCommittedSegmentsInfo().getLastGeneration()); | ||
|
There was a problem hiding this comment. hmm why does this need to change? |
||
| } | ||
| } | ||
| } | ||
|
|
@@ -2711,6 +2714,12 @@ private void maybeThrowFailure() throws IOException { | |
| } | ||
| } | ||
|
|
||
| @Override | ||
| public long softUpdateDocument(Term term, Iterable<? extends IndexableField> doc, Field... softDeletes) throws IOException { | ||
| maybeThrowFailure(); | ||
| return super.softUpdateDocument(term, doc, softDeletes); | ||
|
|
||
| } | ||
|
|
||
| @Override | ||
| public long deleteDocuments(Term... terms) throws IOException { | ||
| maybeThrowFailure(); | ||
|
|
@@ -4537,6 +4546,41 @@ public void testTrimUnsafeCommits() throws Exception { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * A simple test checks that the document history is maintained when a document is updated. | ||
| */ | ||
| public void testUpdateMaintainDocumentHistory() throws Exception { | ||
| final IndexMetaData indexMetaData = IndexMetaData.builder("test") | ||
| .settings(settings(Version.CURRENT).put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), true)) | ||
| .numberOfShards(1).numberOfReplicas(1).build(); | ||
| final IndexSettings indexSettings = IndexSettingsModule.newIndexSettings(indexMetaData); | ||
| final String docId = "doc-id"; | ||
| final int versions = between(2, 20); | ||
| try (Store store = createStore(); | ||
| InternalEngine engine = createEngine(indexSettings, store, createTempDir(), new LogDocMergePolicy())) { | ||
| final ParsedDocument doc = testParsedDocument(docId, null, testDocument(), new BytesArray("{}"), null); | ||
| for (int version = 1; version <= versions; version++) { | ||
| Engine.IndexResult indexResult = engine.index(indexForDoc(doc)); | ||
| assertThat(indexResult.getFailure(), nullValue()); | ||
| if (randomBoolean()) { | ||
| engine.flush(); | ||
| } | ||
| } | ||
| engine.refresh("test"); | ||
| try (Searcher searcher = engine.acquireSearcher("test", Engine.SearcherScope.INTERNAL)) { | ||
| assertThat(searcher.reader().numDocs(), equalTo(1)); | ||
| assertThat(searcher.reader().numDeletedDocs(), equalTo(versions - 1)); | ||
| try (Engine.GetResult get = engine.get(new Engine.Get(true, false, doc.type(), doc.id(), newUid(doc)), engine::acquireSearcher)) { | ||
| assertThat((int) get.version(), equalTo(versions)); | ||
| } | ||
| // Use a reader which includes soft-deleted documents to verify history. | ||
| final UnfilteredReader unfilteredReader = new UnfilteredReader(searcher.getDirectoryReader()); | ||
| assertThat(unfilteredReader.numDocs(), equalTo(versions)); | ||
| assertThat(unfilteredReader.numDeletedDocs(), equalTo(0)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static void trimUnsafeCommits(EngineConfig config) throws IOException { | ||
| final Store store = config.getStore(); | ||
| final TranslogConfig translogConfig = config.getTranslogConfig(); | ||
|
|
@@ -4555,4 +4599,50 @@ void assertLuceneOperations(InternalEngine engine, long expectedAppends, long ex | |
| assertThat(message, engine.getNumDocUpdates(), equalTo(expectedUpdates)); | ||
| assertThat(message, engine.getNumDocDeletes(), equalTo(expectedDeletes)); | ||
| } | ||
|
|
||
| /** | ||
| * A reader that does not exclude soft-deleted documents. | ||
| */ | ||
| static final class UnfilteredReader extends FilterDirectoryReader { | ||
| static final class UnfilteredSubReaderWrapper extends SubReaderWrapper { | ||
| @Override | ||
| public LeafReader wrap(LeafReader in) { | ||
| return new FilterLeafReader(in) { | ||
| @Override | ||
| public CacheHelper getCoreCacheHelper() { | ||
|
There was a problem hiding this comment. this can return There was a problem hiding this comment. I think you can just move this entire thing to a util method in Lucene.java then we can reuse it in other places WDYT? @martijnvg you need this too somewhere right? There was a problem hiding this comment. I removed the test and this wrapper in this PR, and will add it back to Lucene.java later. I prefer to keep this change as a cut-over. |
||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public CacheHelper getReaderCacheHelper() { | ||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public int numDocs() { | ||
| return maxDoc(); | ||
| } | ||
|
|
||
| @Override | ||
| public Bits getLiveDocs() { | ||
| return null; | ||
| } | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| UnfilteredReader(DirectoryReader in) throws IOException { | ||
| super(in, new UnfilteredSubReaderWrapper()); | ||
| } | ||
|
|
||
| @Override | ||
| protected DirectoryReader doWrapDirectoryReader(DirectoryReader in) throws IOException { | ||
| return new UnfilteredReader(in); | ||
| } | ||
|
|
||
| @Override | ||
| public CacheHelper getReaderCacheHelper() { | ||
| return null; // we are modifying live docs. | ||
| } | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
call this
newSoftDeletesField?