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

[ML][Transform] fixing testFailureCounterIsResetOnSuccess test failure #76397 #76417

Merged

Conversation

benwtrent
Copy link
Member

@benwtrent benwtrent commented Aug 12, 2021

There are two possible race conditions that were not
previously handled in this test.

  • Since the syncconfig was null, it may be that the
    transform actually gets set to stopping/stopped
    and its unable to kick off another indexing pass
  • It may also be that the indexer thread is still
    finishing up work when the second execution is
    requested, so it returns false.

Adding a sync config and assertBusy handles these
cases. Ran 1k+ times locally with this change
and it never failed. Without, it failed ~10 runs.

closes #76397

…elastic#76397

There are two possible race conditions that were not
previously handled in this test.

- Since the syncconfig was null, it may be that the
  transform actually gets set to stopping/stopped
  and its unable to kick off another indexing pass
- It may also be that the indexer thread is still
  finishing up work when the second execution is
  requested, so it returns false.

Adding a sync config and assertBusy handles these
cases. Ran 1k+ times locally with this change
and it never failed. Without, it failed ~10 runs.

closes elastic#76397
@benwtrent benwtrent added >test Issues or PRs that are addressing/adding tests v8.0.0 :ml/Transform Transform auto-backport Automatically create backport pull requests when merged v7.14.1 v7.15.0 labels Aug 12, 2021
@elasticmachine elasticmachine added the Team:ML Meta label for the ML team label Aug 12, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

Copy link
Contributor

@dimitris-athanasiou dimitris-athanasiou left a comment

Choose a reason for hiding this comment

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

LGTM

@benwtrent benwtrent added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Aug 12, 2021
@elasticsearchmachine elasticsearchmachine merged commit aaeecd7 into elastic:master Aug 12, 2021
benwtrent added a commit to benwtrent/elasticsearch that referenced this pull request Aug 12, 2021
…elastic#76397 (elastic#76417)

There are two possible race conditions that were not
previously handled in this test.

- Since the syncconfig was null, it may be that the
  transform actually gets set to stopping/stopped
  and its unable to kick off another indexing pass
- It may also be that the indexer thread is still
  finishing up work when the second execution is
  requested, so it returns false.

Adding a sync config and assertBusy handles these
cases. Ran 1k+ times locally with this change
and it never failed. Without, it failed ~10 runs.

closes elastic#76397
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
7.14
7.x

benwtrent added a commit to benwtrent/elasticsearch that referenced this pull request Aug 12, 2021
…elastic#76397 (elastic#76417)

There are two possible race conditions that were not
previously handled in this test.

- Since the syncconfig was null, it may be that the
  transform actually gets set to stopping/stopped
  and its unable to kick off another indexing pass
- It may also be that the indexer thread is still
  finishing up work when the second execution is
  requested, so it returns false.

Adding a sync config and assertBusy handles these
cases. Ran 1k+ times locally with this change
and it never failed. Without, it failed ~10 runs.

closes elastic#76397
elasticsearchmachine pushed a commit that referenced this pull request Aug 12, 2021
…#76397 (#76417) (#76421)

There are two possible race conditions that were not
previously handled in this test.

- Since the syncconfig was null, it may be that the
  transform actually gets set to stopping/stopped
  and its unable to kick off another indexing pass
- It may also be that the indexer thread is still
  finishing up work when the second execution is
  requested, so it returns false.

Adding a sync config and assertBusy handles these
cases. Ran 1k+ times locally with this change
and it never failed. Without, it failed ~10 runs.

closes #76397
elasticsearchmachine pushed a commit that referenced this pull request Aug 12, 2021
…#76397 (#76417) (#76422)

There are two possible race conditions that were not
previously handled in this test.

- Since the syncconfig was null, it may be that the
  transform actually gets set to stopping/stopped
  and its unable to kick off another indexing pass
- It may also be that the indexer thread is still
  finishing up work when the second execution is
  requested, so it returns false.

Adding a sync config and assertBusy handles these
cases. Ran 1k+ times locally with this change
and it never failed. Without, it failed ~10 runs.

closes #76397
@benwtrent benwtrent deleted the test/ml-transform-issue-76397 branch September 15, 2021 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :ml/Transform Transform Team:ML Meta label for the ML team >test Issues or PRs that are addressing/adding tests v7.14.1 v7.15.0 v8.0.0-alpha2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] TransformIndexerFailureHandlingTests testFailureCounterIsResetOnSuccess failing
5 participants