[FEATURE] update slot mapping when module command returns READONLY response

[bknolinski]

Is your feature request related to a problem? Please describe. We're using redis modules to implement a "write" command that updates data. After a server fails over to the replica, a request to the former master returns "READONLY You can't write against a read only replica." Since this isn't a MOVED or ASK response, the slot mapping does not get updated and requests continue to fail.

Describe the solution you'd like The error handling should be updated for this case.
My suggestion is to add a new ReadOnly error class as shown by the patch code below which is provided the behavior requested:

--- a/src/sw/redis++/shards.h 2024-01-31 09:00:54.000000000 -0600 +++ b/src/sw/redis++/shards.h 2024-06-13 10:28:53.222535347 -0500 @@ -122,6 +122,19 @@ virtual ~SlotUncoveredError() override = default; };

+class ReadOnlyError : public ReplyError { +public:

• explicit ReadOnlyError(const std::string &msg) : ReplyError(msg) {}
• ReadOnlyError(const ReadOnlyError &) = default;
• ReadOnlyError& operator=(const ReadOnlyError &) = default;
• ReadOnlyError(ReadOnlyError &&) = default;
• ReadOnlyError& operator=(ReadOnlyError &&) = default;
• virtual ~ReadOnlyError() override = default; +};

• }

  } diff -Naur a/src/sw/redis++/errors.h b/src/sw/redis++/errors.h --- a/src/sw/redis++/errors.h 2024-01-31 09:00:54.000000000 -0600 +++ b/src/sw/redis++/errors.h 2024-06-13 10:30:48.055026535 -0500 @@ -28,7 +28,8 @@ enum ReplyErrorType { ERR, MOVED,

• ASK
• ASK,

• READONLY };

  class Error : public std::exception { @@ -148,6 +149,8 @@

  class AskError;

+class ReadOnlyError; + void throw_error(const redisContext &context, const std::string &err_info);

void throw_error(const redisReply &reply); diff a/src/sw/redis++/errors.cpp b/src/sw/redis++/errors.cpp --- a/src/sw/redis++/errors.cpp 2024-01-31 09:00:54.000000000 -0600 +++ b/src/sw/redis++/errors.cpp 2024-06-13 10:31:51.353083607 -0500 @@ -29,7 +29,8 @@

std::unordered_map<std::string, ReplyErrorType> error_map = { {"MOVED", ReplyErrorType::MOVED},

• {"ASK", ReplyErrorType::ASK}

• {"ASK", ReplyErrorType::ASK},

• {"READONLY", ReplyErrorType::READONLY} };

  } @@ -98,6 +99,9 @@ case ReplyErrorType::ASK: throw AskError(err_msg);

• case ReplyErrorType::READONLY:

• throw ReadOnlyError(err_msg);

• default: throw ReplyError(err_str); } diff a/src/sw/redis++/redis_cluster.hpp b/src/sw/redis++/redis_cluster.hpp --- a/src/sw/redis++/redis_cluster.hpp 2024-01-31 09:00:54.000000000 -0600 +++ b/src/sw/redis++/redis_cluster.hpp 2024-06-13 10:35:44.474546127 -0500 @@ -1387,6 +1387,9 @@ } catch (const MovedError &) { throw Error("Slot migrating... ASKING node hasn't been set to IMPORTING state"); }

• } catch (const ReadOnlyError &) {

• // command went to replica, update slot mapping and try again.

• _pool->update(); } // For other exceptions, just throw it. }

[sewenew]

Sorry, somehow, I missed this message.

The latest code on master branch updates the underlying slot-node mapping every ClusterOptions::slot_map_refresh_interval time interval (by default, 10 seconds). So I think your problem can be solved by updating to the latest code. Please have a try.

Also the latest code should be more CPU friendly.

Regards

[sewenew]

Since there's no update, I'll close this issue. If you still have problem with it, feel free to reopen it.

Regards