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

Allow file descriptors >1024 to be used #1033

Closed
petitgrf opened this issue Jan 2, 2021 · 6 comments
Closed

Allow file descriptors >1024 to be used #1033

petitgrf opened this issue Jan 2, 2021 · 6 comments

Comments

@petitgrf
Copy link

petitgrf commented Jan 2, 2021

select() only supports fd < FD_SETSIZE (typically 1024) so it fails when the application has more than 1024 sockets.
Should MQTTAsync be upgraded from select() to poll()?

@icraggs
Copy link
Contributor

icraggs commented Jan 2, 2021

I don't think so. Poll suffers from the same performance issues as select when the number of sockets becomes large. If a change were to be adopted, it should be to epoll, which solves the performance problem. However, epoll, last time I looked, is not portable so would need specific code for Windows and other platforms.

Additionally, the library wasn't intended to be used for large numners of sockets. 1024 is lot of sockets for an MQTT application. If you are thinking about building a performance testing application then another library, one written specifically with that aim in mind, is probably better.

@petitgrf
Copy link
Author

petitgrf commented Jan 2, 2021 via email

@icraggs
Copy link
Contributor

icraggs commented Jan 4, 2021

Well this unlikely to be a priority for me. The risk of destabilizing the code internals and causing quite a lot of maintenance work is quite high, in my opinion.

You could open a PR for the change, but it could be more work than restructuring your application so that the MQTT interaction took place in a separate process, or some other solution.

@icraggs icraggs changed the title MQTTAsync change from select() to poll() Allow file descriptors >1024 to be used Jan 12, 2021
@icraggs icraggs added this to the Backlog milestone Jan 12, 2021
@fpagliughi
Copy link
Contributor

There was an issue posted to the Rust client page about a segfault when attempting to create more than 1000 active clients. (Coincidentally, trying to create 1100). I'm perfectly fine with a hard limit of ~1000 clients in a process, but I would prefer a clean error code when trying to create or connect the client rather than a segfault. Oddly, the fault seems to happen in a Socket_close() function.

I'll see if I can put together a pure C test app to reproduce the problem:
Basically try to create 1100 clients connected to mosquitto on localhost, and publish a message to each upon creation. Then, when complete, delete them all, one at a time.

@icraggs icraggs modified the milestones: Backlog, 1.3.10 Feb 2, 2022
@icraggs
Copy link
Contributor

icraggs commented Feb 2, 2022

As it happens, I'm trying out using poll rather than select which would at least make this situation different :-). The main reason I was motivated to look at it was to be able to interrupt the call when a socket is closed, but using poll could solve some other issues as well.

icraggs added a commit that referenced this issue Feb 4, 2022
@icraggs
Copy link
Contributor

icraggs commented Feb 17, 2022

The poll code is now in the develop branch, so this restriction ought to have now been removed (not that I've tried to create more than 1000 clients).

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

No branches or pull requests

3 participants