Sets the new engine before closing the previous one in IndexShard.res…

…etEngine (#122255) Current code closes the previous engine instance, creates the new engine and then updates the currentEngineReference: IOUtils.close(currentEngine); var newEngine = createEngine(engineConfig); currentEngineReference.set(newEngine); This leaves more room for callers of getEngineOrNull() to pick a closed instance. Instead we can create the new engine first and atomically update the reference to the new instance.
elastic · Feb 12, 2025 · f3b3297 · f3b3297
1 parent 49eccbe
commit f3b3297
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 10 deletions.
diff --git a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java
@@ -4313,17 +4313,15 @@ public void resetEngine() {
         assert waitForEngineOrClosedShardListeners.isDone();
         try {
             synchronized (engineMutex) {
-                final var currentEngine = getEngine();
-                currentEngine.prepareForEngineReset();
-                var engineConfig = newEngineConfig(replicationTracker);
                 verifyNotClosed();
-                IOUtils.close(currentEngine);
-                var newEngine = createEngine(engineConfig);
-                currentEngineReference.set(newEngine);
+                getEngine().prepareForEngineReset();
+                var newEngine = createEngine(newEngineConfig(replicationTracker));
+                IOUtils.close(currentEngineReference.getAndSet(newEngine));
                 onNewEngine(newEngine);
             }
             onSettingsChanged();
         } catch (Exception e) {
+            // we want to fail the shard in the case prepareForEngineReset throws
             failShard("unable to reset engine", e);
         }
     }

diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java
@@ -4563,11 +4563,9 @@ public void testResetEngine() throws Exception {
         var newEngineCreated = new CountDownLatch(2);
         var indexShard = newStartedShard(true, Settings.EMPTY, config -> {
             try {
-                return new ReadOnlyEngine(config, null, null, true, Function.identity(), true, true) {
+                return new ReadOnlyEngine(config, null, new TranslogStats(), false, Function.identity(), true, true) {
                     @Override
-                    public void prepareForEngineReset() throws IOException {
-                        ;
-                    }
+                    public void prepareForEngineReset() throws IOException {}
                 };
             } finally {
                 newEngineCreated.countDown();