Skip to content
This repository has been archived by the owner on Jun 15, 2024. It is now read-only.

Increase buffers to decrease deadlock probability #143

Closed
wants to merge 1 commit into from
Closed

Increase buffers to decrease deadlock probability #143

wants to merge 1 commit into from

Conversation

sroidl
Copy link

@sroidl sroidl commented Dec 4, 2016

In our pipelines we are experiencing further deadlocks, especially when multiple builds a running and /or there are a lot of build-steps running in-parallel.

With an artificial setup, I can reproduce a deadlock situation, even when :step-updates-per-sec is down to 1. One solution seems to be increasing the buffer size of the pipeline-state-update.

@flosell
Copy link
Owner

flosell commented Dec 6, 2016

hi, thanks for letting me know this issue is still there and giving me an example to reproduce the issue!

Your change seems to be a workaround that works up to a certain load. But I guess travis-ci wants to tell me, the change doesn't always fix the issue and we'll have to do some thinking on a more fundamental level. I have opened #144 to capture the overall issue and the different aspects to it.

I have a feeling like solving the fundamental issue will not be a quick fix so I'm willing to go forward with a quick fix workaround like you outlined.

Three things could be improved though:

  • make the buffer size configurable for the user (like :step-updates-per-sec), this way they can trade off deadlock-safety for memory (e.g. keep it lower if they don't have lots of builds/steps running at the same time or set it excessively high if they have lot's of parallelism)
  • make sure the test is less flaky (or drop it, we can reintroduce it once we are sure the underlying issue is really fixed)
  • add a changelog-entry to document the new parameter and behavior so it's obvious to others

@flosell
Copy link
Owner

flosell commented Dec 11, 2016

@sroidl I reworked the event-bus to try to resolve the root cause for this issue (see #144). Can you confirm that this fixes the issue? (set :use-new-event-bus true to activate the new event-bus)

@sroidl
Copy link
Author

sroidl commented Dec 11, 2016

Will try the fix in the next days. Closing this PR for now.

@sroidl sroidl closed this Dec 11, 2016
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.

2 participants