feat: separate Exec ctx and Closed ctx #206
Conversation
derekperkins
changed the title
|
We can close #202 but there is still a problem with the tests. I found the root cause. There is a concurrent access problem in pipeline.go. It's triggered by the new pipeline concurrency option and it seems that it requires a non-trivial review and refactor. I'm on it. |
|
Cool, thanks for tracking that down. Once that is fixed and this PR merged, I can roll it out again in prod to see if it helps to solve #198 |
|
Hey, @derekperkins I have fixed the data race issue but your branch requires rebase & conflict fix. |
|
Awesome, I'll fix it up |
- fixes a bug where Discard would always return ErrPipelineClosed after calling Exec, making it impossible to reuse it - protects FutureXXX calls from panicking or returning stale/invalid data
derekperkins
force-pushed
the
invalidate-pipeline
branch
from
bd435f9
to
a8a0f70
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
|
I have merged your PR @derekperkins, thank you for your contribution! |
|
Awesome, I'll roll that out today or tomorrow in prod and see how it runs! Hopefully this also resolves #198 |
|
I released Olric v0.5.1. Waiting for the results! https://github.com/buraksezer/olric/releases/tag/v0.5.1 |
feat: add PipelineConcurrency optionThis adds a pipeline option to the DMap pipeline interface, which should be backwards compatible. It also changes the default from 1 to runtime.NumCPU(), which from the variable naming, appears to have been the original intent.
fixes #201
feat: separate Exec ctx and Closed ctxfixes #205
This is rebased on top of #202 to avoid merge conflicts, so we should either merge that one first, or close it and use this PR for both.The second commit is extra noisy because the
closedCtxfield caused indentation changes to happen on all pipeline structs