[ML] Add audit warning for 1000 categories found early in job #51146

droberts195 · 2020-01-17T11:05:28Z

If 1000 different category definitions are created for a job in
the first 100 buckets it processes then an audit warning will now
be created. (This will cause a yellow warning triangle in the
ML UI's jobs list.)

Such a large number of categories suggests that the field that
categorization is working on is not well suited to the ML
categorization functionality.

Relates #50749

If 1000 different category definitions are created for a job in the first 100 buckets it processes then an audit warning will now be created. (This will cause a yellow warning triangle in the ML UI's jobs list.) Such a large number of categories suggests that the field that categorization is working on is not well suited to the ML categorization functionality.

elasticmachine · 2020-01-17T11:05:30Z

Pinging @elastic/ml-core (:ml)

dimitris-athanasiou

Looks good. Left a few minor suggestions.

dimitris-athanasiou · 2020-01-17T11:54:25Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/messages/Messages.java

@@ -135,6 +135,8 @@
            "Adjust the analysis_limits.model_memory_limit setting to ensure all data is analyzed";
    public static final String JOB_AUDIT_MEMORY_STATUS_HARD_LIMIT_PRE_7_2 = "Job memory status changed to hard_limit at {0}; adjust the " +
        "analysis_limits.model_memory_limit setting to ensure all data is analyzed";
+    public static final String JOB_AUDIT_EXCESSIVE_EARLY_CATEGORIES = "{0} categories observed in the first [{1}] buckets." +


We have square brackets about the second number in this message but not the first. Should we make it consistent?

The reason I didn't put the first one in square brackets is that it won't vary (at least for a particular version of the product). The first number will always be 1000 unless somebody changes the code, whereas the second number can vary between different occurrences of the audit message.

dimitris-athanasiou · 2020-01-17T12:26:58Z

...java/org/elasticsearch/xpack/ml/job/process/autodetect/output/AutodetectResultProcessor.java

@@ -87,7 +90,9 @@
    private final FlushListener flushListener;
    private volatile boolean processKilled;
    private volatile boolean failed;
-    private int bucketCount; // only used from the process() thread, so doesn't need to be volatile
+    private long priorRunsBucketCount;
+    private int currentRunBucketCount; // only used from the process() thread, so doesn't need to be volatile


should we take this chance and make this a long as well? It seems like something that could hit overflow problems.

With a 1 second bucket span it will take 68 years to overflow, so pretty unlikely. But maybe I can avoid some casting by making it long, which will make the code cleaner. If so I'll change it.

There wasn't much casting but I changed it anyway just so both variables have the same type.

dimitris-athanasiou · 2020-01-17T12:27:36Z

...java/org/elasticsearch/xpack/ml/job/process/autodetect/output/AutodetectResultProcessor.java

@@ -122,6 +127,7 @@ public AutodetectResultProcessor(Client client,
        this.bulkResultsPersister = persister.bulkPersisterBuilder(jobId, this::isAlive);
        this.timingStatsReporter = new TimingStatsReporter(timingStats, bulkResultsPersister);
        this.deleteInterimRequired = true;
+        this.priorRunsBucketCount = timingStats.getBucketCount();


I was wondering where we'd get this from but it's cool we already pass it in!

dimitris-athanasiou · 2020-01-17T12:29:29Z

...java/org/elasticsearch/xpack/ml/job/process/autodetect/output/AutodetectResultProcessor.java

@@ -225,6 +231,18 @@ void processResult(AutodetectResult result) {
        CategoryDefinition categoryDefinition = result.getCategoryDefinition();
        if (categoryDefinition != null) {
            persister.persistCategoryDefinition(categoryDefinition, this::isAlive);


Now that all this has become more complex than a single call to the persister, I'd be tempted to extract this in a handleCategoryDefinition method.

dimitris-athanasiou

LGTM

If 1000 different category definitions are created for a job in the first 100 buckets it processes then an audit warning will now be created. (This will cause a yellow warning triangle in the ML UI's jobs list.) Such a large number of categories suggests that the field that categorization is working on is not well suited to the ML categorization functionality.

…c#51146) If 1000 different category definitions are created for a job in the first 100 buckets it processes then an audit warning will now be created. (This will cause a yellow warning triangle in the ML UI's jobs list.) Such a large number of categories suggests that the field that categorization is working on is not well suited to the ML categorization functionality.

In elastic#51146 a rudimentary check for poor categorization was added to 7.6. This change replaces that warning based on a Java-side check with a new one based on the categorization_status field that the ML C++ sets. categorization_status was added in 7.7 and above by elastic#51879, so this new warning based on more advanced conditions will also be in 7.7 and above. Closes elastic#50749

…2195) In #51146 a rudimentary check for poor categorization was added to 7.6. This change replaces that warning based on a Java-side check with a new one based on the categorization_status field that the ML C++ sets. categorization_status was added in 7.7 and above by #51879, so this new warning based on more advanced conditions will also be in 7.7 and above. Closes #50749

droberts195 added >enhancement :ml Machine learning v8.0.0 v7.6.0 v7.7.0 labels Jan 17, 2020

dimitris-athanasiou self-requested a review January 17, 2020 11:53

dimitris-athanasiou reviewed Jan 17, 2020

View reviewed changes

droberts195 added 2 commits January 17, 2020 14:44

Merge branch 'master' into excessive_categories_audit_message

30afa3f

Address review comments

69172d3

dimitris-athanasiou approved these changes Jan 17, 2020

View reviewed changes

droberts195 merged commit 160a212 into elastic:master Jan 17, 2020

droberts195 deleted the excessive_categories_audit_message branch January 17, 2020 16:25

droberts195 mentioned this pull request Jan 20, 2020

[ML] Warn if ML categorization job is using data that does not categorize well #50749

Closed

This was referenced Feb 3, 2020

[meta] 7.6 release elastic/elasticsearch-net#4340

Closed

[meta] 7.6 release elastic/elasticsearch-net#4341

Closed

This was referenced Feb 11, 2020

[ML] Switch poor categorization audit warning to use status field #52195

Merged

[ML] Add audit message when categorization detects too many categories #50319

Closed

codebrain mentioned this pull request Apr 1, 2020

7.7.0 meta ticket (Part 2) elastic/elasticsearch-net#4533

Closed

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Add audit warning for 1000 categories found early in job #51146

[ML] Add audit warning for 1000 categories found early in job #51146

droberts195 commented Jan 17, 2020 •

edited

Loading

elasticmachine commented Jan 17, 2020

dimitris-athanasiou left a comment

dimitris-athanasiou Jan 17, 2020

droberts195 Jan 17, 2020

dimitris-athanasiou Jan 17, 2020

droberts195 Jan 17, 2020

droberts195 Jan 17, 2020

dimitris-athanasiou Jan 17, 2020

dimitris-athanasiou Jan 17, 2020

dimitris-athanasiou left a comment

[ML] Add audit warning for 1000 categories found early in job #51146

[ML] Add audit warning for 1000 categories found early in job #51146

Conversation

droberts195 commented Jan 17, 2020 • edited Loading

elasticmachine commented Jan 17, 2020

dimitris-athanasiou left a comment

Choose a reason for hiding this comment

dimitris-athanasiou Jan 17, 2020

Choose a reason for hiding this comment

droberts195 Jan 17, 2020

Choose a reason for hiding this comment

dimitris-athanasiou Jan 17, 2020

Choose a reason for hiding this comment

droberts195 Jan 17, 2020

Choose a reason for hiding this comment

droberts195 Jan 17, 2020

Choose a reason for hiding this comment

dimitris-athanasiou Jan 17, 2020

Choose a reason for hiding this comment

dimitris-athanasiou Jan 17, 2020

Choose a reason for hiding this comment

dimitris-athanasiou left a comment

Choose a reason for hiding this comment

droberts195 commented Jan 17, 2020 •

edited

Loading