Introduce Maintenance Mode to ILM #31164

talevy · 2018-06-07T00:26:49Z

This PR introduces a concept of a maintenance mode for the
lifecycle service. During maintenance mode, no policies are
executed.

To be placed into maintenance mode, users must first issue a
request to be placed in maintenance mode. Once the service
is assured that no policies are in actions that are not to be
interrupted (like ShrinkAction), the service will place itself
in maintenance mode.

APIs introduced in follow-up PR

POST _xpack/index_lifecycle/maintenance/_request
- issues a request to be placed into maintenenance mode.
  This is not immediate, since we must first verify that
  it is safe to go from REQUESTED -> IN maintenance mode.
POST _xpack/index_lifecycle/maintenance/_stop
- issues a request to be taken out (this is immediate)
GET _xpack/index_lifecycle/maintenance
- get back the current mode our lifecycle management is in

This PR introduces a concept of a maintenance mode for the lifecycle service. During maintenance mode, no policies are executed. To be placed into maintenance mode, users must first issue a request to be placed in maintenance mode. Once the service is assured that no policies are in actions that are not to be interrupted (like ShrinkAction), the service will place itself in maintenance mode. APIs introduced: - POST _xpack/index_lifecycle/maintenance/_request - issues a request to be placed into maintenenance mode. This is not immediate, since we must first verify that it is safe to go from REQUESTED -> IN maintenance mode. - POST _xpack/index_lifecycle/maintenance/_stop - issues a request to be taken out (this is immediate) - GET _xpack/index_lifecycle/maintenance - get back the current mode our lifecycle management is in

elasticmachine · 2018-06-07T00:26:51Z

Pinging @elastic/es-core-infra

talevy · 2018-06-07T00:30:11Z

Hi @colings86, I'd appreciate a review (even though it is WIP)!

Things I have not implemented yet, that may or may not make sense in this PR:

Actions for the endpoints
more tests

Things that I found contentious:

I chose to go with IndexMetaData for storing the MaintenanceMode since it felt wrong to be
a setting that changes under the hood from the user (and we do not want people to edit it directly)
- I notice that I am missing something from Diff logic, those cluster-state diff tests are failing, help there would be appreciated!

any early feedback would be much appreciated, thanks!

talevy · 2018-06-12T06:26:28Z

@colings86 for the sake of moving things forward (and since this is already a large enough PR), I will move the API work to follow-up PRs. A review for what has been done so far would be great!

colings86

I left some comments but I think it looks pretty good so far.

colings86 · 2018-06-12T08:58:26Z

...dex-lifecycle/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleRunner.java

 import org.elasticsearch.xpack.core.indexlifecycle.ErrorStep;
 import org.elasticsearch.xpack.core.indexlifecycle.InitializePolicyContextStep;
 import org.elasticsearch.xpack.core.indexlifecycle.LifecycleSettings;
+import org.elasticsearch.xpack.core.indexlifecycle.ShrinkAction;


I don't think this import is needed?

colings86 · 2018-06-12T09:03:13Z

...ifecycle/src/main/java/org/elasticsearch/xpack/indexlifecycle/MaintenanceModeUpdateTask.java

+
+        boolean maintenanceModeToChange = currentMetadata.getMaintenanceMode().equals(mode) == false;
+        boolean maintenanceModeRequested = OperationMode.MAINTENANCE_REQUESTED.equals(mode);
+        boolean inMaintenanceMode = OperationMode.MAINTENANCE.equals(currentMetadata.getMaintenanceMode());


We should do == checks on enums rather than .equals checks

colings86 · 2018-06-12T09:06:09Z

...ifecycle/src/main/java/org/elasticsearch/xpack/indexlifecycle/MaintenanceModeUpdateTask.java

+        boolean inMaintenanceMode = OperationMode.MAINTENANCE.equals(currentMetadata.getMaintenanceMode());
+        if ((inMaintenanceMode && maintenanceModeRequested) || maintenanceModeToChange == false) {
+            return currentState;
+        }


I think we also need to check that if mode == OperationMode.MAINTENANCE then currentMetadata.getMaintenanceMode() == OperationMode.MAINTENANCE_REQUESTED is also true so we don't end up with a race condition where the user requests NORMAL mode while we are iterating through the indices in IndexLifecycleService and when it finishes iterating it suddenly goes into MAINTENANCE mode ignoring the fact that the user cancelled the request

in other words, you can only enter maintenance mode if you were previously in maintenance-requested mode

and in other words, the state transition for operation modes looks like this

<current_mode> → [<next_mode>, ...] NORMAL → [NORMAL, MAINTENANCE_REQUESTED] MAINTENANCE_REQUESTED → [NORMAL, MAINTENANCE] MAINTENANCE → [NORMAL]

talevy · 2018-06-12T17:38:36Z

thanks @colings86, I've made the valid mode transitions clearer and more restrictive.

colings86

I left a comment about the valid mode changes but otherwise I think this is very close.

colings86 · 2018-06-13T08:58:14Z

...ifecycle/src/main/java/org/elasticsearch/xpack/indexlifecycle/MaintenanceModeUpdateTask.java

+    static {
+        VALID_MODE_CHANGES.put(OperationMode.NORMAL, Sets.newHashSet(OperationMode.MAINTENANCE_REQUESTED));
+        VALID_MODE_CHANGES.put(OperationMode.MAINTENANCE_REQUESTED, Sets.newHashSet(OperationMode.NORMAL, OperationMode.MAINTENANCE));
+        VALID_MODE_CHANGES.put(OperationMode.MAINTENANCE, Sets.newHashSet(OperationMode.NORMAL));


Instead of having these here, we could utilise the OperationMode enum and pass the valid next modes into each enum via a constructor then we could have a isValidChange(OperationMode) method on it which check the mode in the parameter exists in the list of valid modes on that enum value. This will mean that the OperationMode enum completely owns the state transitions

hmm, I tried this but it got just as complicated because the constructor elements cannot be forward references. and I preferred not to use strings.

Instead, I'll move all the logic to this isValidChange method. which will encapsulate the state transitions allowed using switch statements and conditionals

colings86

LGTM

- POST _xpack/index_lifecycle/maintenance/_request - issues a request to be placed into maintenenance mode. This is not immediate, since we must first verify that it is safe to go from REQUESTED -> IN maintenance mode. - POST _xpack/index_lifecycle/maintenance/_stop - issues a request to be taken out (this is immediate) - GET _xpack/index_lifecycle/maintenance - get back the current mode our lifecycle management is in - maintenance mode update task was hardened to support uninstalled metadata - if no metadata is installed, the request/stop actions will install metadata and proceed to try and change it (default start mode is NORMAL) follow-up to elastic#31164.

- POST _xpack/index_lifecycle/_stop - issues a request to be placed into STOPPED mode (maintenance mode). This is not immediate, since we must first verify that it is safe to go from STOPPING -> STOPPED. - POST _xpack/index_lifecycle/_start - issues a request to be placed back into RUNNING mode (immediately) - GET _xpack/index_lifecycle/_status - get back the current mode our lifecycle management is in - update task was hardened to support uninstalled metadata - if no metadata is installed, the start/stop actions will install metadata and proceed to try and change it (default start mode is RUNNING) - rename MAINTENANCE -> STOPPED, MAINTENANCE_REQUESTED -> STOPPING, NORMAL -> RUNNING follow-up to #31164.

This PR introduces a concept of a maintenance mode for the lifecycle service. During maintenance mode, no policies are executed. To be placed into maintenance mode, users must first issue a request to be placed in maintenance mode. Once the service is assured that no policies are in actions that are not to be interrupted (like ShrinkAction), the service will place itself in maintenance mode. APIs to-be introduced: - POST _xpack/index_lifecycle/maintenance/_request - issues a request to be placed into maintenenance mode. This is not immediate, since we must first verify that it is safe to go from REQUESTED -> IN maintenance mode. - POST _xpack/index_lifecycle/maintenance/_stop - issues a request to be taken out (this is immediate) - GET _xpack/index_lifecycle/maintenance - get back the current mode our lifecycle management is in

- POST _xpack/index_lifecycle/_stop - issues a request to be placed into STOPPED mode (maintenance mode). This is not immediate, since we must first verify that it is safe to go from STOPPING -> STOPPED. - POST _xpack/index_lifecycle/_start - issues a request to be placed back into RUNNING mode (immediately) - GET _xpack/index_lifecycle/_status - get back the current mode our lifecycle management is in - update task was hardened to support uninstalled metadata - if no metadata is installed, the start/stop actions will install metadata and proceed to try and change it (default start mode is RUNNING) - rename MAINTENANCE -> STOPPED, MAINTENANCE_REQUESTED -> STOPPING, NORMAL -> RUNNING follow-up to #31164.

talevy added WIP :Data Management/ILM+SLM Index and Snapshot lifecycle management labels Jun 7, 2018

talevy requested a review from colings86 June 7, 2018 00:27

talevy added 2 commits June 7, 2018 12:11

clean up

05e4282

add tests

fa46f10

talevy force-pushed the ilm-maintenance branch from dda2da9 to fa46f10 Compare June 12, 2018 06:25

colings86 requested changes Jun 12, 2018

View reviewed changes

talevy removed the WIP label Jun 12, 2018

cleanup valid mode changes

b184347

colings86 requested changes Jun 13, 2018

View reviewed changes

talevy added 2 commits June 13, 2018 07:56

Merge branch 'index-lifecycle' into ilm-maintenance

668dc5c

clean up operation mode validation

cfbb240

colings86 approved these changes Jun 13, 2018

View reviewed changes

talevy added 10 commits June 13, 2018 09:53

Merge branch 'index-lifecycle' into ilm-maintenance

141d323

Merge branch 'index-lifecycle' into ilm-maintenance

7cbcab4

Merge branch 'index-lifecycle' into ilm-maintenance

7ecb484

Merge branch 'index-lifecycle' into ilm-maintenance

71a6563

Merge branch 'index-lifecycle' into ilm-maintenance

a7676d4

remove unused imports

cbb7d9c

remove more unused imports

b442636

make triggered policies more resilient to nulls

8f29e1c

Merge branch 'index-lifecycle' into ilm-maintenance

4a8f839

Merge branch 'index-lifecycle' into ilm-maintenance

67822f4

talevy merged commit 0193979 into elastic:index-lifecycle Jun 15, 2018

talevy deleted the ilm-maintenance branch June 15, 2018 18:38

talevy mentioned this pull request Jun 18, 2018

add APIs to Maintenance Mode in ILM #31410

Merged

Introduce Maintenance Mode to ILM #31164

Introduce Maintenance Mode to ILM #31164

Uh oh!

Conversation

talevy commented Jun 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Jun 7, 2018

Uh oh!

talevy commented Jun 7, 2018

Uh oh!

talevy commented Jun 12, 2018

Uh oh!

colings86 left a comment

Choose a reason for hiding this comment

Uh oh!

colings86 Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

colings86 Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

colings86 Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

talevy Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

colings86 Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

talevy Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

talevy commented Jun 12, 2018

Uh oh!

colings86 left a comment

Choose a reason for hiding this comment

Uh oh!

colings86 Jun 13, 2018

Choose a reason for hiding this comment

Uh oh!

talevy Jun 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

colings86 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

talevy commented Jun 7, 2018 •

edited

Loading

talevy Jun 13, 2018 •

edited

Loading