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

1125 api failures and race conditions with concurrent publish update operations #1156

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

randombenj
Copy link
Member

Fixes #1125

Requirements

All new code should be covered with tests, documentation should be updated. CI should pass.

Description of the Change

While implementing the ?_async=true task api,
resources which must not be handled concurrently
were protected by resource groups.
However the non async resources could still be
operated on concurrently by just doing two api
requests at the same time.

This change fixes this behaviour by
running sync operations in the new task framework
and waiting internally before returning
a result.

Checklist

  • unit-test added (if change is algorithm)
  • functional test added/updated (if change is functional)
  • man page updated (if applicable)
  • bash completion updated (if applicable)
  • documentation updated
  • author name in AUTHORS

@randombenj randombenj force-pushed the 1125-api-failures-and-race-conditions-with-concurrent-publish-update-operations branch 3 times, most recently from c314a70 to f860ca6 Compare March 9, 2023 15:58
@randombenj randombenj added this to the 1.6.0 milestone May 31, 2023
@randombenj randombenj removed the 1.6.0 label May 31, 2023
While implementing the `?_async=true` task api,
resources which must not be handled concurrently
were protected by resource groups.
However the non async resources could still be
operated on concurrently by just doing two api
requests at the same time.

This change fixes this behaviour by
running sync operations in the new task framework
and waiting internally before returning
a result.
@randombenj randombenj force-pushed the 1125-api-failures-and-race-conditions-with-concurrent-publish-update-operations branch from f860ca6 to 44998a2 Compare May 31, 2023 14:13
@r4co0n
Copy link

r4co0n commented Jul 7, 2023

You may be aware, but this pull request mixes concerns that seem completely unrelated to me:

  • Why we do need to deprecate calls to rand.Seed is not explained, and more importantly, I do not see how this is connected to solving API lock errors a.k.a. API race conditions. Why do we need this in here?
  • Why should support for multiple Go versions be dropped from the CI (, or generally, following the commit message)?
  • Why do we fix a CI linting issue here? Why is this problem seemingly only tracked with a FIXME comment and a link to our dependency's issues?
  • Why do we need to bump the Go version here?

Please excuse my rather rude demeanour, I think your changes should probably all be merged, but I see little reasoning as to why this should all be a single pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API: failures and race conditions with concurrent publish update operations
3 participants