[ML] Make GetJobStats work with arbitrary wildcards and groups

[henriquegoncalves98]

Fixing match function to use expandedJobId instead of a single jobid.

Closes #34745

[elasticmachine]

Pinging @elastic/ml-core

[henriquegoncalves98]

In my commit instead of 'expandedJobsIds[i]' it's expandedJobsIds.get(i). All tests pass with the code submited. P.s:Already made a commit editing the mistake mention above.

[benwtrent]

I am not sure what which tests @davidkyle was mentioning in the issue.

I know we have tests here: https://github.com/elastic/elasticsearch/blob/master/client/rest-high-level/src/test/java/org/elasticsearch/client/MachineLearningIT.java but those are more for ensuring the client acts correctly, not that the job stats are handled appropriately.

[benwtrent]

This can be reduced to a single line using stream().anyMatch

Suggested change

	for(int i = 0; i < expandedJobsIds.size(); i++)
	return expandedJobsIds.stream().anyMatch(jobId -> OpenJobAction.JobTaskMatcher.match(task, jobId));

[henriquegoncalves98]

Will commit the changes thx for the review!

[benwtrent]

Jenkins test this please

[benwtrent]

@up201608320 Thanks for taking a look at this issue. Looking at elasticsearch-ci-2 it seems that there is a yml test that is failing. I found the test failure by selecting the details link, looking at the full console output, and then searching the page for REPRODUCE (case sensitive). That turned up a yml test that is failing.

You can rerun these with:

./gradlew :x-pack:plugin:integTestRunner -Dtests.seed=E912E5024622B77D -Dtests.class=org.elasticsearch.xpack.test.rest.XPackRestIT -Dtests.method="test {p0=ml/job_groups/Test close job API}"

Relevant test is here:

elasticsearch/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/job_groups.yml

Line 242 in 4103d3b

"Test close job API":

I ran the test locally against master and it passed. So, either your change is causing the failure, or some past commit (that has since been corrected) caused the failure and you need to merge in the latest master into your branch.

Documentation on how the test formatting is here: https://github.com/elastic/elasticsearch/blob/master/rest-api-spec/src/main/resources/rest-api-spec/test/README.asciidoc

From what I can see, it is a simple re-ordering issue.

[davidkyle]

The failing test in job_groups.yml is because the response of GET job stats is not ordered by job id. Compare the 6.x branch https://github.com/elastic/elasticsearch/blob/6.x/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetJobsStatsAction.java#L163

with master https://github.com/elastic/elasticsearch/blob/master/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetJobsStatsAction.java#L151 where the call to Collections.sort is missing.

Actually this call is also missing from the master branch https://github.com/elastic/elasticsearch/blob/6.x/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetJobsStatsAction.java#L100

I assume this change has somehow affected the ordering causing the tests to fail which is good as it has revealed an issue.

As for testing this change a unit test added to GetJobStatsActionRequestTests would be sufficient. There is already one test on the match function testMatch_GivenAll_FailsForNonJobTasks in that class. Note the function expects a Task parameter which implementsOpenJobAction.JobTaskMatcher, use TransportOpenJobAction.JobTask for this

[davidkyle]

@up201608320 I raised #36841 to fix the ordering of the GET job stats response. Once that is merged please rebase your change on the master branch and the test failure will go away

[droberts195]

@up201608320 #36841 is now merged. If you pull the latest master branch, then merge it into your PR branch then hopefully this PR will pass CI and can be merged.

[droberts195]

Jenkins test this please

[droberts195]

Jenkins run elasticsearch-ci/default-distro

[droberts195]

Jenkins run elasticsearch-ci/default-distro

[droberts195]

Jenkins test this please

[droberts195]

Jenkins test this please

[davidkyle]

LGTM