Skip to content
This repository has been archived by the owner on Sep 6, 2024. It is now read-only.

use MessageWrapper to sync acknowledge and timeout extender #23

Merged
merged 7 commits into from
Jun 13, 2024

Conversation

StephanPraetsch
Copy link
Contributor

sync acknowledge and timeout extender: if the message is already acknowledged (deleted) then timeout extender will fail if this happens in the same millisecond

Jun 10, 2024 @ 11:55:14.353
message task successfully processed and message acknowledged: d15a9248-9a19-44db-b895-0f674ec66a2a
	
Jun 10, 2024 @ 11:55:14.353
error while extending message visibility for d15a9248-9a19-44db-b895-0f674ec66a2a

@OOlsen OOlsen self-assigned this Jun 12, 2024
@OOlsen OOlsen self-requested a review June 12, 2024 08:24
String.class), e);
throw new RuntimeException(e);
if (e.getCause() instanceof AlreadyAcknowledgedException) {
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why introducing the new exception if we just ignore it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, removed with adda920

@Getter
private final Message<I> message;

private final AtomicBoolean acknowledged = new AtomicBoolean(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the advantage of heaving an atomic boolean instead of a native boolean? The manipulating methods are already synchronized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

replaced with boolean d1b1b9b

@StephanPraetsch StephanPraetsch merged commit 36ee1eb into master Jun 13, 2024
1 check passed
@StephanPraetsch StephanPraetsch deleted the concurrency-issue-after-acknowledge branch June 13, 2024 07:00
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