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

[💡FEATURE REQUEST]: Current impl of the worker_watcher queue container may violate Pop <-> Push operation under some circumstances #750

Closed
rustatian opened this issue Jul 17, 2021 · 1 comment
Assignees
Labels
S-RFC Request: Request for comments Y-Medium Priority: Medium
Milestone

Comments

@rustatian
Copy link
Member

rustatian commented Jul 17, 2021

Edit: Fixed in the 2.4.0 for the worker's container based on the channel. As the target of this ticket, we need to create a new container described above.

Under some circumstances (TTL, supervisor), when using custom plugins, the order of Pop and Push operations may be violating. When Pool takes the Worker, another worker, which waits on the channel mutex might be pushed into the channel. So, the Pop <-> Push relation might be broken and the worker won't be able to return a response.
Another issue here is if the user used TTL without requests, the killed processes will be on channel mutex:

func (v *Vec) Push(w worker.BaseProcess) {
	v.workers <- w
}

We need to do the following:

  1. Remove the duplicated slice with processes pointers from the worker_watcher.
  2. Implement the Queue data structure which should support the following operations (all O(1)): Pop, Push, Lish (O(n)), Remove, Destroy. This data structure should also support the allocate_timeouts.
  3. Integrate this data structure with worker_watcher.
  4. Remove operation should be used to remove worker by its PID from the Queue without stacking dead processed on the mutex. The dead worker might be at any position in the Queue.
  5. Rethink List() operation. We don't need to return a full process, but only the Memory, CPU, Execs, Time parts.
@rustatian rustatian added the B-regression Bug: regression bugs label Jul 17, 2021
@rustatian rustatian added this to the 2.5.0 milestone Jul 17, 2021
@rustatian rustatian self-assigned this Jul 17, 2021
@rustatian rustatian changed the title [BUG] Current impl of the worker_watcher queue container may freeze Push operation under some circumstances [BUG] Current impl of the worker_watcher queue container may violate Pop <-> Push operation under some circumstances Jul 17, 2021
@rustatian rustatian changed the title [BUG] Current impl of the worker_watcher queue container may violate Pop <-> Push operation under some circumstances [BUG?] Current impl of the worker_watcher queue container may violate Pop <-> Push operation under some circumstances Aug 2, 2021
@rustatian rustatian changed the title [BUG?] Current impl of the worker_watcher queue container may violate Pop <-> Push operation under some circumstances [BUG] Current impl of the worker_watcher queue container may violate Pop <-> Push operation under some circumstances Sep 1, 2021
@dpodoliak dpodoliak removed this from the 2.5.0 milestone Sep 15, 2021
@rustatian rustatian changed the title [BUG] Current impl of the worker_watcher queue container may violate Pop <-> Push operation under some circumstances [💡FEATURE REQUEST]: Current impl of the worker_watcher queue container may violate Pop <-> Push operation under some circumstances Feb 17, 2022
@rustatian rustatian added S-RFC Request: Request for comments and removed F-need-verification B-regression Bug: regression bugs labels Feb 17, 2022
@rustatian
Copy link
Member Author

fixed by: roadrunner-server/sdk#36

@rustatian rustatian added this to the v2.10.6 milestone Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-RFC Request: Request for comments Y-Medium Priority: Medium
Projects
No open projects
Archived in project
Development

No branches or pull requests

2 participants