kvrocks icon indicating copy to clipboard operation
kvrocks copied to clipboard

feat: add deleterange command and test

Open baobaomaomeng opened this issue 8 months ago • 8 comments

GH-2628 Added DELETERANGE command to delete keys by pattern,use like scan.The command format translates to English as: DELETERANGE cursor [MATCH pattern] [COUNT count] [TYPE type].The default parameters align with the SCAN command.

baobaomaomeng avatar Mar 20 '25 14:03 baobaomaomeng

Seems that this command is implemented as scan and MDel?

I'm not sure should deleterange supports prefix delete ( which could implemented with the help of rocksdb::DeleteRange ) or a scan and delete script like this? @git-hulk @PragmaTwice @ShooterIT

From my side, I think the prefix deletion could be a part of this command and we should use rocksdb::DeleteRange to avoid the performance issue if possible.

And for the DeleteRange command, I have one question: is it good to support the TYPE option? which might heavily affect the performance since it needs to scan and retrieve the metadata.

git-hulk avatar Mar 21 '25 08:03 git-hulk

Seems that this command is implemented as scan and MDel? I'm not sure should deleterange supports prefix delete ( which could implemented with the help of rocksdb::DeleteRange ) or a scan and delete script like this? @git-hulk @PragmaTwice @ShooterIT

From my side, I think the prefix deletion could be a part of this command and we should use rocksdb::DeleteRange to avoid the performance issue if possible.

And for the DeleteRange command, I have one question: is it good to support the TYPE option? which might heavily affect the performance since it needs to scan and retrieve the metadata.

may be we need two commands,deletepattern and deleterange,the range delete.Handling special operations for range deletion in regular expressions is overly challenging. Additionally, the 'type' is decoded from iter->value(), and I don't quite understand why it requires scanning metadata when the entire implementation seems to be just a combination of scan + mdel.

baobaomaomeng avatar Mar 21 '25 09:03 baobaomaomeng

Additionally, the 'type' is decoded from iter->value(), and I don't quite understand why it requires scanning metadata when the entire implementation seems to be just a combination of scan + mdel.

For example, we have 1,000,000 strings and 100 hash keys with random key ranges. And we might need to scan the entire database if we would like to delete keys by the hash type.

And yes, it makes sense to use SCAN + DEL if we could limit the maximum count on each delete. But perhaps it would be better to use SCAN + rocksdb::DeleteRange if possible.

git-hulk avatar Mar 21 '25 09:03 git-hulk

I don't think SCAN is necessary if we have a good semantics and syntax of this command, e.g. for DELETERANGE <prefix>, we don't need to SCAN, instead we can just call DeleteRange, right?

PragmaTwice avatar Mar 21 '25 10:03 PragmaTwice

e.g. for DELETERANGE , we don't need to SCAN, instead we can just call DeleteRange, right?

Yes, this would work if we don't support the COUNT option.

git-hulk avatar Mar 21 '25 10:03 git-hulk

e.g. for DELETERANGE , we don't need to SCAN, instead we can just call DeleteRange, right?

Yes, this would work if we don't support the COUNT option.

We usually have COUNT to limit the work that the server should do (avoid long-running execution).

torwig avatar Mar 21 '25 10:03 torwig

We usually have COUNT to limit the work that the server should do (avoid long-running execution).

@torwig We could simply use rocksdb::DeleteRange if we don't need to support the COUNT. And it would be fast since it just appends a write batch in rocksdb.

git-hulk avatar Mar 21 '25 11:03 git-hulk

@git-hulk @PragmaTwice @ShooterIT

Now, deleterange is a command with a clear semantic for range deletion. This command can be used in two ways:

1.deleterange begin_key end_key, which deletes key-value pairs in the range [begin_key, end_key). 2.deleterange prefix, which deletes key-value pairs with the specified prefix (the prefix must end with * to indicate it is a prefix).

baobaomaomeng avatar Mar 22 '25 10:03 baobaomaomeng