Re: aedis review starts today

From: Marcelo Zimbres Silva (mzimbres_at_[hidden])
Date: 2023-01-21 21:44:50

On Sat, 21 Jan 2023 at 20:00, Ruben Perez <rubenperez038_at_[hidden]> wrote:
>
> This library introduces something unusual to Asio-based components,
> which is the command queue. I see advantages on having it (as it
> provides out-of-the-box efficiency), but also some downsides (it is
> opinionated, not allowing for strategies like connection pooling).

Why do you think it does not allow connection pooling? The user can
create as many connections as he wants. I also don't see it as
opinionated as optimal use of the resp3 protocol mandates this
feature. The reason why node-js performs so well in the benchmarks
even when compared to go, rust, c++ and other languages is because it
gets this correctly (automatic pipelining).

> I like how adapt() works. Parsing into compile-time structures is
> always good.  However, as a Redis novice I am, I've had a hard time
> debugging things when I got a mismatch between the type passed to
> adapt() and what the command expects.  The error happens at parse
> time, which I think is pretty dangerous, as it's very easy to get
> things overlooked. As an example of this, the library's own example
> cpp20_containers, in the hgetall function, uses the type
> std::map<std::string, std::string> for the hgetall response, which
> will be okay unless the key is not set, in which case you will get
> an error. I think this has an enormous potential for causing hidden
> bugs (and even vulnerabilities). I've also noticed that if you
> mismatch the sizes of the pipeline request and response, you get an
> assertion (so UB).

To fix this properly I would need a tool similar to what protobuff is
to grpc: generate requests and responses stub from a schema.

Aedis however is not trying to do that, it provides the infrastructure
with which such a tool could be created.  Perhaps there is no place in
Boost for such a library, but it is still necessary.

> The way of handling network errors is very elegant, and it shows the
> author's effort on it. The reconnect loop is clean. Congratulations.

Thanks

> I have invested around 2 days in playing with the library and
> writing this review.

Wow, great. Thanks for that.

> Thanks again Marcelo for taking your time and effort.

Thank you too for the effort you invested, it is highly appreciated. I
don't think I can address all your points but they will certainly help
me improve Aedis.

Regards,
Marcelo