Replace custom semaphores with golang.org/x/semaphore
What does this MR do?
This MR replaces the custom semaphore logic in builds_helper with the semaphore implementation provided by
This MR renames
tryAcquireRequest. This mirrors the structure of
This MR changes
releaseRequest to panic instead of returning false when a bad release is made. As far as I know, none of the calls to those methods are at risk of making bad releases. This mirrors the structure of
To facilitate that change, it also changes the way that
buildsHelper indexes its semaphores. The current implementation indexes the semaphores by the runner config's token value and uses the max value provided by the passed runner config. That model was incompatible with
semaphore.Weighted because it was effectively creating a semaphore with mutable max.
Both the current implementation and this implementation exhibit some negative behaviors.
The current code:
- Shares a semaphore among all configs with the same token
- Uses the max from given config
- Suffers a collision when two configs use the same token with different concurrency limits
- Behaves oddly when stale configurations are circulating among the workers.
This new code:
- Creates a unique semaphore for each config object in memory
- Uses the max from the original config object
- Can result in higher-than-specified concurrency values when stale configurations are circulating among the workers.
Why was this MR needed?
semaphore.Weighted reduces the amount of synchronization code to be maintained and provides the opportunity to use a blocking call in the future instead of a non-blocking call.
Are there points in the code the reviewer needs to double check?
Does this MR meet the acceptance criteria?
- Documentation created/updated
- Added for this feature/bug
- All builds are passing
Branch has no merge conflicts with
master(if you do - rebase it please)