Re: [boost] [async-mqtt5] Formal review starts today

Thanks for the suggestions and encouraging me to clarify. On 28/10/2024 6:37 am, Marcelo Zimbres Silva wrote:

Hi,

On Sun, 27 Oct 2024 at 09:53, Darryl Green via Boost mailto:boost@lists.boost.org> wrote:

This is another face of the "it needs logging" comments from other reviewers. If the user can be informed what the library is doing, then the user can simply log this, or, potentially, react in other ways.

I would like to mention that depending on how it is implemented, logging will also solve the "notification" problem. In Boost.Redis, for example, the async_run function gets a lightweight log object with the following interface

   class logger {    public:       void on_resolve(...);       void on_connect(...);       void on_ssl_handshake(...);       void on_connection_lost(...);       void on_write(...);       void on_read(...);       void on_run(...);       void on_hello(...);       void on_check_health(...);       ...    };

This is primarily meant for logging but can also be used to communicate events to the upper layer. I must admit though that I don't find this approach scalable, there are too many places that need logging and you don't want to extend the class with new functions every time. Also, there does not seem to be any use for this except logging, so why not use an interface with trace(), info(), warning(), critical() etc. instead.

I simply meant that its somewhat like logging. Im not sure if on_<everything> is appropriate but being able to react to connectivity changes seems pretty fundamental.

...

...

- Does the API match with current best practices?

If we took Beast as an example of best practice, probably not.

I would like to understand how you come to this conclusion, because it is low-level Beast requires users to perform steps manually: resolve, connect, handshake, write, read etc. Also, if anything more advanced like http pipelining is needed you can expect to write a lot more code, at least compared to popular http libs written in go, node-js etc. The library being reviewed on the other hand hides all those steps plus the multiplexing of multiple async_publishes behind a single call async_run function, this is extremely convenient IMO.

Sometimes you just want to do a "it works or it doesn't" operation in, of all things, an MQTT client. That said, since posting my review I saw in other discussion that there is the option to use total cancellation at the async publish level. I'd only seen examples and docs for some destructive cancellation approaches. This alone might be enough to address some of the problems while using only a high level interface.

...

As an interface for implementing latency sensitive real time distributed systems, probably not.

What part of this library exactly is this referring to? I agree it has room for improving latency, for example, the async_receive signature reads ... IMO a better solution would be to allow users direct access to the buffer so he can avoid copies, this is how Boost.Redis and IIRC Boost.Mysql works.

I wasn't referring to the performance of the library code, I was as in all my comments referring to the performance / service offered by a system that includes an application using the library code and that is trying to deliver a particular type of service. Consider a client that has a large, active and complex enough collection of topics to publish that the performance features we are talking about here are of benefit. In some cases not only is low latency in the sense of ability to utilise the large capacity channel important, but so is constraining latency when the rate of publication attempts is actually higher than the capacity of the channel. When and what to "leak" becomes important. Consider an intermittently or "weakly" connected but very active client publishing thousands of topics, that normally get queued and delivered to some remote server (it may or may not be a "broker" in the conventional sense). If even with those features achieving high efficiency / throughput the rate of publication is such that the latency from the publisher to the the server grows due to channel capacity issues (slow channels and in the extreme case, channels that are intermittently unavailable) then for some groups of topics it is likely appropriate and even necessary to drop old/stale "queued" publications in favour of making sure the server remains up to date (at the loss of some visibility / temporal resolution regarding what are now "historical" values/changes). Note that it is likely there are other topic groups for which delivering every publication is important. MQTT and the library claim to support multiplexing topics and messages with different QoS. One common usage/expectation is that QoS 0 messages will be delivered "fast" but that the eventual receiver of them will not see every message (as one could expect from calling this at_most_once). Meanwhile a message that must be delivered exactly once.. is slow(er).. but not expected (or is it) to get "stuck" indefinitely behind a (growing?) set of QoS 0 messages. Id like to see how one can use the library in a system with this type of performance requirement. Id also like to se how it can be used with QoS 1 and a constrained window used specifically to stop the "channel" (TCP stack and general buffer bloat) blowing out latency - its not in practice safe to assume latency sensitive but relatively message loss insensitive traffic should be sent with "QoS 0" in MQTT terms. It is important that if one does intend to drop messages (which with a TCP transport needs to happen prior to the network stack) that this can be done "fairly". A high level library that can do all this needs a tunable "QoS" in the general sense (not just the selection of one of 3 values for an MQTT parameter). A low level library can legitimately claim its out of scope. This library has high level features but appears to lack some tunability (but that could be user ignorance on my part) and also access to low level features to allow the user to take responsibility for these issues. That needs addressing. Ive also seen some informed discussion around whether a library should be accepted exactly so that the author is encouraged to modify and improve it. I am all for encouraging authors to produce high quality, useful libraries and my "reject at this point" is using the "reject" word because that is the only "non accept" word on offer unless one can formulate a detailed list of conditions for "conditional acceptance" - which are as Vinnie already mentioned is hard for a library like this. Regards Darryl.