sewenew / redis-plus-plus

Redis client written in C++
Apache License 2.0
1.64k stars 351 forks source link

RedisCluster just delete keys in the current master connection #452

Closed joaquinbejar closed 1 year ago

joaquinbejar commented 1 year ago

Using RedisCluster if I want to delete a key in another shard the connection is not moved to the shard owner it just returns false.

RedisCluster->del(key);

Any idea how to follow the redirect command coming from redis master?

thx

sewenew commented 1 year ago

RedisCluster DOES redirect the command automatically.

What do you mean it returns false? RedisCluster::del returns value of long long type, not bool. Do you mean that it returns 0? In that case, it means the key does not exist.

Regards

joaquinbejar commented 1 year ago

Yes, I use it as a bool. It returns 0 because the key doesn't exist on that master of the cluster, but it exists in another shard. In redis-cli if you use DEL "key" and the key is not in the current shard, the client gets a redirect to move to the master, which owns that shard and finally deletes the key. Here, the behaviour is different.

sewenew commented 1 year ago

I cannot reproduce your problem. Which version of redis-plus-plus do you use? You can try the latest code on master branch. If you still have the problem, please show me the minimum code that reproduce your problem.

It returns 0 because the key doesn't exist on that master of the cluster, but it exists in another shard.

NO. It returns 0 if and only if the key doesn't exist in the cluster. It has the same behavior as redis-cli -c.

Regards

joaquinbejar commented 1 year ago

You're right; my issue is not in the DEL command is on the SCAN.

As context: I'm using scan to get all keys in the cluster base on a pattern like tag:* using this method:

    std::vector <std::string> SetRedisClient::get_keys() { // get all keys in the current node, not in cluster
        const long long count = 5000;
        long long cursor = 0;
        const std::string pattern = set_tag(m_config.connection_options->tag, "*");
        std::set <std::string> keys;
        std::vector <std::string> output;
        try {
            auto redis = m_server_read->redis("scan"); // m_server_read is a RedisCluster object in SLAVE mode
            while (true) {
                cursor = redis.scan(cursor, pattern, count, std::inserter(keys, keys.begin()));
                if (cursor == 0) {
                    break;
                }
            }
        } catch (const std::exception &e) {
            this->m_config.logger.log_error("RedisDB GET_KEYS failed: " + std::string(e.what()));
            throw;
        }
        output.reserve(keys.size());
        for (const auto &k: keys) {
            std::string key = k;
            key.erase(0, pattern.length() - 1);
            output.push_back(key);
        }
        return output;
    }

But it seems the scan just gets the keys in the current node. Any idea how to do this? Because there is no scan method in RedisCluster.

Regards

sewenew commented 1 year ago

So far, there's no built-in way to scan all nodes. Instead, you have to get the nodes info, create a Redis object for each node, and send scan command to each node manually.

You can use Cluster Slots command to get nodes info. Check code I posted in this issue for reference.

Regards

joaquinbejar commented 1 year ago

That's what I did; many thanks.

Regards.