Skip to content

[Messenger] Extend SQS visibility timeout for messages that are still being processed #58483

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

Merged
merged 1 commit into from
Nov 9, 2024

Conversation

valtzu
Copy link
Contributor

@valtzu valtzu commented Oct 7, 2024

Q A
Branch? 7.2
Bug fix? no
New feature? yes
Deprecations? no
License MIT

Now that #53508 is merged, let's add keepalive implementation for SQS too.

fabpot added a commit that referenced this pull request Oct 14, 2024
…ethods (valtzu)

This PR was merged into the 7.2 branch.

Discussion
----------

[Console][Messenger] Add `$seconds` to `keepalive()` methods

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| License       | MIT

Make the transport aware for how long (at minimum) the message should be kept alive.

F.e. when extending SQS visibility timeout, you need to pass the visibility timeout (seconds) as parameter. If you pass a value which is less than alarm interval, SQS would resend the message too early. By making the transport aware of when the next keepalive call will happen we'll be able to do some assertions/clamping to improve DX.

This is a prerequisite for #58483.

Commits
-------

e3beb18 Add `$seconds` to `keepalive` methods
@valtzu valtzu force-pushed the add-keepalive-sqs branch 2 times, most recently from 4bbe383 to 41a94df Compare October 14, 2024 15:10
@fabpot
Copy link
Member

fabpot commented Nov 9, 2024

Thank you @valtzu.

@fabpot fabpot merged commit 4e682e4 into symfony:7.2 Nov 9, 2024
10 checks passed
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.

6 participants