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

[Messenger] Keepalive support for Doctrine & Redis #20721

Open
wants to merge 1 commit into
base: 7.3
Choose a base branch
from

Conversation

HypeMC
Copy link
Contributor

@HypeMC HypeMC commented Mar 4, 2025

Keepalive support for the Doctrine and Redis transports was added in symfony/symfony#59601 and symfony/symfony#59360, respectively.

Comment on lines -1748 to -1750
The keepalive feature, which prevents messages from being prematurely redelivered during
long-running processing, updates the ``delivered_at`` timestamp periodically to ensure
the message is marked as "in progress".
Copy link
Contributor Author

@HypeMC HypeMC Mar 4, 2025

Choose a reason for hiding this comment

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

I don't think this is needed since no other transport has an explanation of how the keepalive mechanism is implemented and I'm not sure it's even relevant.

Copy link
Contributor

Choose a reason for hiding this comment

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

While this is an implementation detail, it could be helpful to understand the how, that said, its ok for me to remove 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.

Maybe we could add a similar explanation for each transport that has the keepalive mechanism implemented?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes 👍

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.

3 participants