Skip to content

Conversation

@hermanschaaf
Copy link
Member

@hermanschaaf hermanschaaf commented Oct 11, 2022

We were missing a Release call on the resource_concurrency semaphore.

Copy link
Contributor

@yevgenypats yevgenypats left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nit on order

wg.Add(1)
go func() {
defer wg.Done()
defer resourcesSem.Release(1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: let's keep the order the same

so it should be:

defer resourceSem.Release(1) (release the resource last because we acquired it first)
defer wg.Done() (so first we release the workgroup as it was add last)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wg.Done() 👍

@kodiakhq kodiakhq bot merged commit 051e247 into main Oct 11, 2022
@kodiakhq kodiakhq bot deleted the fix-resource-concurrency branch October 11, 2022 11:06
hermanschaaf pushed a commit that referenced this pull request Oct 11, 2022
🤖 I have created a release *beep* *boop*
---


##
[0.13.3](v0.13.2...v0.13.3)
(2022-10-11)


### Bug Fixes

* Call Release on resource semaphore
([#279](#279))
([051e247](051e247))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants