Skip to content

[Messenger] Adding MessageCountAwareInterface to get transport message count #30757

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
Apr 3, 2019

Conversation

weaverryan
Copy link
Member

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets none
License MIT
Doc PR symfony/symfony-docs#11236

This adds a new optional interface that receivers should implement to give an approximate number of the messages "waiting" to be handled. Why? Because, with this, you could design a system that dynamically adds/removes worker processes if a specific transport is getting slammed and needs help. Creating that system could be something we discuss for core later, but this at least makes it possible - and means it could be implemented by the user or in a bundle... which I might do if we don't get it in core ;).

@sroze
Copy link
Contributor

sroze commented Mar 31, 2019

I don't think we should add it to the core without knowing the actually use-case tbh.

@nicolas-grekas nicolas-grekas added this to the next milestone Mar 31, 2019
@weaverryan weaverryan force-pushed the messenger-receiver-count branch 3 times, most recently from dc21b88 to adda867 Compare March 31, 2019 16:26
@weaverryan
Copy link
Member Author

I don't think we should add it to the core without knowing the actually use-case tbh.

Very fair. So, let me try to be more specific... even though I admit, I'm "planning ahead" a bit here. One easy thing to look at is Larave's Horizon, which takes advantage of the "size" of a queue for (A) displaying a health dashboard and (B) for automatically spinning up/down more/less workers for each transport, based on the size. Without this feature (which is pretty simple), creating something like that as a 3rd party bundle, for example, will not be possible.

Rebased and comments addressed!

@weaverryan weaverryan force-pushed the messenger-receiver-count branch 2 times, most recently from b7a472b to decd53f Compare March 31, 2019 17:19
@weaverryan
Copy link
Member Author

@vincenttouzet could you check the Doctrine part of this PR out? There is an integration test, but would love your +1 technically for it.

Copy link
Contributor

@vincenttouzet vincenttouzet left a comment

Choose a reason for hiding this comment

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

Except the comment on the fetch / fetchColumn this looks good to me and very promising !

@weaverryan weaverryan force-pushed the messenger-receiver-count branch 2 times, most recently from 7292b2b to acddcae Compare April 1, 2019 15:33
@nicolas-grekas nicolas-grekas changed the title [Messenger] Adding MessageCountAwareReceiverInterface to get transport message count [Messenger] Adding MessageCountAwareInterface to get transport message count Apr 3, 2019
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

(it'd be nice updating the 1st commit message with the new name of the interface)

@fabpot fabpot force-pushed the messenger-receiver-count branch from acddcae to fc5b0cf Compare April 3, 2019 14:45
@fabpot
Copy link
Member

fabpot commented Apr 3, 2019

Thank you @weaverryan.

@fabpot fabpot merged commit fc5b0cf into symfony:master Apr 3, 2019
fabpot added a commit that referenced this pull request Apr 3, 2019
…ransport message count (weaverryan)

This PR was squashed before being merged into the 4.3-dev branch (closes #30757).

Discussion
----------

[Messenger] Adding MessageCountAwareInterface to get transport message count

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | none
| License       | MIT
| Doc PR        | symfony/symfony-docs#11236

This adds a new optional interface that receivers should implement to give an approximate number of the messages "waiting" to be handled. Why? Because, with this, you could design a system that dynamically adds/removes worker processes if a specific transport is getting slammed and needs help. Creating that system could be something we discuss for core later, but this at least makes it possible - and means it could be implemented by the user or in a bundle... which I might do if we don't get it in core ;).

Commits
-------

fc5b0cf [Messenger] Adding MessageCountAwareInterface to get transport message count

$sender->send($first = new Envelope(new DummyMessage('First')));
$sender->send($second = new Envelope(new DummyMessage('Second')));
$sender->send($second = new Envelope(new DummyMessage('Third')));
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

@weaverryan weaverryan deleted the messenger-receiver-count branch April 4, 2019 11:28
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019
@fabpot fabpot mentioned this pull request May 9, 2019
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.

9 participants