add the skip points for KeyRange

[infdahai]

Search before asking

• I had searched in the issues and found no similar issues.

Motivation

Do we need to add a position to ignore the numkey integer because we only support a consistent range? For example, we add a wrong key currently in the watch command in zdiffstore and zinterstore.

Solution

No response

Are you willing to submit a PR?

• I'm willing to submit a PR!

[infdahai]

@PragmaTwice do you think it is necessary?

[torwig]

@infdahai Could you please elaborate on this? It's not clear to me from the Motivation part what issue you want to solve and what solution/solutions you consider/offer.

[infdahai]

I see the function iterates over range.key_step. And it will find the argument[i] to exec the watching.

kvrocks/src/server/server.cc

Lines 1659 to 1670 in 8539ee5

	void Server::updateWatchedKeysFromRange(const std::vector<std::string> &args, const redis::CommandKeyRange &range) {
	std::shared_lock lock(watched_key_mutex_);
	for (size_t i = range.first_key; range.last_key > 0 ? i <= size_t(range.last_key) : i <= args.size() + range.last_key;
	i += range.key_step) {
	if (auto iter = watched_key_map_.find(args[i]); iter != watched_key_map_.end()) {
	for (auto *conn : iter->second) {
	conn->watched_keys_modified = true;
	}
	}
	}
	}

[infdahai]

The unnecssary numkey will be added into the finding. I think it's innocuous because the numkey shouldn't exist in watched_key_map_.

I'll turn this off if you don't think it's necessary

[PragmaTwice]

The current key range design is divided to two methods:

• a static constant [start, end, step] if the key range can be determined without actual commands, e.g. GET key
• a dynamic [start, end, step] according to actual commands, e.g. opname numkeys key... someflag

To generalize the key range design to a more pervasive form, I think we can add the third method:

• a static constant [start, end, step] if the key range can be determined without actual commands, e.g. GET key -> [1,1,1]
• a dynamic [start, end, step] according to actual commands, e.g. opname numkeys key... someflag -> [2, 1+numkeys,1]
• a dynamic key position vector according to actual commands, e.g. opname numkeys1 key1... someflags... numkeys2 key2... someflag -> {2,3,...,1+numkeys1,4+numkeys1,...}

For the skip points idea, I think it seems not as straightforward as just a key position vector.

[infdahai]

BTW, do we use the vector to store CommandKeyRange? like this {{2,1+numkeys,1},{4+numkeys,-1,1}}

I think it will show well.

[PragmaTwice]

BTW, do we use the vector to store CommandKeyRange? like this {{2,1+numkeys,1},{4+numkeys,-1,1}}

I think it will show well.

Looks fine to me.