sewenew/redis-plus-plus

Add function to allow std::variant of supported redis types

chill1n opened this issue · 12 comments

I have a specific problem with simply parsing the output of "memory stats" using the generic command("memory", "stats") call.
I've tried the obvious but it seems complicated with the current functionality.
Would it not be a generic extension to this library to add support for std::variant of supported redis types?
At this moment the current library supports STRING and INTEGER (not BOOL and DOUBLE yet), but that would suffice for most cases. An example output of "memory stats" is given below (which could be mapped to 25 pairs of either <string,long long> or <string,string>, or a single array of std::variant<string,long long>).
Any thoughts on how to approach parsing of "memory stats" with current functionality?
Any thoughts on the proposed support for std::variant?

  1. "peak.allocated"
  2. (integer) 18949144
  3. "total.allocated"
  4. (integer) 865720
  5. "startup.allocated"
  6. (integer) 802984
  7. "replication.backlog"
  8. (integer) 0
  9. "clients.slaves"
  10. (integer) 0
  11. "clients.normal"
  12. (integer) 16986
  13. "aof.buffer"
  14. (integer) 0
  15. "lua.caches"
  16. (integer) 0
  17. "overhead.total"
  18. (integer) 819970
  19. "keys.count"
  20. (integer) 0
  21. "keys.bytes-per-key"
  22. (integer) 0
  23. "dataset.bytes"
  24. (integer) 45750
  25. "dataset.percentage"
  26. "72.924636840820312"
  27. "peak.percentage"
  28. "4.5686497688293457"
  29. "allocator.allocated"
  30. (integer) 994000
  31. "allocator.active"
  32. (integer) 1363968
  33. "allocator.resident"
  34. (integer) 5042176
  35. "allocator-fragmentation.ratio"
  36. "1.3722012042999268"
  37. "allocator-fragmentation.bytes"
  38. (integer) 369968
  39. "allocator-rss.ratio"
  40. "3.6966967582702637"
  41. "allocator-rss.bytes"
  42. (integer) 3678208
  43. "rss-overhead.ratio"
  44. "0.63931763172149658"
  45. "rss-overhead.bytes"
  46. (integer) -1818624
  47. "fragmentation"
  48. "3.9087760448455811"
  49. "fragmentation.bytes"
  50. (integer) 2398856

Would it not be a generic extension to this library to add support for std::variant of supported redis types?

Yes, it's great! I'll put it on the TODO list. Thanks for the advice!

the current library supports STRING and INTEGER (not BOOL and DOUBLE yet)

In fact, it also supports parsing the result to bool (integer reply that only has value 0 and 1) and double (string reply which can be converted to a double).

Any thoughts on how to approach parsing of "memory stats" with current functionality?

So far, you can do it in an ugly way, i.e. define a tuple which matches the output of memory stats:

// It's ugly, and you'd better double check if it matches
using Result = std::tuple<string, long long, string, long long, string, long long, string, long long, string, long long, string, long long, string, long long, string, long long, string, long long, string, long long, string, long long, string, long long, string, double, string, double, string, long long, string, long long, string, long long, string, double, string, long long, string, double, string, long long, string, double, string, long long, string, double, string, long long>;

auto result = redis.command<Result>("memory", "stats");

Thanks again for the proposal! If I have any progress on it, I'll let you know:)

Regards

Thanks for picking this up!

Good to hear that double and bool are also supported.
I appreciate the tuple suggestion. It's a nasty one, but if it does what I want it's good enough for the time being. However, I'm getting the following run-time error:

C++ exception with description "Expect tuple reply with 50elements" thrown in the test body.

Not sure what's wrong, as type/size mismatches seem relatively hard to debug. The data (size and types) seems to be correct in a gdb session. Another workaround that I've implemented in the mean time is through a Python wrapper function that returns a dictionary from which I can select and return the desired value(s). This call is not performance critical, so it's good enough.

C++ exception with description "Expect tuple reply with 50elements" thrown in the test body.

That means the number of elements in the reply is NOT 50. You'd better use redis-cli to check the exact number and format of the memory stats command. My redis server (5.0.5) returns 52 elements, and one of which is a map<string, long long>:

17) "db.0"
18) 1) "overhead.hashtable.main"
    2) (integer) 12048
    3) "overhead.hashtable.expires"
    4) (integer) 0

Regards

Also I've tried to implement parsing reply to variant. The code is on dev branch, and you can try it:

        using MyVar = Variant<long long, double>;
        auto r = Redis("tcp://127.0.0.1");
        auto v = r.command<unordered_map<string, MyVar>>("memory", "stats");
        for (const auto &ele : v) {
            cout << ele.first << "\t";
            visit([](auto &&arg) -> void { cout << arg << endl; }, ele.second);
        }

However, as I mentioned in previous comment, you need to check out the format of your redis server's output. If the result contains a unordered_map<string, long long>, you need to define MyVar as follows, and overload operator<< for the map.

using MyVar = Variant<long long, double, unordered_map<string, long long>>;

If you still have any problem, feel free to let me know.

Regards

C++ exception with description "Expect tuple reply with 50elements" thrown in the test body.

That means the number of elements in the reply is NOT 50. You'd better use redis-cli to check the exact number and format of the memory stats command. My redis server (5.0.5) returns 52 elements, and one of which is a map<string, long long>:

17) "db.0"
18) 1) "overhead.hashtable.main"
    2) (integer) 12048
    3) "overhead.hashtable.expires"
    4) (integer) 0

Regards

Interesting... I can reproduce your case with 52 response elements.
However, my initial post was based on actual copy&paste data too. So I found out that after a "flushall" command redis will return with 50 response elements.
This makes parsing this response a bit more tricky.
Would it be possible to have a std::variant type with a fallback type if mapping fails, e.g. nullptr? So in this case it could be a std::variant<string, long long, nullptr_t>

Any thoughts on this?

Also I've tried to implement parsing reply to variant. The code is on dev branch, and you can try it:

        using MyVar = Variant<long long, double>;
        auto r = Redis("tcp://127.0.0.1");
        auto v = r.command<unordered_map<string, MyVar>>("memory", "stats");
        for (const auto &ele : v) {
            cout << ele.first << "\t";
            visit([](auto &&arg) -> void { cout << arg << endl; }, ele.second);
        }

However, as I mentioned in previous comment, you need to check out the format of your redis server's output. If the result contains a unordered_map<string, long long>, you need to define MyVar as follows, and overload operator<< for the map.

using MyVar = Variant<long long, double, unordered_map<string, long long>>;

If you still have any problem, feel free to let me know.

Regards

Great! Will try asap.
Any thoughts about my previous proposal to ignore a mapping if the type does not match (and we don't care about that type)?

My testing shows that the implemented Variant functionality in the dev branch works as requested. Thanks a lot for the quick implementation!
Any thoughts on the "don't care" nullptr_t special case would still be appreciated though. It's a use case that matches well with std::variant. Moreover, it could improve performance and code complexity on the user side.

By the way, a more compact way to print a variant is;
get<0>(v)

So I found out that after a "flushall" command redis will return with 50 response elements.

If there’s a db that is not empty, the command will show some stat on the db. That’s why you got 50 elements if you call flushall, and got 52 elements if you set some data to redis.

Any thoughts about my previous proposal to ignore a mapping if the type does not match (and we don't care about that type)?

Thanks for the suggestion! However, instead of nullptr_t, I’m thinking about using std::monostate as a placeholder for unknown types. That might be a c++ idiom. Still need some research. Thanks again!

Regards

std::variant is also new to me, as is std::monostate. Indeed std::monostate seems to be a more generic approach towards "don't care" types. Plus, you get the functionality for free with variant.

@chill1n I refactor the code and add monostate support. The latest code has been merged into master branch. You can take a try :)

Regards

Thanks for the quick resolution. I've tested the master branch and can confirm that variant with monostate is working as expected. I will close this issue.