Re: [async-mqtt5] Review questions

From: Ivica Siladic (ivica.siladic_at_[hidden])
Date: 2024-10-20 18:13:22

Hi Marcelo,

> On 20.10.2024., at 12:36, Marcelo Zimbres Silva <mzimbres_at_[hidden]> wrote:
> 
>> Throughout the library, we’ve been careful to ensure that "you don’t get for what you did’t pay," especially when it comes to performance. In terms of logging, this means we wanted to avoid any runtime overhead associated with logging if you choose not to use it, even simple `if` checks.
> 
> I strongly disagree with you here, in pretty much every scenario I can think of in terms of IO an if check is costless, this includes: resolve, connect, handshake, reads and writes. You can't even measure it since IO operations are orders of magnitude more costly.
> 
> On the other hand, lack of logging results in a lot of frustration not only for your users but also for the authors as bug reports won't contain enough information for you to investigate the problem. There are just so many things that can go wrong, freeze etc.
> 
>> In the absence of a better solution, we recommend users add their own logging code directly into the async_mqtt5 source during development. It’s very far from ideal, but we couldn’t come up with a better approach.
> 
> I don't think it is reasonable to expect users to even know where to add log lines in a 15k loc codebase (that uses callback to implement its composed operations). This will result in a lot of frustration.
> 
> It would also only be possible as long as this library is kept header-only and I think it shouldn't.
> 
> What you could do here is to introduce a BOOST_MQTT5_ENABLE_BOOST_LOG macro and add the lines yourself to the code.
> 

Let me be very clear: the lack of logging is definitely not a good thing, and as you mentioned, debugging network I/O issues can be extremely frustrating. Logging is crucial in such cases. I just want logging to be a NOOP if the developer chooses not to enable it. This can be done quite easily with macros, for instance. However, our goal is to find a more elegant, C++-style solution that achieves the same effect.

>> However, conceptually, the right solution is still unclear. Technically, we understand how it can be done, but the question is more about the intended design. I recall Richard Hodges once explaining the role of a bound executor to a handler, stating that "the final handler, as well as all intermediate handlers of the async operation, will be invoked using the bound executor." While the initiation code of the async function may not use the bound executor, once the internal code calls another internal async function, every subsequent async function, including the final handler, will be executed using the originally bound executor.
> 
> AFAICS this leads into inconsistency since async_read uses the conn.async_run executor while the async_write will use the async_publish executor. That is why I argue that the bound executor in async_publish should only be used to deliver the completion while everything else runs on the connection executor, not on the async_run bound executor, which should also be only used to deliver the completion.
> 

Ok, understood.

> Marcelo

Thanks,

Ivica Siladic