Support DISK Command

[tanruixiang]

It closes #874

[tanruixiang]

@PragmaTwice Hi. Do you have any more suggestions on reducing redundant code?

[tanruixiang]

@PragmaTwice @git-hulk @torwig I noticed a problem. In the following tests. When we write a value with only one bit(like 'a'). Occasionally, the size of the key we get is 0. It occurs frequently when the value is very small. I guess the reason for this problem is that sync in writeoption defaults to false. Or because the GetApproximateSizes can only yield approximate values, which may be ignored for small values.

  std::vector<int> value_size{1, 1024};
  for(auto &p : value_size){
    EXPECT_TRUE(string->Set(key_, std::string(p, 'a')).ok());
    std::string got_value;
    EXPECT_TRUE(string->Get(key_,  &got_value).ok());
    EXPECT_EQ(got_value, std::string(p, 'a'));
    std::string value;
    string->Get(key_, &value);
    uint64_t result = 0;
    EXPECT_TRUE(disk->GetKeySize(key_, kRedisString, &result).ok());
    EXPECT_GE(result, p);
  }

[PragmaTwice]

I am wondering if this could be narrowed down a bit more. This range currently has a hundred times the gap between the upper and lower bound, which I think is still relatively large. But if we can confirm that the estimation accuracy of rocksdb can only be this at most, it is fine.

[tanruixiang]

I think it is theoretically possible, but we need to accurately calculate the key size constructed by kvrocks as the expected size. Also consider rocksdb's space compression for keys (such as prefix compression). In fact, the test of rocksdb is also a relatively large range. I personally don't think it makes much sense to do these jobs, because as long as there is no problem with the rocksdb we depend on, then there will be no problem with this function, If there is a change in the kvrocks level, the scope now is enough to test the problem. Of course, I can give it a try if you feel the need to.

[PragmaTwice]

I think it is fine if rocksdb also has a large range.

[tanruixiang]

@git-hulk @torwig @caipengbo Hi. Do you have time to review?

[git-hulk]

@git-hulk @torwig @caipengbo Hi. Do you have time to review?

Sure, I have read through the implementation, and overall is good to me. I'm still wondering if we have a more general way to reduce duplicate codes, I will approve this PR soon if can't figure out a better way.

[tanruixiang]

Sure, I have read through the implementation, and overall is good to me. I'm still wondering if we have a more general way to reduce duplicate codes, I will approve this PR soon if can't figure out a better way.

Do you have any ideas? It seems like it should be impossible to reduce each type of custom operation.

[git-hulk]

Sure, I have read through the implementation, and overall is good to me. I'm still wondering if we have a more general way to reduce duplicate codes, I will approve this PR soon if can't figure out a better way.

Do you have any ideas? It seems like it should be impossible to reduce each type of custom operation.

The main issue is GetMetadata requires passing the type, so another way is to allow skipping type check in GetMetadata, but the current implementation is also fine to me. To see others have thoughts on this cc @ShooterIT @PragmaTwice @caipengbo @torwig

[git-hulk]

LGTM

[git-hulk]

@PragmaTwice @caipengbo @torwig Do you have any suggestion on this PR? Or I will merge it if we have no further comment.

[caipengbo]

LGTM

[git-hulk]

Thanks for @tanruixiang contribution and patient, also thanks to everyone who reviewed this PR. Merging...

[caipengbo]

We also need to update our website :) @git-hulk

[git-hulk]

Yes, @caipengbo you can help to update as well when you're free. https://github.com/apache/incubator-kvrocks-website/blob/main/docs/01-supported-commands.md

[caipengbo]

Yes, @caipengbo you can help to update as well when you're free. https://github.com/apache/incubator-kvrocks-website/blob/main/docs/01-supported-commands.md

If I update the repository, will the site automatically update?

[tanruixiang]

We also need to update our website :) @git-hulk

Do you want to do it? If you don't have time, I can update the website later today.

[git-hulk]

Yes, GitHub actions will deploy the update automatically.

[git-hulk]

We also need to update our website :) @git-hulk

Do you want to do it? If you don't have time, I can update the website later today.

Cool, thanks. @tanruixiang

[caipengbo]

Yes, you can do it :) @tanruixiang