Skip to content
This repository was archived by the owner on Dec 13, 2023. It is now read-only.

Make LocalOnlyLock reentrant#3513

Merged
v1r3n merged 1 commit intoNetflix:mainfrom
marosmars:reentrant_local_lock
Mar 11, 2023
Merged

Make LocalOnlyLock reentrant#3513
v1r3n merged 1 commit intoNetflix:mainfrom
marosmars:reentrant_local_lock

Conversation

@marosmars
Copy link
Contributor

Pull Request type

  • [ X] Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes (Please run ./gradlew generateLock saveLock to refresh dependencies)
  • WHOSUSING.md
  • Other (please describe):

NOTE: Please remember to run ./gradlew spotlessApply to fix any format violations.

Changes in this PR

It did not support reentrancy.
This was a problem at least in following case:

worker -> updateTask -> throw new TerminateWorkflowException -> terminateWorkflow

The lock was acquired twice during above execution order. With no reentrancy, the second lock (wf terminateion) failed and sweeper had to wait until the first lock (task update) was released and only then workflow could be terminated.

Alternatives considered

No alternatives, this was a clear break of the API contract as defined in the Lock service

It did not support reentrancy.
This was a problem at least in following case:

worker -> updateTask -> throw new TerminateWorkflowException ->
terminateWorkflow

The lock was acquired twice during above execution order. With no
reentrancy, the second lock (wf terminateion) failed and sweeper had to wait until the
first lock (task update) was released and only then workflow could be
terminated.

Signed-off-by: Maros Marsalek <mmarsalek@frinx.io>
@marosmars marosmars force-pushed the reentrant_local_lock branch from 5dd1ab3 to eb55096 Compare March 7, 2023 08:54
@marosmars
Copy link
Contributor Author

Hey @v1r3n , can we get a quick look on this small fix ?

Thx, Maros

@v1r3n v1r3n merged commit ad1c785 into Netflix:main Mar 11, 2023
@marosmars marosmars deleted the reentrant_local_lock branch March 13, 2023 14:45
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