- Boost - lists.preview.boost.org

Fwd: Aedis Review
by Klemens Morgenstern 24 Jan '23

24 Jan '23

This review was sent directly to me. ---------- Forwarded message --------- From: Robert A.H. Leahy <rleahy(a)rleahy.ca> Date: Tue, Jan 24, 2023 at 1:04 PM Subject: Aedis Review To: klemensdavidmorgenstern(a)gmail.com <klemensdavidmorgenstern(a)gmail.com> Sending directly as I'm not on the Boost mailing list. *Knowledge of Redis* No more than can be gathered by Wikipedia. *Does this library bring real benefit to C++ developers for real world use-case?* Yes. Having an idiomatic (i.e. Asio-like) way to interact with something in C++ brings real benefit. *Do you have an application for this library?* Not immediately but Redis is in use within my company so I can see one arising in the future. *Does the API match with current best practices?* I share Zach Laine's concerns about raw strings (status quo) vs. a more structured way to pass data to the API. However I'm sympathetic to arguments that: - Redis-like products may extend the set of commands and that's important to support - Redis itself may extend the set of commands and it'd be a shame to force users to wait for a library upgrade (or worse a storm of forks) to get support for new features Additionally I see the request for a more structured way to pass data to the API as a pure extension: What exists right now is a good lower layer that something higher level can be built on top of. It's not one or the other, in a perfect world both would exist. I think the API should be more generic, for example: - Why is std::tuple used rather than supporting anything which supports the tuple protocol? - std::pmr::string is used internally by aedis::request but the documentation suggests that for deserialization purposes only std::string is supported (along with only std::vector et cetera) - Why the concrete reliance on aedis::resp3::request (rather than any type which obeys some protocol)? I don't think that aedis::basic_connection::reset_stream should exist: It marries the class too heavily to TCP-like streams and doesn't add anything beyond being a convenience wrapper for several functions of the underlying stream. If anything it should be a free function. Note that the above concern is mirrored somewhat in several parts of the code where members such as is_open are used. Several opportunities to add noexcept (conditional or otherwise) should be taken. I don't see any reason why detail::connection_base::get_executor shouldn't be noexcept, for example. I feel as though read and write buffers should be customizable rather than being married to std::pmr::string (detail/connection_base.hpp). If the implementation is going to maintain its own buffers I think there should be a way to inspect them for post mortems (e.g. generating clear and context-rich error messages). *Is the documentation helpful and clear?* Yes, although some things could be made clearer. It was unclear to me at first glance what it mean to "[r]eset[] the underlying stream" (documentation for aedis::basic_connection::reset_stream), for example. *Did you try to use it?* I did not. *What is your evaluation of the implementation?* I'd rather see operator[] (possibly combined with an assert) rather than at(). I dislike the AEDIS_CHECK_OP# macros (by virtue of the fact that they're macros and, in my opinion, substantially reduce the readability of the code at the point where they're used). In detail/connection_ops.hpp:271 the operation appears to complete sucessfully because stopping "was requested from the outside." My first instinct is that in this situation the operation should end with operation_canceled. *Please explicitly state that you either accept or reject the inclusion of this library into Boost* Accept. I don't think any of the concerns I've raised are breaking and so they can be addressed later. Thanks, --Robert

1 0

Fwd: aedis review starts today
by Ruben Perez 23 Jan '23

23 Jan '23

Forwarding to the whole ML, I hit the wrong button. Thanks Marcelo for noting it. ---------- Forwarded message --------- From: Ruben Perez <rubenperez038(a)gmail.com> Date: Mon, 23 Jan 2023 at 14:04 Subject: Re: [boost] aedis review starts today To: Marcelo Zimbres Silva <mzimbres(a)gmail.com> Hi Marcelo, I'll try to answer to the several points you made in a single email. > > 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. I do think there is a place in Boost for a library like this. I don't think we need neither a 100% type-safe solution, nor to completely hide the general runtime interface that request::push() gives us. But I think we do have some usability barrier here, and you need to have a plan on how to evolve it to make it better, before the library gets into Boost and you loose the ability to introduce big breaking changes. I don't think the type-safe solution should span *all* Redis commands, but only the most common ones. If the command the user wants does not have type-safety implemented, she can always use what it's in place now. Simple interfaces for simple uses, complex ones for advanced use cases (that's something I learnt in MySQL the bad way). > > 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 avoid this you can wrap the response type in std::optional, for example > > std::optional<std::map<std::string, std::string>> > > I don't lock users in this behaviour because they might not want it. > Perhaps it is desirable to have an error instead of having to check > whether the optional has a value. The user must know what he needs. Hand't think of that. Although I don't have data to speak, I get the impression that this feature will be used unintentionally more than intentionally. But it might just be an opinion. > As an alternative I could lock-in users on resp3 types: maps, sets, > etc. But then how to represent data structures that have no > corresponding C++ types. It would also be inefficient as it would > require first copying and then converting. That's something I didn't take into account when I wrote the review, and that gets solving this harder. Is there a way, given a type T, to know whether the response to a given command (say "GET") is compatible with T? Without performing the actual parsing? I can think of the following solutions (excuse me if any of these have flaws I haven't taken into account): 1) A runtime checking option. For the most common commands, you provide a set of helper functions like request::push_set or request::push_hgetall(). request remembers which commands have been added. The deserializing function checks for type compatibility before proceeding to the actual deserialization. This would pick the std::optional<T> vs T mismatch. Not sure if it's worth paying the price, though. You could provide an option to disable this at runtime. 2) A compile-time checking option. I envision this as something like typed_request<commands::get, commands::hgetall, ...>. You can check for compatibility at compile-time. You will probably want something like commands::any to provide access to commands that don't have a type-safe interface. Note that I don't think this should replace aedis::request, but complement it. Do you find any of these solutions appealing? Knowing that a strategy is in-place to evolve the library towards type-safety would alleviate many of my concerns. > I am trying to understand how you avoid these risks in Boost.MySql but > it seems it suffers from the same problems? > > I mean this: https://github.com/boostorg/mysql/blob/ac7285c62163c0de40a466ecaeca13f7de5a… > > conn.query("SELECT salary FROM employee WHERE first_name = > 'Underpaid'", result); > double salary = result.rows().at(0).at(0).as_double(); > > The query is not type safe [1] and the result can mismatch the the > query. Why would it be considered ok for Boost.MySql and not > Boost.Aedis? > > Disclaimer: I am not trying to make a case or open a precedent, it is > purely technical. I might be wrong but I would like to clarify anyway. So we have two parts here. The "client -> server" interface has two flavours: connection::query(string_view), and statement::execute(const ParamsTuple&). query() should only be used for queries that don't have parameters provided by the user (I should change that example, BTW). statement::execute takes a tuple of parameters. The client will check for compatibility in terms of number of parameters. Types are not checked in the client because the server is *very* flexible with type conversions. The second part is the "server -> client" interface (mysql::resultset), which is the one I think you're referring to here. It's a purely variant-like interface. Still, there is code to runtime-check that what the server sent matches the query metadata, which is provided by the server. I view this variant-like interface similar to Boost.JSON parsing into a json::value. I'd say the differences I see from the Aedis case are: * There is no way for the MySQL client to know the resulting data structure from a query given the contents of a query. "SELECT product FROM products" can yield 10 different data types, and without the table definition, you can't know. I got the impression this is not the case in Redis - "HGETALL" always returns the same data structure (please correct me if I'm wrong). * From a user perspective, when I use a variant-like interface (like the one in Boost.JSON or in Python database access), my head is set to "type checking is my responsibility". I know I'm in runtime. When I use a compile-time interface, I get much more relaxed. I got the "I'm at compile-time safety" with Aedis, but that impression is not right. Again, this is just based on what I felt while using the library for the first time, other users might feel differently. I think if I had encountered an aedis::value, similar to json::value, I would have switched my mind to "I'm at runtime". I'm not saying this is the solution, I'm just trying to explain myself. * I still got criticism for this in MySQL, and I've got a safer compile-time interface planned (https://github.com/boostorg/mysql/issues/60) The idea is that you define a row type T, so you can then read a typed resultset<T> (or individual rows by calling connection::read_one_row(T&)). My idea is to fully check for type compatibility as soon as I've got the query metadata available - that would be similar to solution 1) I proposed you earlier in this email. * If you're referring to the ability to compose SQL queries in a type-safe way, as sqlpp11 does, I did determine this as out of scope. As SQL is a standard, you usually get this at the database compatibility layer, rather than in individual drivers. I see it as more involved than providing type-safety to Redis commands - please apologies if this statement is not correct. > > This is to make it safe by default and in part because Aedis is > centered around the adapter. In the same sense that you can write an > adapter to receive responses in the data-structure that makes most > sense to the application e.g. std::map, std::unordered_map, > boost::flat_map etc. You can also customize the way errors are > handled. For example, > > * Errors like type mismatch are usually unrecoverable and you might > want to fail fast and let the async_ operation itself fail instead of > proceeding in an unknown state. > > * If you can tolerate errors like that you can write an adapter that > does not communicate an error to the parser. I did not make this the > default because it looks unsafe, people will usually expect > async_exec(req, adapt(resp)) to throw when the response can't be > stored in resp. > > I am perhaps at fault here for not documenting this feature. I thought > however not many people would want this behaviour. I think you're right and that this is the most common case. I had a look at the Python redis driver, and they do the following: * By default, fail the operation if any of the pipeline requests failed. * If you pass a certain config option, individual errors are reported as error response objects. So if you sent [C1, C2, C3] and C2 failed, you will get [C1_response, C2_error_info, C3_response]. I think your default is right. Maybe you could provide the option to report errors individually by having the user provide a) std::tuple<leaf::result<T1>, leaf::result<T2>> Instead of b) std::tuple<T1, T2> If the user did b), then do the same you're currently doing, otherwise report errors individually. I would like to know your thoughts on these two matters (type-safety and error handling), as having a plan would alleviate many of my concerns. Regards, Ruben.

2 1

Regarding password protected file
by Daggumati, Rajesh 23 Jan '23

23 Jan '23

Hi Team, We have a project requirement where we store the logs in .csv file and that .csv file should be password protected. It means that User can enter the password to open the .csv file. do we have any library to create a password to open a .csv file in boost? Please let me know your thoughts on above query. Regards, Rajesh D Confidential - Company Proprietary Please be advised that this email may contain confidential information. If you are not the intended recipient, please notify us by email by replying to the sender and delete this message. The sender disclaims that the content of this email constitutes an offer to enter into, or the acceptance of, any agreement; provided that the foregoing does not invalidate the binding effect of any digital or other electronic reproduction of a manual signature that is included in any attachment.

3 3

boost.aedis ongoing review reminder
by Klemens Morgenstern 20 Jan '23

20 Jan '23

The review period for boost.aedis is half way done, please consider submitting a review!

1 0

Re: [boost] aedis review starts today
by Zach Laine 18 Jan '23

18 Jan '23

As Marcelo pointed out, I did not Reply All; here is the email: On Tue, Jan 17, 2023 at 3:40 PM Zach Laine <whatwasthataddress(a)gmail.com> wrote: > > On Tue, Jan 17, 2023 at 3:11 PM Marcelo Zimbres Silva > <mzimbres(a)gmail.com> wrote: > > > > On Tue, 17 Jan 2023 at 20:39, Zach Laine via Boost > > <boost(a)lists.boost.org> wrote: > > > > > > No. In particular, the API requires the user to write lots of Redis > > > strings by hand. A typo will not be detected until runtime. The > > > set of Redis keywords is fixed; there's no reason not to make an > > > enumeration of them all, like this: > > > > I had enums in the past but I had to remove them for a couple of reasons > > > > 1. There are a multitude of possible commands in the Redis stack, for example > > > > * Redis search: https://redis.io/commands/?group=search > > * Redis graph: https://redis.io/commands/?group=graph > > * Redis time series: https://redis.io/commands/?group=timeseries > > * Redis json: https://redis.io/commands/?group=json > > * Perhaps more I might be missing righ now. > > I don't yet see the problem. In the enumeration I made, there's a > JSON_DEBUG (JSON.DEBUG from https://redis.io/commands/?group=json, but > with s/./_/), etc. None of these enumerators collides with any other; > I compiled the resulting enumeration to check exactly this. Am I > missing something? > > > Then, we would probably agree that the command syntax is equally > > important. To check it properly I would have to write a generator that > > parses the description of each command and generates a checker, see > > https://github.com/redis/redis/tree/unstable/src/commands. > > > > I have made no thoughts so far about how that would work and if I get > > that correctly I would still have to watch for versioning (what > > changes from release to release), what new commands were implemented > > and which were deprecated. > > Yep. That's the job. The good news is that a new version need not be > that disruptive. You only ever need to add to the enumeration, and > you can add a single deprecation-detection function that compares any > enumerator used against the list of enumerators that are deprecated. > > > That might be enough for Redis, but them I would be keeping other > > Redis-like databases off, for example > > > > * https://github.com/Snapchat/KeyDB > > * https://github.com/scylladb/scylladb/blob/master/docs/dev/redis.md > > * https://github.com/dragonflydb/dragonfly > > * Perhaps more. > > > > I don't know how compatible they are with the Redis API. I can't > > afford working on this right now and in the foreseeable future, I see > > it almost as a second library of a module of my library e.g. > > aedis::redis::search::request or something. > > Ok, this is something different. If you don't want to break support > for those other DBs, that's understandable. But since you don't have > support for them at the moment, I don't think possibly adding support > for those later (which could also be added in much the same way) > should slow down adoption of a better interface for Redis today. > > Zach

1 0

Implemented support for recvmmsg / sendmmsg (on supported operating systems: Linux / *BSD / AIX / QNX)
by Virgilio Fornazin 17 Jan '23

17 Jan '23

As stated on asio-users mailing list ( https://sourceforge.net/p/asio/mailman/asio-users/thread/CADfydx%2BDF1kvqch… ), I wrote a patch that enables sendmmsg / recvmmsg on supported operating systems (Linux, [Free/Net/Open]BDS, AIX, Blackberry QNXNeutrino). The syscalls are implemented in reactive_socket_service backend (the io_uring backend on Linux does not support sendmmsg / recvmmsg yet, so no recvmmsg / sendmmsg when using newest support for io_uring on asio). The config.hpp has macros for forcible disable those calls, and detect supported operating system compiler macros. New methods were developed for (send/send_to/receive/receive_from) suffixed with '_multiple_buffer_sequence' that accepts a multiple_buffer_sequence class (backed by asio::detail::array<multiple_buffer_sequence_op, N> or std::vector<multiple_buffer_sequence_op>) and a multiple_buffer_sequence_adapter that convert ops to struct mmsghdr* for sendmmsg/recvmmsg. The multiple_buffer_sequence implementations are basically a collection of multiple_buffer_sequence_op instances that represents each socket operation . It defines the objects needed to perform fill on mmsghdr structures buffer_sequence_type buffer_sequence_; buffer_sequence_adapter_type buffer_sequence_adapter_; endpoint_type endpoint_; bool completed_; socket_base::message_flags flags_; std::size_t bytes_transferred_; asio::error_code error_code_; The multiple_buffer_sequence implementations has full container support, iterators, and support Mutable and Const buffer sequences. I finished the first version of the patch. It's able to send and receive 1million packet / second on my box at same time (an intel i9 9900k / ubuntu 22.04 lts low latency kernel) using packet sizes of 64 / 1430 bytes without packet loss (using 64mb udp socket buffers) A little running demo is able to being watched at https://www.youtube.com/watch?v=Q3hqe6ntM3w. I need to write some automatic tests for (async_)[send/receive]_multiple_buffer_sequence calls[_to/_from] and push a pure ASIO example in examples repository I made the code available for review, scrutiny and suggestions. The source code is in my github repo ( https://github.com/virgiliofornazin/asio ),on branch feature/multiple_datagram_buffers_sequence-io-sendmmsg-recvmmsg ( the full URL of patch diff is https://github.com/chriskohlhoff/asio/compare/master...virgiliofornazin:asi… ).

2 7

Regression Results Not Updating
by Tom Kent 15 Jan '23

15 Jan '23

The results from the pages at: https://www.boost.org/development/tests/develop/developer/summary.html https://www.boost.org/development/tests/master/developer/summary.html Have not updated in almost a month. Can someone check on the process that manages this? Thanks, Tom

4 10

delayed_forward: good, bad, ugly?
by Hans Dembinski 12 Jan '23

12 Jan '23

Dear all, a user noticed that some classes in Boost.Histogram do not provide a strong exception guarantee, although with a few changes they could. I never promised that guarantee for those classes, but I am inclined to support it. To that end, I am considering use a type called delayed_forward to the public interface of that class. I would like to get a mini-review on the concept. The change is backward-compatible and it offers some advantages, so it seems like a good solution to me, but maybe I am missing some caveats. The constructor of my class, let’s call it Foo, accepts some potentially non-trivial parameters by value, because I didn’t want to write an interface with a templated forward reference. Passing non-trivial parameters by value is cheap if the compiler can do RVO or if the user calls the move constructor via std::move. Not using a forward reference saves a few template instantiations and thus reduces compile time. I think I got that from Herb Sutter in a GotW entry, but I cannot find it right now. With delayed_forward one can efficiently pass values to Foo with a single ctor. The trade-off is a tiny runtime cost, but that’s ok if we know that Foos are not created en mass in a hot loop. That already guided my choice of passing by value. A minimal demo is here: https://godbolt.org/z/z71E1jno1 The design of delayed_forward is so simple that it probably already exists somewhere, likely under another name. Maybe even in Boost. What do you think? Hans

5 13

Upcoming aedis review
by Klemens Morgenstern 10 Jan '23

10 Jan '23

The aedis review will start in less than a week, on the 15th and last until the 24th of this month. Aedis is a Redis client library built on top of Boost.Asio that implements the latest version of the Redis communication protocol RESP3. Some of its distinctive features are * A connection class that provides high-level functions to execute Redis commands and receive server pushes concurrently. * Automatic pipelining of commands. * Support for STL containers and serialization of user-defined data types. * Low-latency: Responses can be read in their final data-structure without creating copies. Aedis links Github: https://github.com/mzimbres/aedis Documentation: https://mzimbres.github.io/aedis/ Git tag: v1.4.1 Tarball: https://github.com/mzimbres/aedis/archive/refs/tags/v1.4.1.tar.gz Redis Links: Redis: https://redis.io/ RESP3: https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md Pipelines: https://redis.io/docs/manual/pipelining/ Redis commands: https://redis.io/commands/ Running Redis: Redis can be also easily installed on your local machine, for example, on Debian run "apt install redis". You might also want to use docker https://www.docker.com/blog/how-to-use-the-redis-docker-official-image/ Please consider writing a review next week.

1 0

CMake documentation (mostly) available
by Peter Dimov 07 Jan '23

07 Jan '23

I've written a document that explains how to use CMake to build or consume Boost, at https://github.com/boostorg/cmake/blob/develop/README.md This targets users of Boost. Developer-centric documentation is also available at https://github.com/boostorg/cmake/blob/develop/developer.md This is still unfinished, but I decided to "publish" it because the portions that are present are still (hopefully) valuable.

1 0