honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix assertion error when caching the result of a search in a read-only index (#41900) 

The ReadOnlyEngine wraps its reader with a SoftDeletesDirectoryReaderWrapper if soft deletes
are enabled. However the wrapping is done on top of the ElasticsearchDirectoryReader and that
trips assertion later on since the cache key of these directories are different. This commit
changes the order of the wrapping to put the ElasticsearchDirectoryReader first in order to
ensure that it is always retrieved first when we unwrap the directory.

Closes #41795
This commit is contained in:
Jim Ferenczi 2019-05-09 08:46:34 +02:00 committed by jimczi
parent 83fef23fd1
commit a329aaec90
2 changed files with 14 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java
View File

@ -157,11 +157,11 @@ public class ReadOnlyEngine extends Engine {
protected final DirectoryReader wrapReader(DirectoryReader reader,
Function<DirectoryReader, DirectoryReader> readerWrapperFunction) throws IOException {
reader = ElasticsearchDirectoryReader.wrap(reader, engineConfig.getShardId());
if (engineConfig.getIndexSettings().isSoftDeleteEnabled()) {
reader = new SoftDeletesDirectoryReaderWrapper(reader, Lucene.SOFT_DELETES_FIELD);
}
return readerWrapperFunction.apply(reader);
reader = readerWrapperFunction.apply(reader);
return ElasticsearchDirectoryReader.wrap(reader, engineConfig.getShardId());
}
protected DirectoryReader open(IndexCommit commit) throws IOException {

13
server/src/test/java/org/elasticsearch/index/engine/ReadOnlyEngineTests.java
View File

@ -18,9 +18,12 @@
*/
package org.elasticsearch.index.engine;
import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.IndexReader;
import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.Version;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.index.mapper.ParsedDocument;
import org.elasticsearch.index.seqno.SeqNoStats;
@ -32,7 +35,9 @@ import java.util.List;
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Function;
import static org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader.getElasticsearchDirectoryReader;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
public class ReadOnlyEngineTests extends EngineTestCase {
@ -80,6 +85,13 @@ public class ReadOnlyEngineTests extends EngineTestCase {
Engine.Searcher external = readOnlyEngine.acquireSearcher("test", Engine.SearcherScope.EXTERNAL);
Engine.Searcher internal = readOnlyEngine.acquireSearcher("test", Engine.SearcherScope.INTERNAL);
assertSame(external.reader(), internal.reader());
assertThat(external.reader(), instanceOf(DirectoryReader.class));
DirectoryReader dirReader = external.getDirectoryReader();
ElasticsearchDirectoryReader esReader = getElasticsearchDirectoryReader(dirReader);
IndexReader.CacheHelper helper = esReader.getReaderCacheHelper();
assertNotNull(helper);
assertEquals(helper.getKey(), dirReader.getReaderCacheHelper().getKey());
IOUtils.close(external, internal);
// the locked down engine should still point to the previous commit
assertThat(readOnlyEngine.getLocalCheckpoint(), equalTo(lastSeqNoStats.getLocalCheckpoint()));
@ -88,7 +100,6 @@ public class ReadOnlyEngineTests extends EngineTestCase {
try (Engine.GetResult getResult = readOnlyEngine.get(get, readOnlyEngine::acquireSearcher)) {
assertTrue(getResult.exists());
}
}
// Close and reopen the main engine
try (InternalEngine recoveringEngine = new InternalEngine(config)) {