Skip to content

Conversation

@benwtrent
Copy link
Member

@benwtrent benwtrent commented Sep 28, 2020

This fixes the two test failures.

The shard failure seems to be due to the .ml-stats index being in the middle of being created.

See:

private void getProgress(DataFrameAnalyticsConfig config, ActionListener<List<PhaseProgress>> listener) {
GetDataFrameAnalyticsStatsAction.Request getStatsRequest = new GetDataFrameAnalyticsStatsAction.Request(config.getId());
executeAsyncWithOrigin(client, ML_ORIGIN, GetDataFrameAnalyticsStatsAction.INSTANCE, getStatsRequest, ActionListener.wrap(
statsResponse -> {
List<GetDataFrameAnalyticsStatsAction.Response.Stats> stats = statsResponse.getResponse().results();
if (stats.isEmpty()) {
// The job has been deleted in between
listener.onFailure(ExceptionsHelper.missingDataFrameAnalytics(config.getId()));
} else {
listener.onResponse(stats.get(0).getProgress());
}
},
listener::onFailure
));
}

We fail grabbing the progress on starting the second job.

This commit splits the creating/starting of the two jobs.

We still want to fail on shard failures like this as knowing the previous progress (if it exists) is vitally important.

closes #62064
closes #55807

@benwtrent benwtrent added >test Issues or PRs that are addressing/adding tests v8.0.0 v7.10.0 labels Sep 28, 2020
@davidkyle davidkyle added the :ml Machine learning label Sep 29, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (:ml)

Copy link

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

This is OK as a fix for the test, but I don't like that we're having to change tests that are doing things that users could be doing (running two jobs together on a clean system).

It all goes back to the problem that searches treat indices that don't exist differently to indices that have just been created and are not yet ready to use (but which are obviously empty as they've only just been created). I will bump the thread about doing something about that.

@benwtrent
Copy link
Member Author

It all goes back to the problem that searches treat indices that don't exist differently to indices that have just been created and are not yet ready to use

100% agree. I wish this "fix" was not necessary.

@benwtrent benwtrent merged commit 471403e into elastic:master Sep 29, 2020
benwtrent added a commit to benwtrent/elasticsearch that referenced this pull request Sep 29, 2020
…lastic#62976)

This fixes the two test failures.

The shard failure seems to be due to the .ml-stats index being in the middle of being created.
benwtrent added a commit that referenced this pull request Sep 29, 2020
…tests (#62976) (#62999)

* [ML] fixing testTwoJobsWithSameRandomizeSeedUseSameTrainingSet tests (#62976)

This fixes the two test failures.

The shard failure seems to be due to the .ml-stats index being in the middle of being created.
@benwtrent benwtrent deleted the test/ml-dfa-random-seed-test-fix branch October 19, 2020 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:ml Machine learning >test Issues or PRs that are addressing/adding tests v7.10.0 v8.0.0-alpha1

Projects

None yet

5 participants