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

refactor(concurrency): consistent node-level locks #13055

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

samugi
Copy link
Member

@samugi samugi commented May 22, 2024

Summary

Several places in the gateway need a node-level lock, some of them used slightly different implementations. This refactor brings consistency in the ways we do node-level locking by using the same implementation (concurrency.with_worker_mutex) everywhere.

Note: in order to maintain the logic of declarative.import unaltered, the concurrency.with_worker_mutex function has been slightly modified to return an additional value ttl in case of timeout error, so that retry_after could be implemented as before.

Checklist

  • (no/refactor) The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
  • (noo) There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

KAG-2337

@samugi samugi marked this pull request as draft May 22, 2024 10:27
@github-actions github-actions bot added core/clustering core/db cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels May 22, 2024
@samugi samugi force-pushed the refactor/concurrency-worker-mutex branch from 5119f87 to 4442aa2 Compare May 22, 2024 11:36
Several places in the gateway need a node-level lock, some of them used
slightly different implementations. This refactor brings consistency in
the ways we do node-level locking by using the same implementation
(concurrency.with_worker_mutex) everywhere.
@samugi samugi force-pushed the refactor/concurrency-worker-mutex branch from 4442aa2 to 5755745 Compare May 22, 2024 13:33
@samugi samugi marked this pull request as ready for review May 22, 2024 13:54
Copy link
Member

@bungle bungle left a comment

Choose a reason for hiding this comment

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

Just one comment.

kong/cluster_events/init.lua Show resolved Hide resolved
@samugi samugi requested a review from bungle June 26, 2024 14:26
Copy link
Contributor

@nowNick nowNick left a comment

Choose a reason for hiding this comment

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

I like the refactoring but I'm missing a little bit of context to fully understand what's going on here. I added one comment that might help me wrap my head around it.

kong/concurrency.lua Show resolved Hide resolved
Copy link
Contributor

@nowNick nowNick left a comment

Choose a reason for hiding this comment

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

Now it all makes sense! Thank you for explanation 🙇 . I went through the logic here and in the libs you mentioned and everything looks good to me 👍


I just left one comment that's not really related to your refactor. I was just curious what 0.001 means. If you know - maybe you can extract it to a constant and give it a name?

kong/cluster_events/init.lua Show resolved Hide resolved
@samugi samugi merged commit 7da959c into master Jul 11, 2024
29 checks passed
@samugi samugi deleted the refactor/concurrency-worker-mutex branch July 11, 2024 17:17
@team-gateway-bot
Copy link
Collaborator

Successfully created cherry-pick PR for master:

@samugi samugi restored the refactor/concurrency-worker-mutex branch July 12, 2024 10:10
@samugi samugi deleted the refactor/concurrency-worker-mutex branch July 12, 2024 10:10
@bungle bungle restored the refactor/concurrency-worker-mutex branch August 13, 2024 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/clustering core/db size/L skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants