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

Use removeTask instead of finishTask in PersistentTasksClusterService #29055

Merged
merged 1 commit into from
Mar 16, 2018

Conversation

tlrx
Copy link
Member

@tlrx tlrx commented Mar 14, 2018

The method PersistentTasksClusterService.finishTask() has been
modified since it was added and does not use any removeOncompletion
flag anymore. Its behavior is now similar to removeTask() and can be
replaced by this one. When a non existing task is removed, the cluster
state update task will fail and its source will still indicate
finish persistent task/remove persistent task.

This commit also fixes few typos.

The method `PersistentTasksClusterService.finishTask()` has been
modified since it was added and does not use any `removeOncompletion`
flag anymore. Its behavior is now similar to `removeTask()` and can be
replaced by this one. When a non existing task is removed, the cluster
state update task will fail and its `source` will still indicate
`finish persistent task`/`remove persistent task`.

This commit also fixes few typos.
@tlrx tlrx added review :Distributed Coordination/Task Management Issues for anything around the Tasks API - both persistent and node level. v7.0.0 >refactoring labels Mar 14, 2018
@tlrx tlrx requested a review from imotov March 14, 2018 10:32
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

Copy link
Contributor

@bleskes bleskes left a comment

Choose a reason for hiding this comment

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

I think this can go to 6.x too no?

Copy link
Contributor

@imotov imotov left a comment

Choose a reason for hiding this comment

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

LGTM too. Nice cleanup!

@tlrx tlrx merged commit f141469 into elastic:master Mar 16, 2018
@tlrx
Copy link
Member Author

tlrx commented Mar 16, 2018

Thanks @bleskes and @imotov

I think this can go to 6.x too no?

Yes

@tlrx tlrx deleted the persistenttasks-cleanups branch March 16, 2018 09:26
tlrx added a commit that referenced this pull request Mar 16, 2018
…#29055)

The method `PersistentTasksClusterService.finishTask()` has been
modified since it was added and does not use any `removeOncompletion`
flag anymore. Its behavior is now similar to `removeTask()` and can be
replaced by this one. When a non existing task is removed, the cluster
state update task will fail and its `source` will still indicate
`finish persistent task`/`remove persistent task`.
@tlrx tlrx added the v6.3.0 label Mar 16, 2018
martijnvg added a commit that referenced this pull request Mar 16, 2018
* es/6.x: (89 commits)
  Clarify requirements of strict date formats. (#29090)
  Clarify that dates are always rendered as strings. (#29093)
  [Docs] Fix link to Grok patterns (#29088)
  Fix starting on Windows from another drive (#29086)
  Use removeTask instead of finishTask in PersistentTasksClusterService (#29055)
  Added minimal docs for reindex api in java-api docs
  Allow overriding JVM options in Windows service (#29044)
  Clarify how to set compiler and runtime JDKs (#29101)
  Fix Parsing Bug with Update By Query for Stored Scripts (#29039)
  TEST: write ops should execute under shard permit (#28966)
  [DOCS] Add X-Pack upgrade details (#29038)
  Revert "Improve error message for installing plugin (#28298)"
  Docs: HighLevelRestClient#exists (#29073)
  Validate regular expressions in dynamic templates. (#29013)
  [Tests] Fix GetResultTests and DocumentFieldTests failures (#29083)
  Reenable LiveVersionMapTests.testRamBytesUsed on Java 9. (#29063)
  Mute failing GetResultTests and DocumentFieldTests
  [Docs] Fix Java Api index administration usage (#28260)
  Improve error message for installing plugin (#28298)
  Decouple XContentBuilder from BytesReference (#28972)
  ...
martijnvg added a commit that referenced this pull request Mar 16, 2018
* es/master: (97 commits)
  Clarify requirements of strict date formats. (#29090)
  Clarify that dates are always rendered as strings. (#29093)
  Compilation fix for #29067
  [Docs] Fix link to Grok patterns (#29088)
  Store offsets in index prefix fields when stored in the parent field (#29067)
  Fix starting on Windows from another drive (#29086)
  Use removeTask instead of finishTask in PersistentTasksClusterService (#29055)
  Added minimal docs for reindex api in java-api docs
  Allow overriding JVM options in Windows service (#29044)
  Clarify how to set compiler and runtime JDKs (#29101)
  CLI: Close subcommands in MultiCommand (#28954)
  TEST: write ops should execute under shard permit (#28966)
  [DOCS] Add X-Pack upgrade details (#29038)
  Revert "Improve error message for installing plugin (#28298)"
  Docs: HighLevelRestClient#exists (#29073)
  Validate regular expressions in dynamic templates. (#29013)
  [Tests] Fix GetResultTests and DocumentFieldTests failures (#29083)
  Reenable LiveVersionMapTests.testRamBytesUsed on Java 9. (#29063)
  Mute failing GetResultTests and DocumentFieldTests
  Improve error message for installing plugin (#28298)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Task Management Issues for anything around the Tasks API - both persistent and node level. >refactoring v6.3.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants