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] Adding MessageCountAwareInterface to get transport message count #30757

Merged
merged 1 commit into from Apr 3, 2019

Conversation

@weaverryan
Copy link
Member

commented Mar 28, 2019

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

This comment has been minimized.

Copy link
Member

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 weaverryan:messenger-receiver-count branch 3 times, most recently from dc21b88 to adda867 Mar 31, 2019

@weaverryan

This comment has been minimized.

Copy link
Member Author

commented Mar 31, 2019

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 weaverryan:messenger-receiver-count branch 2 times, most recently from b7a472b to decd53f Mar 31, 2019

@weaverryan

This comment has been minimized.

Copy link
Member Author

commented Mar 31, 2019

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

@vincenttouzet
Copy link
Contributor

left a comment

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

@weaverryan weaverryan force-pushed the weaverryan:messenger-receiver-count branch 2 times, most recently from 7292b2b to acddcae Apr 1, 2019

@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

@nicolas-grekas
Copy link
Member

left a comment

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

@fabpot

fabpot approved these changes Apr 3, 2019

@fabpot fabpot force-pushed the weaverryan:messenger-receiver-count branch from acddcae to fc5b0cf Apr 3, 2019

@fabpot

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

Thank you @weaverryan.

@fabpot fabpot merged commit fc5b0cf into symfony:master Apr 3, 2019

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Apr 3, 2019

feature #30757 [Messenger] Adding MessageCountAwareInterface to get t…
…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')));

This comment has been minimized.

Copy link
@ro0NL

ro0NL Apr 4, 2019

Contributor

🤔

This comment has been minimized.

Copy link
@weaverryan

@weaverryan weaverryan deleted the weaverryan:messenger-receiver-count branch Apr 4, 2019

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019

@fabpot fabpot referenced this pull request May 9, 2019

Merged

Release v4.3.0-BETA1 #31435

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.