Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HLRC: Adding ML Job stats #33183

Merged
merged 19 commits into from
Sep 1, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ public void getBucketsAsync(GetBucketsRequest request, RequestOptions options, A
}

/**
* Gets one or more Machine Learning job statistics.
* Gets usage statistics for one or more Machine Learning jobs
*
* <p>
* For additional info
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.client.ml.job.config.Job;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
Expand All @@ -35,25 +36,23 @@


/**
* Request object to get {@link org.elasticsearch.client.ml.job.stats.JobStats} objects with the matching `jobId`s
* Request object to get {@link org.elasticsearch.client.ml.job.stats.JobStats} by their respective jobIds
*
* `_all` explicitly gets all the jobs' statistics in the cluster
* An empty request (no `jobId`s) implicitly gets all the jobs' statistics in the cluster
*/
public class GetJobsStatsRequest extends ActionRequest implements ToXContentObject {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name of this and the GetJobRequest needs to be consistent. Not sure if plural or singular is best. Potentially should also be consistent with the results (where I've been using plural). @droberts195 Any preference?


public static final ParseField JOB_ID = new ParseField("job_id");
public static final ParseField ALLOW_NO_JOBS = new ParseField("allow_no_jobs");

@SuppressWarnings("unchecked")
public static final ConstructingObjectParser<GetJobsStatsRequest, Void> PARSER = new ConstructingObjectParser<>(
"get_jobs_stats_request",
true, a -> new GetJobsStatsRequest((List<String>) a[0]));
"get_jobs_stats_request", a -> new GetJobsStatsRequest((List<String>) a[0]));

static {
PARSER.declareField(ConstructingObjectParser.constructorArg(),
p -> Arrays.asList(Strings.commaDelimitedListToStringArray(p.text())),
JOB_ID, ObjectParser.ValueType.STRING_ARRAY);
Job.ID, ObjectParser.ValueType.STRING_ARRAY);
PARSER.declareBoolean(GetJobsStatsRequest::setAllowNoJobs, ALLOW_NO_JOBS);
}

Expand All @@ -67,7 +66,7 @@ public class GetJobsStatsRequest extends ActionRequest implements ToXContentObje
*
* @return a {@link GetJobsStatsRequest} for all existing jobs
*/
public static GetJobsStatsRequest allJobsStats(){
public static GetJobsStatsRequest getAllJobsStatsRequest(){
return new GetJobsStatsRequest(ALL_JOBS);
}

Expand Down Expand Up @@ -137,7 +136,7 @@ public ActionRequestValidationException validate() {
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field(JOB_ID.getPreferredName(), Strings.collectionToCommaDelimitedString(jobIds));
builder.field(Job.ID.getPreferredName(), Strings.collectionToCommaDelimitedString(jobIds));
if (allowNoJobs != null) {
builder.field(ALLOW_NO_JOBS.getPreferredName(), allowNoJobs);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public class GetJobsStatsResponse extends AbstractResultResponse<JobStats> {
/**
* The collection of {@link JobStats} objects found in the query
*/
public List<JobStats> jobs() {
public List<JobStats> jobStats() {
return results;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ public class ForecastStats implements ToXContentObject {

public static final ParseField TOTAL = new ParseField("total");
public static final ParseField FORECASTED_JOBS = new ParseField("forecasted_jobs");
public static final ParseField MEMORY = new ParseField("memory_bytes");
public static final ParseField RUNTIME = new ParseField("processing_time_ms");
public static final ParseField MEMORY_BYTES = new ParseField("memory_bytes");
public static final ParseField PROCESSING_TIME_MS = new ParseField("processing_time_ms");
public static final ParseField RECORDS = new ParseField("records");
public static final ParseField STATUSES = new ParseField("status");
public static final ParseField STATUS = new ParseField("status");

@SuppressWarnings("unchecked")
public static final ConstructingObjectParser<ForecastStats, Void> PARSER =
Expand All @@ -58,19 +58,19 @@ public class ForecastStats implements ToXContentObject {

static {
PARSER.declareLong(ConstructingObjectParser.constructorArg(), TOTAL);
PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), SimpleStats.PARSER, MEMORY);
PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), SimpleStats.PARSER, MEMORY_BYTES);
PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), SimpleStats.PARSER, RECORDS);
PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), SimpleStats.PARSER, RUNTIME);
PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), SimpleStats.PARSER, PROCESSING_TIME_MS);
PARSER.declareField(ConstructingObjectParser.optionalConstructorArg(),
p -> {
Map<String, Long> counts = new HashMap<>();
p.map().forEach((key, value) -> counts.put(key, ((Number)value).longValue()));
return counts;
}, STATUSES, ObjectParser.ValueType.OBJECT);
}, STATUS, ObjectParser.ValueType.OBJECT);
}

private long total;
private long forecastedJobs;
private final long total;
private final long forecastedJobs;
private SimpleStats memoryStats;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest could be final as well, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dimitris-athanasiou No, because they are optionally set in the constructor if the total is > 0

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just means we need to explicitly set them to null if the condition is not true. Just informational, it's no big deal whether we do it or not.

private SimpleStats recordStats;
private SimpleStats runtimeStats;
Expand Down Expand Up @@ -140,10 +140,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.field(FORECASTED_JOBS.getPreferredName(), forecastedJobs);

if (total > 0) {
builder.field(MEMORY.getPreferredName(), memoryStats);
builder.field(MEMORY_BYTES.getPreferredName(), memoryStats);
builder.field(RECORDS.getPreferredName(), recordStats);
builder.field(RUNTIME.getPreferredName(), runtimeStats);
builder.field(STATUSES.getPreferredName(), statusCounts);
builder.field(PROCESSING_TIME_MS.getPreferredName(), runtimeStats);
builder.field(STATUS.getPreferredName(), statusCounts);
}
return builder.endObject();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,13 @@ public class JobStats implements ToXContentObject {


private final String jobId;
private DataCounts dataCounts;
private JobState state;
private ModelSizeStats modelSizeStats;
private ForecastStats forecastStats;
private NodeAttributes node;
private String assignmentExplanation;
private TimeValue openTime;
private final DataCounts dataCounts;
private final JobState state;
private final ModelSizeStats modelSizeStats;
private final ForecastStats forecastStats;
private final NodeAttributes node;
private final String assignmentExplanation;
private final TimeValue openTime;

JobStats(String jobId, DataCounts dataCounts, JobState state, @Nullable ModelSizeStats modelSizeStats,
@Nullable ForecastStats forecastStats, @Nullable NodeAttributes node,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ public class SimpleStats implements ToXContentObject {
PARSER.declareDouble(ConstructingObjectParser.constructorArg(), AVG);
}

private double total;
private double min;
private double max;
private double avg;
private final double total;
private final double min;
private final double max;
private final double avg;

SimpleStats(double total, double min, double max, double avg) {
this.total = total;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,9 @@ public void testGetJobsStats() throws Exception {
GetJobsStatsResponse response = execute(request, machineLearningClient::getJobStats, machineLearningClient::getJobStatsAsync);

assertEquals(2, response.count());
assertThat(response.jobs(), hasSize(2));
assertThat(response.jobs().stream().map(JobStats::getJobId).collect(Collectors.toList()), containsInAnyOrder(jobId1, jobId2));
for (JobStats stats : response.jobs()) {
assertThat(response.jobStats(), hasSize(2));
assertThat(response.jobStats().stream().map(JobStats::getJobId).collect(Collectors.toList()), containsInAnyOrder(jobId1, jobId2));
for (JobStats stats : response.jobStats()) {
if (stats.getJobId().equals(jobId1)) {
assertEquals(JobState.OPENED, stats.getState());
} else {
Expand All @@ -174,26 +174,26 @@ public void testGetJobsStats() throws Exception {
}

// Test getting all explicitly
request = GetJobsStatsRequest.allJobsStats();
request = GetJobsStatsRequest.getAllJobsStatsRequest();
response = execute(request, machineLearningClient::getJobStats, machineLearningClient::getJobStatsAsync);

assertTrue(response.count() >= 2L);
assertTrue(response.jobs().size() >= 2L);
assertThat(response.jobs().stream().map(JobStats::getJobId).collect(Collectors.toList()), hasItems(jobId1, jobId2));
assertTrue(response.jobStats().size() >= 2L);
assertThat(response.jobStats().stream().map(JobStats::getJobId).collect(Collectors.toList()), hasItems(jobId1, jobId2));

// Test getting all implicitly
response = execute(new GetJobsStatsRequest(), machineLearningClient::getJobStats, machineLearningClient::getJobStatsAsync);

assertTrue(response.count() >= 2L);
assertTrue(response.jobs().size() >= 2L);
assertThat(response.jobs().stream().map(JobStats::getJobId).collect(Collectors.toList()), hasItems(jobId1, jobId2));
assertTrue(response.jobStats().size() >= 2L);
assertThat(response.jobStats().stream().map(JobStats::getJobId).collect(Collectors.toList()), hasItems(jobId1, jobId2));

// Test getting all with wildcard
request = new GetJobsStatsRequest("ml-get-job-stats-test-id-*");
response = execute(request, machineLearningClient::getJobStats, machineLearningClient::getJobStatsAsync);
assertTrue(response.count() >= 2L);
assertTrue(response.jobs().size() >= 2L);
assertThat(response.jobs().stream().map(JobStats::getJobId).collect(Collectors.toList()), hasItems(jobId1, jobId2));
assertTrue(response.jobStats().size() >= 2L);
assertThat(response.jobStats().stream().map(JobStats::getJobId).collect(Collectors.toList()), hasItems(jobId1, jobId2));

// Test when allow_no_jobs is false
final GetJobsStatsRequest erroredRequest = new GetJobsStatsRequest("jobs-that-do-not-exist*");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -479,12 +479,12 @@ public void testGetJobStats() throws Exception {
//tag::x-pack-ml-get-job-stats-execute
GetJobsStatsResponse response = client.machineLearning().getJobStats(request, RequestOptions.DEFAULT);
long numberOfJobsStats = response.count(); //<1>
List<JobStats> jobsStats = response.jobs(); //<2>
List<JobStats> jobsStats = response.jobStats(); //<2>
//end::x-pack-ml-get-job-stats-execute

assertEquals(2, response.count());
assertThat(response.jobs(), hasSize(2));
assertThat(response.jobs().stream().map(JobStats::getJobId).collect(Collectors.toList()),
assertThat(response.jobStats(), hasSize(2));
assertThat(response.jobStats().stream().map(JobStats::getJobId).collect(Collectors.toList()),
containsInAnyOrder(job.getId(), secondJob.getId()));
}
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
public class GetJobsStatsRequestTests extends AbstractXContentTestCase<GetJobsStatsRequest> {

public void testAllJobsRequest() {
GetJobsStatsRequest request = GetJobsStatsRequest.allJobsStats();
GetJobsStatsRequest request = GetJobsStatsRequest.getAllJobsStatsRequest();

assertEquals(request.getJobIds().size(), 1);
assertEquals(request.getJobIds().get(0), "_all");
Expand Down Expand Up @@ -64,6 +64,6 @@ protected GetJobsStatsRequest doParseInstance(XContentParser parser) throws IOEx

@Override
protected boolean supportsUnknownFields() {
return true;
return false;
}
}
2 changes: 1 addition & 1 deletion docs/java-rest/high-level/ml/get-job-stats.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ in the `RestHighLevelClient` object, accessed via the `machineLearningClient()`
include-tagged::{doc-tests}/MlClientDocumentationIT.java[x-pack-ml-get-job-stats-execute]
--------------------------------------------------
<1> `getCount()` from the `GetJobsStatsResponse` indicates the number of jobs statistics found
<2> `getJobs()` is the collection of {ml} `JobStats` objects found
<2> `getJobStats()` is the collection of {ml} `JobStats` objects found

[[java-rest-high-x-pack-ml-get-job-stats-execution-async]]
==== Asynchronous Execution
Expand Down