From c345e54909876188f3c6c8648b8107cafff16efa Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 6 Jun 2018 17:36:32 -0600 Subject: [PATCH 01/21] Add a NoopEngine implementation This adds a new Engine implementation that does.. nothing. Any operations throw an `UnsupportedOperationException` when tried. This engine is intended as a building block for replicated closed indices in subsequent code changes. Relates to #31141 --- .../elasticsearch/index/engine/Engine.java | 7 +- .../index/engine/EngineFactory.java | 1 + .../index/engine/InternalEngine.java | 5 + .../index/engine/InternalEngineFactory.java | 5 + .../index/engine/NoopEngine.java | 339 ++++++++++++++++++ .../index/engine/NoopEngineTests.java | 75 ++++ .../IndexLevelReplicationTests.java | 5 + .../RecoveryDuringReplicationTests.java | 5 + .../index/engine/EngineTestCase.java | 4 + .../test/engine/MockEngineFactory.java | 6 + 10 files changed, 451 insertions(+), 1 deletion(-) create mode 100644 server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java create mode 100644 server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java diff --git a/server/src/main/java/org/elasticsearch/index/engine/Engine.java b/server/src/main/java/org/elasticsearch/index/engine/Engine.java index 314eeffd7aa6a..e1c38eece43ca 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/Engine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/Engine.java @@ -838,7 +838,7 @@ private void fillSegmentInfo(SegmentReader segmentReader, boolean verbose, boole */ public abstract List segments(boolean verbose); - public final boolean refreshNeeded() { + public boolean refreshNeeded() { if (store.tryIncRef()) { /* we need to inc the store here since we acquire a searcher and that might keep a file open on the @@ -1631,4 +1631,9 @@ public boolean isRecovering() { * Tries to prune buffered deletes from the version map. */ public abstract void maybePruneDeletes(); + + /** + * Returns true if the engine is a noop engine + */ + public abstract boolean isNoopEngine(); } diff --git a/server/src/main/java/org/elasticsearch/index/engine/EngineFactory.java b/server/src/main/java/org/elasticsearch/index/engine/EngineFactory.java index b477e27b6e150..eabca8ad7c0e3 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/EngineFactory.java +++ b/server/src/main/java/org/elasticsearch/index/engine/EngineFactory.java @@ -25,4 +25,5 @@ public interface EngineFactory { Engine newReadWriteEngine(EngineConfig config); + Engine newNoopEngine(EngineConfig config); } diff --git a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index bca84f81a29c4..fa1fabed03aad 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -1252,6 +1252,11 @@ public void maybePruneDeletes() { } } + @Override + public boolean isNoopEngine() { + return false; + } + @Override public NoOpResult noOp(final NoOp noOp) { NoOpResult noOpResult; diff --git a/server/src/main/java/org/elasticsearch/index/engine/InternalEngineFactory.java b/server/src/main/java/org/elasticsearch/index/engine/InternalEngineFactory.java index d151bcf49ce95..f7c58ef1b20be 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/InternalEngineFactory.java +++ b/server/src/main/java/org/elasticsearch/index/engine/InternalEngineFactory.java @@ -24,4 +24,9 @@ public class InternalEngineFactory implements EngineFactory { public Engine newReadWriteEngine(EngineConfig config) { return new InternalEngine(config); } + + @Override + public Engine newNoopEngine(EngineConfig config) { + return new NoopEngine(config); + } } diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java new file mode 100644 index 0000000000000..bc7ab5f5671ec --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -0,0 +1,339 @@ +/* + * 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.index.engine; + +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.IndexCommit; +import org.apache.lucene.index.SegmentInfos; +import org.elasticsearch.common.Nullable; +import org.elasticsearch.core.internal.io.IOUtils; +import org.elasticsearch.index.seqno.LocalCheckpointTracker; +import org.elasticsearch.index.seqno.SequenceNumbers; +import org.elasticsearch.index.translog.Translog; +import org.elasticsearch.index.translog.TranslogConfig; +import org.elasticsearch.index.translog.TranslogCorruptedException; +import org.elasticsearch.index.translog.TranslogDeletionPolicy; + +import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.function.BiFunction; +import java.util.function.LongSupplier; +import java.util.stream.Stream; + +/** + * NoopEngine is an engine implementation that does nothing but the bare minimum + * required in order to have an engine. All attempts to do something (search, + * index, get), throw {@link UnsupportedOperationException}. This does maintain + * a translog with a deletion policy so that when flushing, no translog is + * retained on disk (setting a retention size and age of 0). + * + * It's also important to notice that this does list the commits of the Store's + * Directory so that the last commit's user data can be read for the historyUUID + * and last committed segment info. + */ +public class NoopEngine extends Engine { + + private final Translog translog; + private final IndexCommit lastCommit; + private final LocalCheckpointTracker localCheckpointTracker; + private final String historyUUID; + private SegmentInfos lastCommittedSegmentInfos; + + public NoopEngine(EngineConfig engineConfig) { + super(engineConfig); + + store.incRef(); + boolean success = false; + Translog translog = null; + + // The deletion policy for the translog should not keep any translogs around, so the min age/size is set to -1 + final TranslogDeletionPolicy translogDeletionPolicy = new TranslogDeletionPolicy(-1, -1); + + try { + lastCommittedSegmentInfos = store.readLastCommittedSegmentsInfo(); + translog = openTranslog(engineConfig, translogDeletionPolicy, engineConfig.getGlobalCheckpointSupplier()); + assert translog.getGeneration() != null; + this.translog = translog; + List indexCommits = DirectoryReader.listCommits(store.directory()); + lastCommit = indexCommits.get(indexCommits.size()-1); + historyUUID = lastCommit.getUserData().get(HISTORY_UUID_KEY); + // We don't want any translogs hanging around for recovery, so we need to set these accordingly + final long lastGen = Long.parseLong(lastCommit.getUserData().get(Translog.TRANSLOG_GENERATION_KEY)); + translogDeletionPolicy.setTranslogGenerationOfLastCommit(lastGen); + translogDeletionPolicy.setMinTranslogGenerationForRecovery(lastGen); + + localCheckpointTracker = createLocalCheckpointTracker(); + success = true; + } catch (IOException | TranslogCorruptedException e) { + throw new EngineCreationFailureException(shardId, "failed to create engine", e); + } finally { + if (success == false) { + IOUtils.closeWhileHandlingException(translog); + if (isClosed.get() == false) { + // failure we need to dec the store reference + store.decRef(); + } + } + } + logger.trace("created new NoopEngine"); + } + + private Translog openTranslog(EngineConfig engineConfig, TranslogDeletionPolicy translogDeletionPolicy, + LongSupplier globalCheckpointSupplier) throws IOException { + final TranslogConfig translogConfig = engineConfig.getTranslogConfig(); + final String translogUUID = loadTranslogUUIDFromLastCommit(); + // We expect that this shard already exists, so it must already have an existing translog else something is badly wrong! + return new Translog(translogConfig, translogUUID, translogDeletionPolicy, globalCheckpointSupplier, + engineConfig.getPrimaryTermSupplier()); + } + + /** + * Reads the current stored translog ID from the last commit data. + */ + @Nullable + private String loadTranslogUUIDFromLastCommit() { + final Map commitUserData = lastCommittedSegmentInfos.getUserData(); + if (commitUserData.containsKey(Translog.TRANSLOG_GENERATION_KEY) == false) { + throw new IllegalStateException("commit doesn't contain translog generation id"); + } + return commitUserData.get(Translog.TRANSLOG_UUID_KEY); + } + + private LocalCheckpointTracker createLocalCheckpointTracker() { + final long maxSeqNo; + final long localCheckpoint; + final SequenceNumbers.CommitInfo seqNoStats = + SequenceNumbers.loadSeqNoInfoFromLuceneCommit(lastCommittedSegmentInfos.userData.entrySet()); + maxSeqNo = seqNoStats.maxSeqNo; + localCheckpoint = seqNoStats.localCheckpoint; + logger.trace("recovered maximum sequence number [{}] and local checkpoint [{}]", maxSeqNo, localCheckpoint); + return new LocalCheckpointTracker(maxSeqNo, localCheckpoint); + } + + @Override + protected SegmentInfos getLastCommittedSegmentInfos() { + return lastCommittedSegmentInfos; + } + + @Override + public String getHistoryUUID() { + return historyUUID; + } + + @Override + public long getWritingBytes() { + return 0; + } + + @Override + public boolean isNoopEngine() { + return true; + } + + @Override + public long getIndexThrottleTimeInMillis() { + return 0; + } + + @Override + public boolean isThrottled() { + return false; + } + + @Override + public IndexResult index(Index index) { + throw new UnsupportedOperationException("indexing is not supported on a noop engine"); + } + + @Override + public DeleteResult delete(Delete delete) { + throw new UnsupportedOperationException("deletion is not supported on a noop engine"); + } + + @Override + public NoOpResult noOp(NoOp noOp) { + throw new UnsupportedOperationException("noop is not supported on a noop engine"); + } + + @Override + public SyncedFlushResult syncFlush(String syncId, CommitId expectedCommitId) throws EngineException { + throw new UnsupportedOperationException("synced flush is not supported on a noop engine"); + } + + @Override + public GetResult get(Get get, BiFunction searcherFactory) throws EngineException { + throw new UnsupportedOperationException("gets are not supported on a noop engine"); + } + + @Override + public Searcher acquireSearcher(String source, SearcherScope scope) throws EngineException { + throw new UnsupportedOperationException("searching is not supported on a noop engine"); + } + + @Override + public Translog getTranslog() { + return translog; + } + + @Override + public boolean ensureTranslogSynced(Stream locations) { + return false; + } + + @Override + public void syncTranslog() { + } + + @Override + public LocalCheckpointTracker getLocalCheckpointTracker() { + return localCheckpointTracker; + } + + @Override + public long getIndexBufferRAMBytesUsed() { + return 0; + } + + @Override + public List segments(boolean verbose) { + return Arrays.asList(getSegmentInfo(lastCommittedSegmentInfos, verbose)); + } + + @Override + public void refresh(String source) throws EngineException { + } + + // Override the refreshNeeded method so that we don't attempt to acquire a searcher checking if we need to refresh + @Override + public boolean refreshNeeded() { + // We never need to refresh a noop engine so always return false + return false; + } + + @Override + public void writeIndexingBuffer() throws EngineException { + } + + @Override + public boolean shouldPeriodicallyFlush() { + return false; + } + + @Override + public CommitId flush(boolean force, boolean waitIfOngoing) throws EngineException { + return new CommitId(lastCommittedSegmentInfos.getId()); + } + + @Override + public CommitId flush() throws EngineException { + try { + translog.rollGeneration(); + translog.trimUnreferencedReaders(); + } catch (IOException e) { + maybeFailEngine("flush", e); + throw new FlushFailedEngineException(shardId, e); + } + return new CommitId(lastCommittedSegmentInfos.getId()); + } + + @Override + public void trimTranslog() throws EngineException { + } + + @Override + public void rollTranslogGeneration() throws EngineException { + } + + @Override + public void forceMerge(boolean flush, int maxNumSegments, boolean onlyExpungeDeletes, boolean upgrade, + boolean upgradeOnlyAncientSegments) throws EngineException { + } + + @Override + public IndexCommitRef acquireLastIndexCommit(boolean flushFirst) throws EngineException { + return new Engine.IndexCommitRef(lastCommit, () -> {}); + } + + @Override + public IndexCommitRef acquireSafeIndexCommit() throws EngineException { + return acquireLastIndexCommit(false); + } + + /** + * Closes the engine without acquiring the write lock. This should only be + * called while the write lock is hold or in a disaster condition ie. if the engine + * is failed. + */ + @Override + protected final void closeNoLock(String reason, CountDownLatch closedLatch) { + if (isClosed.compareAndSet(false, true)) { + assert rwl.isWriteLockedByCurrentThread() || failEngineLock.isHeldByCurrentThread() : + "Either the write lock must be held or the engine must be currently be failing itself"; + try { + IOUtils.close(translog); + } catch (Exception e) { + logger.warn("Failed to close translog", e); + } finally { + try { + store.decRef(); + logger.debug("engine closed [{}]", reason); + } finally { + closedLatch.countDown(); + } + } + } + } + + @Override + public void activateThrottling() { + throw new UnsupportedOperationException("closed engine can't throttle"); + } + + @Override + public void deactivateThrottling() { + throw new UnsupportedOperationException("closed engine can't throttle"); + } + + @Override + public void restoreLocalCheckpointFromTranslog() { + + } + + @Override + public int fillSeqNoGaps(long primaryTerm) { + return 0; + } + + @Override + public Engine recoverFromTranslog() { + return this; + } + + @Override + public void skipTranslogRecovery() { + } + + @Override + public void maybePruneDeletes() { + } +} diff --git a/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java new file mode 100644 index 0000000000000..65bea28529167 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java @@ -0,0 +1,75 @@ +/* + * 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.index.engine; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.mapper.ParsedDocument; +import org.elasticsearch.test.IndexSettingsModule; + +import java.io.IOException; + +import static org.hamcrest.Matchers.equalTo; + +public class NoopEngineTests extends EngineTestCase { + private static final IndexSettings INDEX_SETTINGS = IndexSettingsModule.newIndexSettings("index", Settings.EMPTY); + + public void testNoopEngine() throws IOException { + engine.close(); + final NoopEngine engine = new NoopEngine(noopConfig(INDEX_SETTINGS, store, primaryTranslogDir)); + expectThrows(UnsupportedOperationException.class, () -> engine.index(null)); + expectThrows(UnsupportedOperationException.class, () -> engine.delete(null)); + expectThrows(UnsupportedOperationException.class, () -> engine.noOp(null)); + expectThrows(UnsupportedOperationException.class, () -> engine.syncFlush(null, null)); + expectThrows(UnsupportedOperationException.class, () -> engine.get(null, null)); + expectThrows(UnsupportedOperationException.class, () -> engine.acquireSearcher(null, null)); + expectThrows(UnsupportedOperationException.class, engine::activateThrottling); + expectThrows(UnsupportedOperationException.class, engine::deactivateThrottling); + assertThat(engine.refreshNeeded(), equalTo(false)); + assertThat(engine.shouldPeriodicallyFlush(), equalTo(false)); + assertThat(engine.isNoopEngine(), equalTo(true)); + engine.close(); + } + + public void testTwoNoopEngines() throws IOException { + engine.close(); + // It's so noop you can even open two engines for the same store without tripping anything + final NoopEngine engine1 = new NoopEngine(noopConfig(INDEX_SETTINGS, store, primaryTranslogDir)); + final NoopEngine engine2 = new NoopEngine(noopConfig(INDEX_SETTINGS, store, primaryTranslogDir)); + engine1.close(); + engine2.close(); + } + + public void testFlushingTranslogRemovesTranslogOperations() throws IOException { + ParsedDocument doc = testParsedDocument("1", null, testDocument(), B_1, null); + Engine.Index index = indexForDoc(doc); + Engine.IndexResult indexResult = engine.index(index); + assertTrue(indexResult.isCreated()); + engine.flush(true, true); + engine.close(); + + final NoopEngine noopEngine = new NoopEngine(noopConfig(INDEX_SETTINGS, store, primaryTranslogDir)); + assertThat(noopEngine.getTranslog().totalOperations(), equalTo(1)); + noopEngine.flush(); + assertThat(noopEngine.getTranslog().totalOperations(), equalTo(0)); + noopEngine.close(); + } + +} diff --git a/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java b/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java index 736dc40e6867d..6ea485ed63588 100644 --- a/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java +++ b/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java @@ -460,6 +460,11 @@ public long addDocument(Iterable doc) throws IOExcepti } }, null, null, config); } + + @Override + public Engine newNoopEngine(EngineConfig config) { + throw new UnsupportedOperationException("not used"); + } } private static void assertNoOpTranslogOperationForDocumentFailure( diff --git a/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java b/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java index a34963a475155..8c3fb6ffde6a1 100644 --- a/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java +++ b/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java @@ -714,6 +714,11 @@ public long addDocument(final Iterable doc) throws IOE config); } + @Override + public Engine newNoopEngine(EngineConfig config) { + throw new UnsupportedOperationException("not used"); + } + @Override public void close() throws Exception { releaseLatchedIndexers(); diff --git a/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java index 8fff17900b072..a851a584ba739 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java @@ -457,6 +457,10 @@ public void onFailedEngine(String reason, @Nullable Exception e) { return config; } + protected EngineConfig noopConfig(IndexSettings indexSettings, Store store, Path translogPath) { + return config(indexSettings, store, translogPath, newMergePolicy(), null, null, null); + } + protected static final BytesReference B_1 = new BytesArray(new byte[]{1}); protected static final BytesReference B_2 = new BytesArray(new byte[]{2}); protected static final BytesReference B_3 = new BytesArray(new byte[]{3}); diff --git a/test/framework/src/main/java/org/elasticsearch/test/engine/MockEngineFactory.java b/test/framework/src/main/java/org/elasticsearch/test/engine/MockEngineFactory.java index 2956e44d50799..12bfee28a46a9 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/engine/MockEngineFactory.java +++ b/test/framework/src/main/java/org/elasticsearch/test/engine/MockEngineFactory.java @@ -22,6 +22,7 @@ import org.elasticsearch.index.engine.Engine; import org.elasticsearch.index.engine.EngineConfig; import org.elasticsearch.index.engine.EngineFactory; +import org.elasticsearch.index.engine.NoopEngine; public final class MockEngineFactory implements EngineFactory { @@ -35,4 +36,9 @@ public MockEngineFactory(Class wrapper) { public Engine newReadWriteEngine(EngineConfig config) { return new MockInternalEngine(config, wrapper); } + + @Override + public Engine newNoopEngine(EngineConfig config) { + return new NoopEngine(config); + } } From 7d469cc157785ff10eb22a6cd3d536893a6faea0 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 6 Jun 2018 18:25:04 -0600 Subject: [PATCH 02/21] Remove isNoopEngine() method --- .../src/main/java/org/elasticsearch/index/engine/Engine.java | 4 ---- .../java/org/elasticsearch/index/engine/InternalEngine.java | 5 ----- .../main/java/org/elasticsearch/index/engine/NoopEngine.java | 5 ----- 3 files changed, 14 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/Engine.java b/server/src/main/java/org/elasticsearch/index/engine/Engine.java index e1c38eece43ca..e8ac292dc7e06 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/Engine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/Engine.java @@ -1632,8 +1632,4 @@ public boolean isRecovering() { */ public abstract void maybePruneDeletes(); - /** - * Returns true if the engine is a noop engine - */ - public abstract boolean isNoopEngine(); } diff --git a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index fa1fabed03aad..bca84f81a29c4 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -1252,11 +1252,6 @@ public void maybePruneDeletes() { } } - @Override - public boolean isNoopEngine() { - return false; - } - @Override public NoOpResult noOp(final NoOp noOp) { NoOpResult noOpResult; diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java index bc7ab5f5671ec..d1aaf773ec2b6 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -145,11 +145,6 @@ public long getWritingBytes() { return 0; } - @Override - public boolean isNoopEngine() { - return true; - } - @Override public long getIndexThrottleTimeInMillis() { return 0; From 5c0ed00f0a23a5838062ae58fc35c02177b053c6 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 6 Jun 2018 18:48:12 -0600 Subject: [PATCH 03/21] Remove test using isNoopEngine --- .../java/org/elasticsearch/index/engine/NoopEngineTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java index 65bea28529167..21b7377adea47 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java @@ -44,7 +44,6 @@ public void testNoopEngine() throws IOException { expectThrows(UnsupportedOperationException.class, engine::deactivateThrottling); assertThat(engine.refreshNeeded(), equalTo(false)); assertThat(engine.shouldPeriodicallyFlush(), equalTo(false)); - assertThat(engine.isNoopEngine(), equalTo(true)); engine.close(); } From 2985692d24091cb3432ce60496f4a8182140c7f7 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 7 Jun 2018 11:43:02 -0600 Subject: [PATCH 04/21] Remove additional EngineFactory method --- .../java/org/elasticsearch/index/engine/EngineFactory.java | 1 - .../elasticsearch/index/engine/InternalEngineFactory.java | 5 ----- .../index/replication/IndexLevelReplicationTests.java | 5 ----- .../index/replication/RecoveryDuringReplicationTests.java | 5 ----- .../org/elasticsearch/test/engine/MockEngineFactory.java | 6 ------ 5 files changed, 22 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/EngineFactory.java b/server/src/main/java/org/elasticsearch/index/engine/EngineFactory.java index eabca8ad7c0e3..b477e27b6e150 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/EngineFactory.java +++ b/server/src/main/java/org/elasticsearch/index/engine/EngineFactory.java @@ -25,5 +25,4 @@ public interface EngineFactory { Engine newReadWriteEngine(EngineConfig config); - Engine newNoopEngine(EngineConfig config); } diff --git a/server/src/main/java/org/elasticsearch/index/engine/InternalEngineFactory.java b/server/src/main/java/org/elasticsearch/index/engine/InternalEngineFactory.java index f7c58ef1b20be..d151bcf49ce95 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/InternalEngineFactory.java +++ b/server/src/main/java/org/elasticsearch/index/engine/InternalEngineFactory.java @@ -24,9 +24,4 @@ public class InternalEngineFactory implements EngineFactory { public Engine newReadWriteEngine(EngineConfig config) { return new InternalEngine(config); } - - @Override - public Engine newNoopEngine(EngineConfig config) { - return new NoopEngine(config); - } } diff --git a/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java b/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java index 6ea485ed63588..736dc40e6867d 100644 --- a/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java +++ b/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java @@ -460,11 +460,6 @@ public long addDocument(Iterable doc) throws IOExcepti } }, null, null, config); } - - @Override - public Engine newNoopEngine(EngineConfig config) { - throw new UnsupportedOperationException("not used"); - } } private static void assertNoOpTranslogOperationForDocumentFailure( diff --git a/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java b/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java index 8c3fb6ffde6a1..a34963a475155 100644 --- a/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java +++ b/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java @@ -714,11 +714,6 @@ public long addDocument(final Iterable doc) throws IOE config); } - @Override - public Engine newNoopEngine(EngineConfig config) { - throw new UnsupportedOperationException("not used"); - } - @Override public void close() throws Exception { releaseLatchedIndexers(); diff --git a/test/framework/src/main/java/org/elasticsearch/test/engine/MockEngineFactory.java b/test/framework/src/main/java/org/elasticsearch/test/engine/MockEngineFactory.java index 12bfee28a46a9..2956e44d50799 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/engine/MockEngineFactory.java +++ b/test/framework/src/main/java/org/elasticsearch/test/engine/MockEngineFactory.java @@ -22,7 +22,6 @@ import org.elasticsearch.index.engine.Engine; import org.elasticsearch.index.engine.EngineConfig; import org.elasticsearch.index.engine.EngineFactory; -import org.elasticsearch.index.engine.NoopEngine; public final class MockEngineFactory implements EngineFactory { @@ -36,9 +35,4 @@ public MockEngineFactory(Class wrapper) { public Engine newReadWriteEngine(EngineConfig config) { return new MockInternalEngine(config, wrapper); } - - @Override - public Engine newNoopEngine(EngineConfig config) { - return new NoopEngine(config); - } } From 19b50b02669f7419588f3ef1140dcdd81ef01cbb Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 7 Jun 2018 13:41:43 -0600 Subject: [PATCH 05/21] Remove newline --- .../src/main/java/org/elasticsearch/index/engine/NoopEngine.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java index d1aaf773ec2b6..a215e9bff1a96 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -311,7 +311,6 @@ public void deactivateThrottling() { @Override public void restoreLocalCheckpointFromTranslog() { - } @Override From 7d8e9dec46fcd8e1ad59d148548352ed14b9ff4f Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 7 Jun 2018 13:42:03 -0600 Subject: [PATCH 06/21] Move translog deletion policy creation into try block --- .../java/org/elasticsearch/index/engine/NoopEngine.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java index a215e9bff1a96..a0876f6517ccc 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -66,10 +66,10 @@ public NoopEngine(EngineConfig engineConfig) { boolean success = false; Translog translog = null; - // The deletion policy for the translog should not keep any translogs around, so the min age/size is set to -1 - final TranslogDeletionPolicy translogDeletionPolicy = new TranslogDeletionPolicy(-1, -1); - try { + // The deletion policy for the translog should not keep any translogs around, so the min age/size is set to -1 + final TranslogDeletionPolicy translogDeletionPolicy = new TranslogDeletionPolicy(-1, -1); + lastCommittedSegmentInfos = store.readLastCommittedSegmentsInfo(); translog = openTranslog(engineConfig, translogDeletionPolicy, engineConfig.getGlobalCheckpointSupplier()); assert translog.getGeneration() != null; From 21de3a479ddb506e9f0a32d60451e15f8e1ad834 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 7 Jun 2018 13:42:34 -0600 Subject: [PATCH 07/21] Make lastCommittedSegmentInfos final --- .../main/java/org/elasticsearch/index/engine/NoopEngine.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java index a0876f6517ccc..e471ebc02955f 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -57,7 +57,7 @@ public class NoopEngine extends Engine { private final IndexCommit lastCommit; private final LocalCheckpointTracker localCheckpointTracker; private final String historyUUID; - private SegmentInfos lastCommittedSegmentInfos; + private final SegmentInfos lastCommittedSegmentInfos; public NoopEngine(EngineConfig engineConfig) { super(engineConfig); From ce634e9afcd6730bc26d1b9a0e55717f931df72a Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 7 Jun 2018 13:43:13 -0600 Subject: [PATCH 08/21] Make NoopEngine final and package private --- .../main/java/org/elasticsearch/index/engine/NoopEngine.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java index e471ebc02955f..9a3e022a78641 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -51,7 +51,7 @@ * Directory so that the last commit's user data can be read for the historyUUID * and last committed segment info. */ -public class NoopEngine extends Engine { +final class NoopEngine extends Engine { private final Translog translog; private final IndexCommit lastCommit; From e49a854c1f4cceed813d12516bf82d95178020bc Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Fri, 8 Jun 2018 10:07:24 -0600 Subject: [PATCH 09/21] Remove translog flushing --- .../elasticsearch/index/engine/NoopEngine.java | 7 ------- .../index/engine/NoopEngineTests.java | 16 ---------------- 2 files changed, 23 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java index 9a3e022a78641..b0e78f5fa67b1 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -241,13 +241,6 @@ public CommitId flush(boolean force, boolean waitIfOngoing) throws EngineExcepti @Override public CommitId flush() throws EngineException { - try { - translog.rollGeneration(); - translog.trimUnreferencedReaders(); - } catch (IOException e) { - maybeFailEngine("flush", e); - throw new FlushFailedEngineException(shardId, e); - } return new CommitId(lastCommittedSegmentInfos.getId()); } diff --git a/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java index 21b7377adea47..160940a2e12ce 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java @@ -21,7 +21,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexSettings; -import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.test.IndexSettingsModule; import java.io.IOException; @@ -56,19 +55,4 @@ public void testTwoNoopEngines() throws IOException { engine2.close(); } - public void testFlushingTranslogRemovesTranslogOperations() throws IOException { - ParsedDocument doc = testParsedDocument("1", null, testDocument(), B_1, null); - Engine.Index index = indexForDoc(doc); - Engine.IndexResult indexResult = engine.index(index); - assertTrue(indexResult.isCreated()); - engine.flush(true, true); - engine.close(); - - final NoopEngine noopEngine = new NoopEngine(noopConfig(INDEX_SETTINGS, store, primaryTranslogDir)); - assertThat(noopEngine.getTranslog().totalOperations(), equalTo(1)); - noopEngine.flush(); - assertThat(noopEngine.getTranslog().totalOperations(), equalTo(0)); - noopEngine.close(); - } - } From 2629329f25edb031d92c86117e2d9f76a22cfec1 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Fri, 8 Jun 2018 10:42:36 -0600 Subject: [PATCH 10/21] Fix checkstyle --- .../main/java/org/elasticsearch/index/engine/NoopEngine.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java index b0e78f5fa67b1..5a3487ffbcb53 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -59,7 +59,7 @@ final class NoopEngine extends Engine { private final String historyUUID; private final SegmentInfos lastCommittedSegmentInfos; - public NoopEngine(EngineConfig engineConfig) { + NoopEngine(EngineConfig engineConfig) { super(engineConfig); store.incRef(); @@ -273,7 +273,7 @@ public IndexCommitRef acquireSafeIndexCommit() throws EngineException { * is failed. */ @Override - protected final void closeNoLock(String reason, CountDownLatch closedLatch) { + protected void closeNoLock(String reason, CountDownLatch closedLatch) { if (isClosed.compareAndSet(false, true)) { assert rwl.isWriteLockedByCurrentThread() || failEngineLock.isHeldByCurrentThread() : "Either the write lock must be held or the engine must be currently be failing itself"; From 191eae0f4e7a6bbf9a9952974f15fdf3bf2c040c Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Fri, 8 Jun 2018 12:16:29 -0600 Subject: [PATCH 11/21] Throw UOE from ensureTranslogSynced --- .../main/java/org/elasticsearch/index/engine/NoopEngine.java | 2 +- .../java/org/elasticsearch/index/engine/NoopEngineTests.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java index 5a3487ffbcb53..185c99bd0f93a 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -192,7 +192,7 @@ public Translog getTranslog() { @Override public boolean ensureTranslogSynced(Stream locations) { - return false; + throw new UnsupportedOperationException("translog synchronization should never be needed"); } @Override diff --git a/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java index 160940a2e12ce..026c261d80bf6 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java @@ -39,6 +39,7 @@ public void testNoopEngine() throws IOException { expectThrows(UnsupportedOperationException.class, () -> engine.syncFlush(null, null)); expectThrows(UnsupportedOperationException.class, () -> engine.get(null, null)); expectThrows(UnsupportedOperationException.class, () -> engine.acquireSearcher(null, null)); + expectThrows(UnsupportedOperationException.class, () -> engine.ensureTranslogSynced(null)); expectThrows(UnsupportedOperationException.class, engine::activateThrottling); expectThrows(UnsupportedOperationException.class, engine::deactivateThrottling); assertThat(engine.refreshNeeded(), equalTo(false)); From 5ed1baedcac92c5e498621ac5b075d887a832076 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Fri, 8 Jun 2018 13:53:09 -0600 Subject: [PATCH 12/21] Reduce visibility of getTranslog --- .../main/java/org/elasticsearch/index/engine/NoopEngine.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java index 185c99bd0f93a..3566ed7aa6f1c 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -186,7 +186,7 @@ public Searcher acquireSearcher(String source, SearcherScope scope) throws Engin } @Override - public Translog getTranslog() { + Translog getTranslog() { return translog; } From 8f59a7e67ecaa6e66682acc876ac11b82964189c Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Mon, 11 Jun 2018 10:51:53 -0600 Subject: [PATCH 13/21] Get rid of LocalCheckpointTracker and Translog in NoopEngine --- .../index/engine/NoopEngine.java | 151 ++++++++++-------- 1 file changed, 84 insertions(+), 67 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java index 3566ed7aa6f1c..384c8ee1a3666 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -22,22 +22,17 @@ import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.IndexCommit; import org.apache.lucene.index.SegmentInfos; -import org.elasticsearch.common.Nullable; -import org.elasticsearch.core.internal.io.IOUtils; -import org.elasticsearch.index.seqno.LocalCheckpointTracker; -import org.elasticsearch.index.seqno.SequenceNumbers; +import org.elasticsearch.index.seqno.SeqNoStats; import org.elasticsearch.index.translog.Translog; -import org.elasticsearch.index.translog.TranslogConfig; import org.elasticsearch.index.translog.TranslogCorruptedException; -import org.elasticsearch.index.translog.TranslogDeletionPolicy; +import org.elasticsearch.index.translog.TranslogStats; +import java.io.Closeable; import java.io.IOException; import java.util.Arrays; import java.util.List; -import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.function.BiFunction; -import java.util.function.LongSupplier; import java.util.stream.Stream; /** @@ -53,9 +48,26 @@ */ final class NoopEngine extends Engine { - private final Translog translog; + private static final Translog.Snapshot EMPTY_TRANSLOG_SNAPSHOT = new Translog.Snapshot() { + @Override + public int totalOperations() { + return 0; + } + + @Override + public Translog.Operation next() { + return null; + } + + @Override + public void close() { + } + }; + + private static final TranslogStats EMPTY_TRANSLOG_STATS = new TranslogStats(0, 0, 0, 0, 0); + private static final Translog.Location EMPTY_TRANSLOG_LOCATION = new Translog.Location(0, 0, 0); + private final IndexCommit lastCommit; - private final LocalCheckpointTracker localCheckpointTracker; private final String historyUUID; private final SegmentInfos lastCommittedSegmentInfos; @@ -64,31 +76,20 @@ final class NoopEngine extends Engine { store.incRef(); boolean success = false; - Translog translog = null; try { - // The deletion policy for the translog should not keep any translogs around, so the min age/size is set to -1 - final TranslogDeletionPolicy translogDeletionPolicy = new TranslogDeletionPolicy(-1, -1); - lastCommittedSegmentInfos = store.readLastCommittedSegmentsInfo(); - translog = openTranslog(engineConfig, translogDeletionPolicy, engineConfig.getGlobalCheckpointSupplier()); - assert translog.getGeneration() != null; - this.translog = translog; List indexCommits = DirectoryReader.listCommits(store.directory()); lastCommit = indexCommits.get(indexCommits.size()-1); historyUUID = lastCommit.getUserData().get(HISTORY_UUID_KEY); // We don't want any translogs hanging around for recovery, so we need to set these accordingly final long lastGen = Long.parseLong(lastCommit.getUserData().get(Translog.TRANSLOG_GENERATION_KEY)); - translogDeletionPolicy.setTranslogGenerationOfLastCommit(lastGen); - translogDeletionPolicy.setMinTranslogGenerationForRecovery(lastGen); - localCheckpointTracker = createLocalCheckpointTracker(); success = true; } catch (IOException | TranslogCorruptedException e) { throw new EngineCreationFailureException(shardId, "failed to create engine", e); } finally { if (success == false) { - IOUtils.closeWhileHandlingException(translog); if (isClosed.get() == false) { // failure we need to dec the store reference store.decRef(); @@ -98,38 +99,6 @@ final class NoopEngine extends Engine { logger.trace("created new NoopEngine"); } - private Translog openTranslog(EngineConfig engineConfig, TranslogDeletionPolicy translogDeletionPolicy, - LongSupplier globalCheckpointSupplier) throws IOException { - final TranslogConfig translogConfig = engineConfig.getTranslogConfig(); - final String translogUUID = loadTranslogUUIDFromLastCommit(); - // We expect that this shard already exists, so it must already have an existing translog else something is badly wrong! - return new Translog(translogConfig, translogUUID, translogDeletionPolicy, globalCheckpointSupplier, - engineConfig.getPrimaryTermSupplier()); - } - - /** - * Reads the current stored translog ID from the last commit data. - */ - @Nullable - private String loadTranslogUUIDFromLastCommit() { - final Map commitUserData = lastCommittedSegmentInfos.getUserData(); - if (commitUserData.containsKey(Translog.TRANSLOG_GENERATION_KEY) == false) { - throw new IllegalStateException("commit doesn't contain translog generation id"); - } - return commitUserData.get(Translog.TRANSLOG_UUID_KEY); - } - - private LocalCheckpointTracker createLocalCheckpointTracker() { - final long maxSeqNo; - final long localCheckpoint; - final SequenceNumbers.CommitInfo seqNoStats = - SequenceNumbers.loadSeqNoInfoFromLuceneCommit(lastCommittedSegmentInfos.userData.entrySet()); - maxSeqNo = seqNoStats.maxSeqNo; - localCheckpoint = seqNoStats.localCheckpoint; - logger.trace("recovered maximum sequence number [{}] and local checkpoint [{}]", maxSeqNo, localCheckpoint); - return new LocalCheckpointTracker(maxSeqNo, localCheckpoint); - } - @Override protected SegmentInfos getLastCommittedSegmentInfos() { return lastCommittedSegmentInfos; @@ -155,6 +124,10 @@ public boolean isThrottled() { return false; } + @Override + public void trimOperationsFromTranslog(long belowTerm, long aboveSeqNo) throws EngineException { + } + @Override public IndexResult index(Index index) { throw new UnsupportedOperationException("indexing is not supported on a noop engine"); @@ -186,8 +159,8 @@ public Searcher acquireSearcher(String source, SearcherScope scope) throws Engin } @Override - Translog getTranslog() { - return translog; + public boolean isTranslogSyncNeeded() { + return false; } @Override @@ -200,8 +173,51 @@ public void syncTranslog() { } @Override - public LocalCheckpointTracker getLocalCheckpointTracker() { - return localCheckpointTracker; + public Closeable acquireTranslogRetentionLock() { + return () -> { }; + } + + @Override + public Translog.Snapshot newTranslogSnapshotFromMinSeqNo(long minSeqNo) { + return EMPTY_TRANSLOG_SNAPSHOT; + } + + @Override + public int estimateTranslogOperationsFromMinSeq(long minSeqNo) { + return 0; + } + + @Override + public TranslogStats getTranslogStats() { + return EMPTY_TRANSLOG_STATS; + } + + @Override + public Translog.Location getTranslogLastWriteLocation() { + return EMPTY_TRANSLOG_LOCATION; + } + + @Override + public long getLocalCheckpoint() { + return 0; + } + + @Override + public void waitForOpsToComplete(long seqNo) { + } + + @Override + public void resetLocalCheckpoint(long localCheckpoint) { + } + + @Override + public SeqNoStats getSeqNoStats(long globalCheckpoint) { + return new SeqNoStats(0, 0, 0); + } + + @Override + public long getLastSyncedGlobalCheckpoint() { + return 0; } @Override @@ -245,7 +261,13 @@ public CommitId flush() throws EngineException { } @Override - public void trimTranslog() throws EngineException { + public void trimUnreferencedTranslogFiles() throws EngineException { + + } + + @Override + public boolean shouldRollTranslogGeneration() { + return false; } @Override @@ -278,17 +300,12 @@ protected void closeNoLock(String reason, CountDownLatch closedLatch) { assert rwl.isWriteLockedByCurrentThread() || failEngineLock.isHeldByCurrentThread() : "Either the write lock must be held or the engine must be currently be failing itself"; try { - IOUtils.close(translog); - } catch (Exception e) { - logger.warn("Failed to close translog", e); + store.decRef(); + logger.debug("engine closed [{}]", reason); } finally { - try { - store.decRef(); - logger.debug("engine closed [{}]", reason); - } finally { - closedLatch.countDown(); - } + closedLatch.countDown(); } + } } From de63a5e97427b180142810eb92d7d0e0b9d222cf Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Mon, 11 Jun 2018 13:19:38 -0600 Subject: [PATCH 14/21] Remove unused lastGen, validate Translog UUID --- .../index/engine/NoopEngine.java | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java index 384c8ee1a3666..62e89c14c796c 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -22,17 +22,22 @@ import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.IndexCommit; import org.apache.lucene.index.SegmentInfos; +import org.elasticsearch.common.Nullable; import org.elasticsearch.index.seqno.SeqNoStats; import org.elasticsearch.index.translog.Translog; +import org.elasticsearch.index.translog.TranslogConfig; import org.elasticsearch.index.translog.TranslogCorruptedException; +import org.elasticsearch.index.translog.TranslogDeletionPolicy; import org.elasticsearch.index.translog.TranslogStats; import java.io.Closeable; import java.io.IOException; import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.function.BiFunction; +import java.util.function.LongSupplier; import java.util.stream.Stream; /** @@ -82,8 +87,13 @@ public void close() { List indexCommits = DirectoryReader.listCommits(store.directory()); lastCommit = indexCommits.get(indexCommits.size()-1); historyUUID = lastCommit.getUserData().get(HISTORY_UUID_KEY); - // We don't want any translogs hanging around for recovery, so we need to set these accordingly - final long lastGen = Long.parseLong(lastCommit.getUserData().get(Translog.TRANSLOG_GENERATION_KEY)); + + // The deletion policy for the translog should not keep any translogs around, so the min age/size is set to -1 + final TranslogDeletionPolicy translogDeletionPolicy = new TranslogDeletionPolicy(-1, -1); + + // The translog is opened and closed to validate that the translog UUID from lucene is the same as the one in the translog + Translog translog = openTranslog(engineConfig, translogDeletionPolicy, engineConfig.getGlobalCheckpointSupplier()); + translog.close(); success = true; } catch (IOException | TranslogCorruptedException e) { @@ -99,6 +109,27 @@ public void close() { logger.trace("created new NoopEngine"); } + private Translog openTranslog(EngineConfig engineConfig, TranslogDeletionPolicy translogDeletionPolicy, + LongSupplier globalCheckpointSupplier) throws IOException { + final TranslogConfig translogConfig = engineConfig.getTranslogConfig(); + final String translogUUID = loadTranslogUUIDFromLastCommit(); + // We expect that this shard already exists, so it must already have an existing translog else something is badly wrong! + return new Translog(translogConfig, translogUUID, translogDeletionPolicy, globalCheckpointSupplier, + engineConfig.getPrimaryTermSupplier()); + } + + /** + * Reads the current stored translog ID from the last commit data. + */ + @Nullable + private String loadTranslogUUIDFromLastCommit() { + final Map commitUserData = lastCommittedSegmentInfos.getUserData(); + if (commitUserData.containsKey(Translog.TRANSLOG_GENERATION_KEY) == false) { + throw new IllegalStateException("commit doesn't contain translog generation id"); + } + return commitUserData.get(Translog.TRANSLOG_UUID_KEY); + } + @Override protected SegmentInfos getLastCommittedSegmentInfos() { return lastCommittedSegmentInfos; From c2c820ad185814e15fd891eff836f388a8ce137d Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Mon, 11 Jun 2018 15:14:03 -0600 Subject: [PATCH 15/21] Read local checkpoint and max seq no out of commit data --- .../org/elasticsearch/index/engine/NoopEngine.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java index 62e89c14c796c..7343794b3b723 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -24,6 +24,7 @@ import org.apache.lucene.index.SegmentInfos; import org.elasticsearch.common.Nullable; import org.elasticsearch.index.seqno.SeqNoStats; +import org.elasticsearch.index.seqno.SequenceNumbers; import org.elasticsearch.index.translog.Translog; import org.elasticsearch.index.translog.TranslogConfig; import org.elasticsearch.index.translog.TranslogCorruptedException; @@ -73,6 +74,8 @@ public void close() { private static final Translog.Location EMPTY_TRANSLOG_LOCATION = new Translog.Location(0, 0, 0); private final IndexCommit lastCommit; + private final long localCheckpoint; + private final long maxSeqNo; private final String historyUUID; private final SegmentInfos lastCommittedSegmentInfos; @@ -87,6 +90,8 @@ public void close() { List indexCommits = DirectoryReader.listCommits(store.directory()); lastCommit = indexCommits.get(indexCommits.size()-1); historyUUID = lastCommit.getUserData().get(HISTORY_UUID_KEY); + localCheckpoint = Long.parseLong(lastCommit.getUserData().get(SequenceNumbers.LOCAL_CHECKPOINT_KEY)); + maxSeqNo = Long.parseLong(lastCommit.getUserData().get(SequenceNumbers.MAX_SEQ_NO)); // The deletion policy for the translog should not keep any translogs around, so the min age/size is set to -1 final TranslogDeletionPolicy translogDeletionPolicy = new TranslogDeletionPolicy(-1, -1); @@ -230,7 +235,7 @@ public Translog.Location getTranslogLastWriteLocation() { @Override public long getLocalCheckpoint() { - return 0; + return this.localCheckpoint; } @Override @@ -239,11 +244,13 @@ public void waitForOpsToComplete(long seqNo) { @Override public void resetLocalCheckpoint(long localCheckpoint) { + assert localCheckpoint == getLocalCheckpoint() : "expected reset to existing local checkpoint of " + + getLocalCheckpoint() + " got: " + localCheckpoint; } @Override public SeqNoStats getSeqNoStats(long globalCheckpoint) { - return new SeqNoStats(0, 0, 0); + return new SeqNoStats(maxSeqNo, localCheckpoint, globalCheckpoint); } @Override From ba56f2593a22aed39627053ac1cabff7794c9e08 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Mon, 11 Jun 2018 16:18:42 -0600 Subject: [PATCH 16/21] Add check for translog operations and test --- .../index/engine/NoopEngine.java | 3 ++ .../index/engine/NoopEngineTests.java | 45 +++++++++++++++++++ .../index/engine/EngineTestCase.java | 6 ++- 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java index 7343794b3b723..47138f3a02b92 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -98,6 +98,9 @@ public void close() { // The translog is opened and closed to validate that the translog UUID from lucene is the same as the one in the translog Translog translog = openTranslog(engineConfig, translogDeletionPolicy, engineConfig.getGlobalCheckpointSupplier()); + if (translog.totalOperations() != 0) { + throw new IllegalArgumentException("expected 0 translog operations but there were " + translog.totalOperations()); + } translog.close(); success = true; diff --git a/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java index 026c261d80bf6..ff13230a3910e 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java @@ -19,11 +19,22 @@ package org.elasticsearch.index.engine; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.IndexReader; +import org.elasticsearch.cluster.routing.AllocationId; +import org.elasticsearch.cluster.routing.IndexShardRoutingTable; +import org.elasticsearch.cluster.routing.ShardRouting; +import org.elasticsearch.cluster.routing.ShardRoutingState; +import org.elasticsearch.cluster.routing.TestShardRouting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.mapper.ParsedDocument; +import org.elasticsearch.index.seqno.ReplicationTracker; +import org.elasticsearch.index.seqno.SequenceNumbers; import org.elasticsearch.test.IndexSettingsModule; import java.io.IOException; +import java.util.Collections; import static org.hamcrest.Matchers.equalTo; @@ -56,4 +67,38 @@ public void testTwoNoopEngines() throws IOException { engine2.close(); } + public void testNoopAfterRegularEngine() throws IOException { + int docs = randomIntBetween(1, 10); + ReplicationTracker tracker = (ReplicationTracker) engine.config().getGlobalCheckpointSupplier(); + ShardRouting routing = TestShardRouting.newShardRouting("test", shardId.id(), "node", null, true, ShardRoutingState.STARTED, allocationId); + IndexShardRoutingTable table = new IndexShardRoutingTable.Builder(shardId).addShard(routing).build(); + tracker.updateFromMaster(1L, Collections.singleton(allocationId.getId()), table, Collections.emptySet()); + tracker.activatePrimaryMode(SequenceNumbers.NO_OPS_PERFORMED); + for (int i = 0; i < docs; i++) { + ParsedDocument doc = testParsedDocument("" + i, null, testDocumentWithTextField(), B_1, null); + engine.index(indexForDoc(doc)); + tracker.updateLocalCheckpoint(allocationId.getId(), i); + } + + engine.flush(true, true); + engine.getTranslog().getDeletionPolicy().setRetentionSizeInBytes(-1); + engine.getTranslog().getDeletionPolicy().setRetentionAgeInMillis(-1); + engine.getTranslog().getDeletionPolicy().setMinTranslogGenerationForRecovery( + engine.getTranslog().getGeneration().translogFileGeneration); + engine.flush(true, true); + + long localCheckpoint = engine.getLocalCheckpoint(); + long maxSeqNo = engine.getSeqNoStats(100L).getMaxSeqNo(); + engine.close(); + + final NoopEngine noopEngine = new NoopEngine(noopConfig(INDEX_SETTINGS, store, primaryTranslogDir, tracker)); + assertThat(noopEngine.getLocalCheckpoint(), equalTo(localCheckpoint)); + assertThat(noopEngine.getSeqNoStats(100L).getMaxSeqNo(), equalTo(maxSeqNo)); + try (Engine.IndexCommitRef ref = noopEngine.acquireLastIndexCommit(false)) { + try (IndexReader reader = DirectoryReader.open(ref.getIndexCommit())) { + assertThat(reader.numDocs(), equalTo(docs)); + } + } + noopEngine.close(); + } } diff --git a/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java index 671ec84d6d308..3946617ec73c1 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java @@ -467,7 +467,11 @@ public void onFailedEngine(String reason, @Nullable Exception e) { } protected EngineConfig noopConfig(IndexSettings indexSettings, Store store, Path translogPath) { - return config(indexSettings, store, translogPath, newMergePolicy(), null, null, null); + return noopConfig(indexSettings, store, translogPath, null); + } + + protected EngineConfig noopConfig(IndexSettings indexSettings, Store store, Path translogPath, LongSupplier globalCheckpointSupplier) { + return config(indexSettings, store, translogPath, newMergePolicy(), null, null, globalCheckpointSupplier); } protected static final BytesReference B_1 = new BytesArray(new byte[]{1}); From 023effbb5bf08bdb7dbb42e30e66dcaf1701723d Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Tue, 12 Jun 2018 08:32:20 -0600 Subject: [PATCH 17/21] Fix checkstyle --- .../java/org/elasticsearch/index/engine/NoopEngineTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java index ff13230a3910e..8d68547b2b43e 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java @@ -70,7 +70,8 @@ public void testTwoNoopEngines() throws IOException { public void testNoopAfterRegularEngine() throws IOException { int docs = randomIntBetween(1, 10); ReplicationTracker tracker = (ReplicationTracker) engine.config().getGlobalCheckpointSupplier(); - ShardRouting routing = TestShardRouting.newShardRouting("test", shardId.id(), "node", null, true, ShardRoutingState.STARTED, allocationId); + ShardRouting routing = TestShardRouting.newShardRouting("test", shardId.id(), "node", + null, true, ShardRoutingState.STARTED, allocationId); IndexShardRoutingTable table = new IndexShardRoutingTable.Builder(shardId).addShard(routing).build(); tracker.updateFromMaster(1L, Collections.singleton(allocationId.getId()), table, Collections.emptySet()); tracker.activatePrimaryMode(SequenceNumbers.NO_OPS_PERFORMED); From a4d2e3b8f9f0f2e26aa8c0195bd650f79cb9ed38 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Tue, 12 Jun 2018 15:02:43 -0600 Subject: [PATCH 18/21] Use try-with-resources for translog opening --- .../java/org/elasticsearch/index/engine/NoopEngine.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java index 47138f3a02b92..883b5919854b9 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -97,11 +97,11 @@ public void close() { final TranslogDeletionPolicy translogDeletionPolicy = new TranslogDeletionPolicy(-1, -1); // The translog is opened and closed to validate that the translog UUID from lucene is the same as the one in the translog - Translog translog = openTranslog(engineConfig, translogDeletionPolicy, engineConfig.getGlobalCheckpointSupplier()); - if (translog.totalOperations() != 0) { - throw new IllegalArgumentException("expected 0 translog operations but there were " + translog.totalOperations()); + try (Translog translog = openTranslog(engineConfig, translogDeletionPolicy, engineConfig.getGlobalCheckpointSupplier())) { + if (translog.totalOperations() != 0) { + throw new IllegalArgumentException("expected 0 translog operations but there were " + translog.totalOperations()); + } } - translog.close(); success = true; } catch (IOException | TranslogCorruptedException e) { From 78ae62dd80cc70ed38fe8dcfceab4dad62c7badb Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 13 Jun 2018 08:04:37 -0600 Subject: [PATCH 19/21] Formatting nit --- .../main/java/org/elasticsearch/index/engine/NoopEngine.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java index 883b5919854b9..c8ee32464fc5b 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoopEngine.java @@ -88,7 +88,7 @@ public void close() { try { lastCommittedSegmentInfos = store.readLastCommittedSegmentsInfo(); List indexCommits = DirectoryReader.listCommits(store.directory()); - lastCommit = indexCommits.get(indexCommits.size()-1); + lastCommit = indexCommits.get(indexCommits.size() - 1); historyUUID = lastCommit.getUserData().get(HISTORY_UUID_KEY); localCheckpoint = Long.parseLong(lastCommit.getUserData().get(SequenceNumbers.LOCAL_CHECKPOINT_KEY)); maxSeqNo = Long.parseLong(lastCommit.getUserData().get(SequenceNumbers.MAX_SEQ_NO)); From 13c06b955eac47957101bac31d4e9ae47e720557 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 13 Jun 2018 11:52:32 -0600 Subject: [PATCH 20/21] Add a test for validating translog UUID --- .../index/engine/NoopEngineTests.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java index 8d68547b2b43e..bf6170809551e 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java @@ -21,7 +21,6 @@ import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.IndexReader; -import org.elasticsearch.cluster.routing.AllocationId; import org.elasticsearch.cluster.routing.IndexShardRoutingTable; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.ShardRoutingState; @@ -31,12 +30,16 @@ import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.seqno.ReplicationTracker; import org.elasticsearch.index.seqno.SequenceNumbers; +import org.elasticsearch.index.translog.Translog; +import org.elasticsearch.index.translog.TranslogCorruptedException; import org.elasticsearch.test.IndexSettingsModule; import java.io.IOException; +import java.nio.file.Path; import java.util.Collections; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; public class NoopEngineTests extends EngineTestCase { private static final IndexSettings INDEX_SETTINGS = IndexSettingsModule.newIndexSettings("index", Settings.EMPTY); @@ -102,4 +105,14 @@ public void testNoopAfterRegularEngine() throws IOException { } noopEngine.close(); } + + public void testNoopEngineWithInvalidTranslogUUID() throws IOException { + Path newTranslogDir = createTempDir(); + // A new translog will have a different UUID than the existing store/noop engine does + Translog newTranslog = createTranslog(newTranslogDir, () -> 1L); + newTranslog.close(); + EngineCreationFailureException e = expectThrows(EngineCreationFailureException.class, + () -> new NoopEngine(noopConfig(INDEX_SETTINGS, store, newTranslogDir))); + assertThat(e.getCause(), instanceOf(TranslogCorruptedException.class)); + } } From 37398ac0f3501651b31cef8fd611b5e84a600191 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 14 Jun 2018 13:31:28 -0600 Subject: [PATCH 21/21] Enhance comment about two noop engine test --- .../java/org/elasticsearch/index/engine/NoopEngineTests.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java index bf6170809551e..814f928e3d7ee 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/NoopEngineTests.java @@ -63,7 +63,9 @@ public void testNoopEngine() throws IOException { public void testTwoNoopEngines() throws IOException { engine.close(); - // It's so noop you can even open two engines for the same store without tripping anything + // It's so noop you can even open two engines for the same store without tripping anything, + // this ensures we're not doing any kind of locking on the store or filesystem level in + // the noop engine final NoopEngine engine1 = new NoopEngine(noopConfig(INDEX_SETTINGS, store, primaryTranslogDir)); final NoopEngine engine2 = new NoopEngine(noopConfig(INDEX_SETTINGS, store, primaryTranslogDir)); engine1.close();