[ML] Pass job config in JSON file to autodetect (#63865)

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
elastic · Oct 19, 2020 · 734c959 · 734c959
1 parent 245663e
commit 734c959
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 2 deletions.
diff --git a/...ml/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectBuilder.java b/...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<String> 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<String> command) throws IOException {
             command.add(fieldConfig);
         }
     }
+
+    private void buildJobConfig(List<String> 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/...c/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectBuilderTests.java b/...c/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<List<String>> 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<String> command = commandCaptor.getValue();
+        assertThat(command, hasItem(matchesPattern("--config=.*\\.json")));
+    }
 }