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

refactor(rumqttd): let Broker derive Clone and remove start mut self #700

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

crimx
Copy link

@crimx crimx commented Aug 29, 2023

fixes #699

@swanandx
Copy link
Member

swanandx commented Sep 1, 2023

Hey, I'm curious how does this PR solves the mentioned discussion? can you elaborate more on this 😅

@crimx
Copy link
Author

crimx commented Sep 1, 2023

With the current implementation, we are moving a mut broker to a thread and start. There is no way to access the broker outside the thread afterwards.

By removing mut self on start, we can simply share the broker via Arc.

Or since both Broker.config and Broker.router_tx are thread safe and clonable, we can just move a broker.clone() to a thread and start, move another clone to other thread and link.

@h3nill
Copy link

h3nill commented Sep 3, 2023

while sharing would be possible, it would also allow start to be called multiple times which shouldn't happen.

Can you share why do you want to share the broker to another thread?

@crimx
Copy link
Author

crimx commented Sep 3, 2023

You are right this could be an issue.

Mu use case is that I want to link and subscribe to dynamic topics that are created after broker starts.

@swanandx
Copy link
Member

swanandx commented Sep 3, 2023

I want to link and subscribe to dynamic topics that are created after broker starts.

You can create link using LinkBuilder with just client id and router_tx like here .

So you might not need whole Broker, just router_tx should be enough!

@crimx
Copy link
Author

crimx commented Sep 4, 2023

Yes that'll do. I've read this but therouter_tx and LinkBuilder are private so I assumed you want to hide the details.

@swanandx
Copy link
Member

swanandx commented Sep 4, 2023

I've read this but therouter_tx and LinkBuilder are private

Good catch, i somehow managed to miss this detail 😬😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants