rumqtt icon indicating copy to clipboard operation
rumqtt copied to clipboard

rumqttc | Unbounded channel

Open carlocorradini opened this issue 1 year ago • 9 comments

At the moment, the only way to construct a client is to provide a fixed client capacity. A client with unlimited channel capacity would be desirable. Furthermore, the pure implementation is already available because rumqttc is built on flume. The client/event_loop has to be refactored in order to enable support for both bounded and unbounded channels. Let me know what you think 🤯

carlocorradini avatar Jul 06 '23 13:07 carlocorradini

Hey, can you elaborate more on why client with unlimited channel capacity would be desirable? cuz, the channel getting full means the client isn't able to process the messages right? so having unlimited capacity will eventually just run out of memory.

swanandx avatar Jul 18 '23 12:07 swanandx

I don't know what the channel capacity is a priori. I prefer the client to run out of memory (very rare) rather than to block :/ Moreover, it's up to the application designer to decide whether to use a bounded or unbounded channel without forcing it to use a bounded channel. Finally, since we rely on a library to do all the "channel stuff" we don't even have to rewrite it from scratch :)

These are my ideas but I prefer to also listen yours :)

carlocorradini avatar Jul 18 '23 13:07 carlocorradini

I do agree with you. I think only change required will be to make cap as Option<> and then based on it we can choose to use bounded/unbounded channel.

Would need to give it a little thought on is it really required/helpful, and worth to break the API. Will get back to you on this, thank you so much!

swanandx avatar Jul 18 '23 13:07 swanandx

Awesome 😎

carlocorradini avatar Jul 18 '23 14:07 carlocorradini

Hey @carlocorradini , lets do it! Would you like to open PR for this?

swanandx avatar Jul 19 '23 08:07 swanandx

@swanandx Yeah 🥳

Unfortunately currently I'm unable to develop on my computer but I'll try in the next weeks 🥳

carlocorradini avatar Jul 19 '23 08:07 carlocorradini

no worries, thank for your contribution :100:

swanandx avatar Jul 19 '23 09:07 swanandx

API idea:

Client::new_bounded(mqttoptions, 10);
Client::new_unbounded(mqttoptions);

and deprecate Client::new(...);

I think this new API is more self descriptive.

What do you think?

carlocorradini avatar Jul 26 '23 06:07 carlocorradini

That's a great suggestion, but for now I think Option<cap> will work out better. In future, we are planning to have builder pattern for it 😃

e.g. suggested by @de-sh

let client = AsyncClient::builder().host("...").port(...).capacity(...).build();

swanandx avatar Jul 28 '23 13:07 swanandx