On Sat, 21 Jan 2023 at 16:57, Ruben Perez <rubenperez038@gmail.com> wrote:
Final set of questions, I hope.
1. If you write something like
aedis::request req; std::tuple<std::optional<std::string>> res; req.push("LPOP", "something_not_a_list"); // will cause an error co_await conn.async_exec(req, adapt(res));
You seem to get an error code reported. The server does send an error response with diagnostics, but I haven't found a way to access that message. Is there any way?
One way is to change the response type to `std::vector<resp3:node<String>>` so it can store both success and error replies (without discarding the error message), namely * On error vec.front().value will contain the diagnostic error message sent by the server. And vec.front().data_type will be resp3::type::simple_error. * On success vec.front().value will contain the expected value and vec.front().data_type will be resp3::type::blob_string. This is a bit cumbersome but works. There are plans to simplify this, follow this issue for more https://github.com/mzimbres/aedis/issues/40. I know Boost.MySql invented error_info to deal with this problem. I did not adopt it because * Deviates the completion signature from well established practice of having error_code as first parameter f(erro_code ec, ...) or * Requires additional parameters to the completion e.g. f(error_code ec, error_info ei, ...) * Raises question about who allocates the string. * The error information is also only useful for logging and the user can't react to it so why not log it right away.
2. I've modified cpp20_subscriber to add another task that sends commands with async_exec while reading subscription replies. If any of the commands issued with async_exec contain any errors (like the LPOP above), the program crashes with a "conn->cmds_ != 0" assertion.
That is probably a bug in the demultiplexing of events (the most complex thing in Aedis). You shouldn't be able to cause any crash. Tracking it here: https://github.com/mzimbres/aedis/issues/50 and thanks for reporting.
3. I've seen other Redis (async) libraries implementing connection pools. Unless I'm mistaken, it seems likely that some of the user base will eventually require this feature. I see some overlap between the queuing system you have in place in connection and the responsibilities of the connection pool (I'd usually implement this kind of queueing at the connection pool level). What's your view of this?
All libraries I am aware of don't provide event demultiplexing and reconnectable connections, therefore the connection cannot be shared among e.g. individual http or websocket sessions in a server. To circumvent this problem connection pools must be implemented. They add an overhead though * Increased number of connections on the Redis server. * Loses the ability to implement eficient command pipelines, which will increase the number of syscalls: instead of writing all requests once (one syscall), each connection will write its own request (n syscalls). The same problem is also valid for reading. * Increased usage of resources: 1 vs n connection objects. * Increased overall complexity: again 1 vs n connection objects. So I would say Aedis doesn't need connection pools.
a. Is connection pooling planned in the future for Aedis? Or is it considered the responsibility of a higher-level component?
Ditto. Aedis doesn't need them for the reasons stated above.
b. In the latter case, would that component have to make use of the low-level API?
If somebody wants to implement a connection pool it could use the connection object directly I would say. Regards, Marcelo