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

feat: Make SingleConsumerMultiProducer the default mailbox for stream. #917

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

He-Pin
Copy link
Member

@He-Pin He-Pin commented Jan 8, 2024

Motivation:
Make stream actor using the SingleConsumerMultiProducer by default for performance.

refs: #915

@He-Pin He-Pin added this to the 1.1.0 milestone Jan 8, 2024
@laglangyue
Copy link
Contributor

It seems that a default configuration has been added. Are there any unit test?

@He-Pin
Copy link
Member Author

He-Pin commented Jan 9, 2024

@laglangyue I don't this this code need any test, and a test may need another pr first:), you can try with a debuger to see the mailboxtype.

@laglangyue
Copy link
Contributor

lgtm

@He-Pin He-Pin requested a review from mdedetrich January 9, 2024 06:55
@He-Pin He-Pin added the t:stream Pekko Streams label Jan 9, 2024
@He-Pin
Copy link
Member Author

He-Pin commented Jan 10, 2024

@mdedetrich Would you like to give this a review? this was landed in Akka 2.7

Copy link
Contributor

@mdedetrich mdedetrich left a comment

Choose a reason for hiding this comment

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

lgtm. It may be worth it to create a simple unit test that confirms PhasedFusingActorMaterializer.MailboxConfigName is actually being used when a stream is run, but if this is too hard then don't worry about it.

@He-Pin
Copy link
Member Author

He-Pin commented Jan 11, 2024

@mdedetrich Not easy to test, that's why I want the #919 . and I did confirm that works with debuger.

@mdedetrich
Copy link
Contributor

mdedetrich commented Jan 11, 2024

@mdedetrich Not easy to test, that's why I want the #919 . and I did confirm that works with debuger.

No worries, I approved the PR anyways so its good from my end

@He-Pin He-Pin merged commit 3f8be56 into apache:main Jan 11, 2024
18 checks passed
@He-Pin He-Pin deleted the typed branch January 11, 2024 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t:stream Pekko Streams
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants