Browse Source

Fix FileSettingsService hang on error update (#89630)

Nikola Grcevski 3 years ago
parent
commit
94d7a31873

+ 5 - 0
docs/changelog/89630.yaml

@@ -0,0 +1,5 @@
+pr: 89630
+summary: Fix `FileSettingsService` hang on error update
+area: Infra/Core
+type: bug
+issues: []

+ 6 - 5
server/src/main/java/org/elasticsearch/reservedstate/service/ReservedClusterStateService.java

@@ -102,7 +102,7 @@ public class ReservedClusterStateService {
             stateChunk = stateChunkParser.apply(parser, null);
         } catch (Exception e) {
             ErrorState errorState = new ErrorState(namespace, -1L, e, ReservedStateErrorMetadata.ErrorKind.PARSING);
-            saveErrorState(errorState);
+            saveErrorState(clusterService.state(), errorState);
             logger.debug("error processing state change request for [{}] with the following errors [{}]", namespace, errorState);
 
             errorListener.accept(
@@ -137,7 +137,7 @@ public class ReservedClusterStateService {
                 ReservedStateErrorMetadata.ErrorKind.PARSING
             );
 
-            saveErrorState(errorState);
+            saveErrorState(clusterService.state(), errorState);
             logger.debug("error processing state change request for [{}] with the following errors [{}]", namespace, errorState);
 
             errorListener.accept(
@@ -156,7 +156,7 @@ public class ReservedClusterStateService {
                 reservedStateChunk,
                 handlers,
                 orderedHandlers,
-                (errorState) -> saveErrorState(errorState),
+                (clusterState, errorState) -> saveErrorState(clusterState, errorState),
                 new ActionListener<>() {
                     @Override
                     public void onResponse(ActionResponse.Empty empty) {
@@ -170,6 +170,8 @@ public class ReservedClusterStateService {
                         if (isNewError(existingMetadata, reservedStateVersion.version())) {
                             logger.debug("Failed to apply reserved cluster state", e);
                             errorListener.accept(e);
+                        } else {
+                            errorListener.accept(null);
                         }
                     }
                 }
@@ -186,8 +188,7 @@ public class ReservedClusterStateService {
             || existingMetadata.errorMetadata().version() < newStateVersion);
     }
 
-    private void saveErrorState(ErrorState errorState) {
-        ClusterState clusterState = clusterService.state();
+    private void saveErrorState(ClusterState clusterState, ErrorState errorState) {
         ReservedStateMetadata existingMetadata = clusterState.metadata().reservedStateMetadata().get(errorState.namespace());
 
         if (isNewError(existingMetadata, errorState.version()) == false) {

+ 4 - 4
server/src/main/java/org/elasticsearch/reservedstate/service/ReservedStateUpdateTask.java

@@ -28,7 +28,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.function.Consumer;
+import java.util.function.BiConsumer;
 
 import static org.elasticsearch.ExceptionsHelper.stackTrace;
 import static org.elasticsearch.core.Strings.format;
@@ -47,7 +47,7 @@ public class ReservedStateUpdateTask implements ClusterStateTaskListener {
     private final ReservedStateChunk stateChunk;
     private final Map<String, ReservedClusterStateHandler<?>> handlers;
     private final Collection<String> orderedHandlers;
-    private final Consumer<ErrorState> errorReporter;
+    private final BiConsumer<ClusterState, ErrorState> errorReporter;
     private final ActionListener<ActionResponse.Empty> listener;
 
     public ReservedStateUpdateTask(
@@ -55,7 +55,7 @@ public class ReservedStateUpdateTask implements ClusterStateTaskListener {
         ReservedStateChunk stateChunk,
         Map<String, ReservedClusterStateHandler<?>> handlers,
         Collection<String> orderedHandlers,
-        Consumer<ErrorState> errorReporter,
+        BiConsumer<ClusterState, ErrorState> errorReporter,
         ActionListener<ActionResponse.Empty> listener
     ) {
         this.namespace = namespace;
@@ -112,7 +112,7 @@ public class ReservedStateUpdateTask implements ClusterStateTaskListener {
                 ReservedStateErrorMetadata.ErrorKind.VALIDATION
             );
 
-            errorReporter.accept(errorState);
+            errorReporter.accept(currentState, errorState);
 
             throw new IllegalStateException("Error processing state change request for " + namespace + ", errors: " + errorState);
         }

+ 2 - 2
server/src/test/java/org/elasticsearch/reservedstate/service/ReservedClusterStateServiceTests.java

@@ -143,7 +143,7 @@ public class ReservedClusterStateServiceTests extends ESTestCase {
                 null,
                 Collections.emptyMap(),
                 Collections.emptySet(),
-                (errorState) -> {},
+                (clusterState, errorState) -> {},
                 new ActionListener<>() {
                     @Override
                     public void onResponse(ActionResponse.Empty empty) {}
@@ -329,7 +329,7 @@ public class ReservedClusterStateServiceTests extends ESTestCase {
             new ReservedStateChunk(Map.of("one", "two", "maker", "three"), new ReservedStateVersion(2L, Version.CURRENT)),
             Map.of(exceptionThrower.name(), exceptionThrower, newStateMaker.name(), newStateMaker),
             List.of(exceptionThrower.name(), newStateMaker.name()),
-            (errorState) -> { assertFalse(ReservedClusterStateService.isNewError(operatorMetadata, errorState.version())); },
+            (clusterState, errorState) -> { assertFalse(ReservedClusterStateService.isNewError(operatorMetadata, errorState.version())); },
             new ActionListener<>() {
                 @Override
                 public void onResponse(ActionResponse.Empty empty) {}