From 8355a093ebbeaeabd803b2013278168db85c2866 Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Wed, 10 Apr 2019 15:22:08 -0500 Subject: [PATCH 1/3] [ML] checking if p-tasks metadata is null before updating state --- .../org/elasticsearch/xpack/ml/MlConfigMigrator.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrator.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrator.java index e48cdc999ce8f..9986bf41bab19 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrator.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrator.java @@ -247,10 +247,14 @@ public ClusterState execute(ClusterState currentState) { currentState.metaData().custom(PersistentTasksCustomMetaData.TYPE), currentState.nodes()); ClusterState.Builder newState = ClusterState.builder(currentState); - newState.metaData(MetaData.builder(currentState.getMetaData()) - .putCustom(MlMetadata.TYPE, removed.mlMetadata) - .putCustom(PersistentTasksCustomMetaData.TYPE, updatedTasks) - .build()); + MetaData.Builder metaDataBuilder = MetaData.builder(currentState.getMetaData()) + .putCustom(MlMetadata.TYPE, removed.mlMetadata); + + // If there are no tasks in the cluster state metadata to begin with, this could be null. + if (updatedTasks != null) { + metaDataBuilder = metaDataBuilder.putCustom(PersistentTasksCustomMetaData.TYPE, updatedTasks); + } + newState.metaData(metaDataBuilder.build()); return newState.build(); } From 3a89790f3d04f733471dd85f369c8ee5d94f9a06 Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Thu, 11 Apr 2019 08:09:43 -0500 Subject: [PATCH 2/3] Adding test that validates fix --- .../xpack/ml/integration/MlConfigMigratorIT.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/MlConfigMigratorIT.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/MlConfigMigratorIT.java index 61761706a1a33..134518e35062c 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/MlConfigMigratorIT.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/MlConfigMigratorIT.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.ml.integration; +import com.carrotsearch.hppc.cursors.ObjectCursor; import org.elasticsearch.Version; import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.index.IndexRequestBuilder; @@ -48,10 +49,12 @@ import java.io.IOException; import java.io.InputStream; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.SortedMap; import java.util.TreeMap; @@ -149,9 +152,14 @@ public void testMigrateConfigs() throws InterruptedException, IOException { .routingTable(routingTable.build()) .build(); when(clusterService.state()).thenReturn(clusterState); - + List customs = new ArrayList<>(); doAnswer(invocation -> { ClusterStateUpdateTask listener = (ClusterStateUpdateTask) invocation.getArguments()[1]; + ClusterState result = listener.execute(clusterState); + System.out.println(result); + for (ObjectCursor value : result.metaData().customs().values()){ + customs.add(value.value); + } listener.clusterStateProcessed("source", mock(ClusterState.class), mock(ClusterState.class)); return null; }).when(clusterService).submitStateUpdateTask(eq("remove-migrated-ml-configs"), any()); @@ -165,6 +173,9 @@ public void testMigrateConfigs() throws InterruptedException, IOException { blockingCall(actionListener -> mlConfigMigrator.migrateConfigs(clusterState, actionListener), responseHolder, exceptionHolder); + // Verify that we have custom values in the new cluster state and that none of them is null + assertThat(customs.size(), greaterThan(0)); + assertThat(customs.stream().anyMatch(Objects::isNull), is(false)); assertNull(exceptionHolder.get()); assertTrue(responseHolder.get()); assertSnapshot(mlMetadata.build()); From 305d95fa05e6d3b60245211e42af5cedbbc80de6 Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Thu, 11 Apr 2019 08:22:47 -0500 Subject: [PATCH 3/3] removing debug println --- .../elasticsearch/xpack/ml/integration/MlConfigMigratorIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/MlConfigMigratorIT.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/MlConfigMigratorIT.java index 134518e35062c..bc5befb390f61 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/MlConfigMigratorIT.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/MlConfigMigratorIT.java @@ -156,7 +156,6 @@ public void testMigrateConfigs() throws InterruptedException, IOException { doAnswer(invocation -> { ClusterStateUpdateTask listener = (ClusterStateUpdateTask) invocation.getArguments()[1]; ClusterState result = listener.execute(clusterState); - System.out.println(result); for (ObjectCursor value : result.metaData().customs().values()){ customs.add(value.value); }