From 70b5d1cbe80bebf851615938ac9af69c9dd0cb6b Mon Sep 17 00:00:00 2001 From: Ed Savage Date: Tue, 20 Oct 2020 14:35:41 +0100 Subject: [PATCH] [7.x][ML] Pass job config in JSON file to autodetect (#63865) (#63873) Write job config in the form of a JSON formatted file and pass its location to autodetect as an additional command line option. This will ultimately allow all relevant job config to be read from the JSON config file and hence reduce the number of command line arguments for autodetect. Relates to elastic/ml-cpp#1253 Backport of #63865 --- .../process/autodetect/AutodetectBuilder.java | 29 ++++++++++++++++++- .../autodetect/AutodetectBuilderTests.java | 26 ++++++++++++++++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectBuilder.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectBuilder.java index 144347c40a5b9..a8985c7c99617 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectBuilder.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectBuilder.java @@ -9,6 +9,9 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.env.Environment; import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.ml.calendars.ScheduledEvent; @@ -63,7 +66,9 @@ public class AutodetectBuilder { public static final String QUANTILES_STATE_PATH_ARG = "--quantilesState="; private static final String CONF_EXTENSION = ".conf"; + private static final String JSON_EXTENSION = ".json"; static final String JOB_ID_ARG = "--jobid="; + private static final String CONFIG_ARG = "--config="; private static final String LIMIT_CONFIG_ARG = "--limitconfig="; private static final String MODEL_PLOT_CONFIG_ARG = "--modelplotconfig="; private static final String FIELD_CONFIG_ARG = "--fieldconfig="; @@ -173,11 +178,20 @@ public void build() throws IOException, InterruptedException { List command = buildAutodetectCommand(); + // While it may appear that the JSON formatted job config file contains data that + // is duplicated in the existing limits, modelPlot and field config files, over time + // the C++ backend will retrieve all its required configuration data from the new + // JSON config file and the old-style configuration files will be removed. + buildJobConfig(command); + + // Per the comment above, these three lines will eventually be removed once migration + // to the new JSON formatted configuration file has been completed. buildLimits(command); buildModelPlotConfig(command); + buildFieldConfig(command); buildQuantiles(command); - buildFieldConfig(command); + processPipes.addArgs(command); controller.startProcess(command); } @@ -350,4 +364,17 @@ private void buildFieldConfig(List command) throws IOException { command.add(fieldConfig); } } + + private void buildJobConfig(List command) throws IOException { + Path configFile = Files.createTempFile(env.tmpFile(), "config", JSON_EXTENSION); + filesToDelete.add(configFile); + try (OutputStreamWriter osw = new OutputStreamWriter(Files.newOutputStream(configFile),StandardCharsets.UTF_8); + XContentBuilder jsonBuilder = JsonXContent.contentBuilder()) { + + job.toXContent(jsonBuilder, ToXContent.EMPTY_PARAMS); + osw.write(Strings.toString(jsonBuilder)); + } + + command.add(CONFIG_ARG + configFile.toString()); + } } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectBuilderTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectBuilderTests.java index debbdb222f48b..87cd751e548a4 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectBuilderTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectBuilderTests.java @@ -19,14 +19,21 @@ import org.elasticsearch.xpack.ml.process.NativeController; import org.elasticsearch.xpack.ml.process.ProcessPipes; import org.junit.Before; +import org.mockito.ArgumentCaptor; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import static org.elasticsearch.xpack.core.ml.job.config.JobTests.buildJobBuilder; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.matchesPattern; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; public class AutodetectBuilderTests extends ESTestCase { @@ -36,11 +43,14 @@ public class AutodetectBuilderTests extends ESTestCase { private Settings settings; private NativeController nativeController; private ProcessPipes processPipes; + private ArgumentCaptor> commandCaptor; + @SuppressWarnings("unchecked") @Before public void setUpTests() { logger = mock(Logger.class); - filesToDelete = Collections.emptyList(); + filesToDelete = new ArrayList<>(); + commandCaptor = ArgumentCaptor.forClass((Class)List.class); settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()).build(); env = TestEnvironment.newEnvironment(settings); nativeController = mock(NativeController.class); @@ -126,4 +136,18 @@ public void testBuildAutodetectCommand_givenPersistModelState() { private AutodetectBuilder autodetectBuilder(Job job) { return new AutodetectBuilder(job, filesToDelete, logger, env, settings, nativeController, processPipes); } + + public void testBuildAutodetect() throws Exception { + Job.Builder job = buildJobBuilder("unit-test-job"); + + autodetectBuilder(job.build()).build(); + + assertThat(filesToDelete, hasSize(3)); + + verify(nativeController).startProcess(commandCaptor.capture()); + verifyNoMoreInteractions(nativeController); + + List command = commandCaptor.getValue(); + assertThat(command, hasItem(matchesPattern("--config=.*\\.json"))); + } }