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

AR Fixes: task leader handling, restoring, state updating, AR.Destroy deadlocks #4803

Merged
merged 8 commits into from
Oct 29, 2018

Conversation

schmichael
Copy link
Member

@schmichael schmichael commented Oct 18, 2018

Replacement of #4775 rebased on master post-#4792 merge.

Fixes:

  • Fix panic on restore due to nil prev alloc watcher ee06875
  • Fix panic when copying a nil TaskHandle 942f1d7
  • Port task leader tests from deprecated alloc runner and get them passing (with -race)
  • Refactor AR task killing into a single method that kills the leader first
  • Fix deadlock in AR.Destroy (see note below)
  • Documented and tested state database behavior to make restoring more robust.

Notable Code Change: AllocRunner.Run

Instead of go ar.Run() you should now call ar.Run() as AllocRunner.Run now creates its own goroutine. This allows it to synchronously mark that it has run so that when AR.Destroy is called it knows whether or not to wait for ar.Run() to exit.

Notable Test Change: TestClient Cleanup

Made TestClientConfig and TestClient return a cleanup func to clean up its state and alloc dirs after a test finishes.

* Migrated all of the old leader task tests and got them passing
* Refactor and consolidate task killing code in AR to always kill leader
  tasks first
* Fixed lots of issues with state restoring
* Fixed deadlock in AR.Destroy if AR.Run had never been called
* Added a new in memory statedb for testing
Fixes a panic. Left a comment on how the behavior could be improved, but
this is what releases <0.9.0 did.
@dadgar
Copy link
Contributor

dadgar commented Oct 29, 2018

Reviewed alloc_runner and it LGTM. Didn't look at all the files

@schmichael schmichael merged commit 16c25b8 into master Oct 29, 2018
@schmichael schmichael deleted the b-leader-fixes branch October 29, 2018 22:39
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants