-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: list rename cmd #165
fix: list rename cmd #165
Changes from 1 commit
6f59c35
640b6e1
437dbcc
c07bb77
0b64a3d
73b2e1a
818e5ac
0f26214
c5128c1
5ee3235
d7ffa9b
945a88e
35e553a
c02cbf8
120cc3f
c928b1b
6cabd52
fc2c06f
1ae6c08
7d448ef
3b56a5a
2c422e1
a066ce0
fb55368
0246161
0fc8851
198de69
04ad524
288e6e5
9d485d4
3350a78
dba7663
b026cfc
f68c038
6fc3aa0
a7d01e6
192d741
7a51044
ec794dc
21d6eaf
df0402b
de36c2f
3494ba8
ebd0907
413a2ed
71578e5
3868f52
be90c58
feae1bf
c6aaa7c
887c930
666e1c0
909bbb3
b61d489
97e258b
b9ee3dc
b18402a
b689acf
f7a6473
74cd22c
d7b919c
55496e8
e093dc4
b8d69af
ba010be
d3afca5
5171617
df7f242
880bce1
e229f38
c523f82
ad2acc8
41f7621
408e1b5
7ad81b9
159bbb2
a8608d1
db1b621
8693246
5c39bc0
19af026
8acd3ce
3edd7f8
d14f52f
e2d89b7
81bef3a
c9ceb4a
2538afa
db6655f
faf02fe
002853b
d7840f4
f754323
67d89b4
70d11cb
0db5c0d
e3f865a
2cb7529
0036e68
2aada3b
f0e0fe1
886a3d4
f27731b
c7e0f35
b4f8d9b
9937125
1711028
762f95a
b3a3ef2
3478091
9c7cd3b
4b46b73
63192bd
73442c1
a71e593
ee9c154
4d184dd
63b4620
1a61d7c
15153d5
9cf0798
ba98be6
e627036
9495344
ccfe0a1
ed38403
974e491
b98b67a
3647b6c
3157224
e1b9d88
c160cba
26d5e43
1b8d2d4
b4cdce9
ddf8de6
89ae06f
418fe20
729ebec
6bb67b4
d212a91
377102e
c0e9207
96a752a
79d791d
603ee2d
c59a38f
e83d199
3572109
e5d757b
beec235
658043a
71dd903
d469927
593891e
a779d8d
34af973
86b205f
89c1097
4713c1a
14ea5fe
cb68e12
8cfff76
5cfdb87
0def6b7
6e2654a
91b9a07
3bd4f3e
9d9eaf0
c93d6d1
f018f0a
1d926ca
466c328
222e763
27aaf88
4cf903c
9d703d2
c2642fc
88fea47
349110a
6910a31
9367839
083ce98
59f0af0
3080e0d
55932c9
9f864d6
452726f
fdf9a53
823f9d5
4068391
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -973,6 +973,7 @@ Status Redis::RPushx(const Slice& key, const std::vector<std::string>& values, u | |
} | ||
|
||
Status Redis::ListsRename(const Slice& key, Redis* new_inst, const Slice& newkey) { | ||
auto batch = Batch::CreateBatch(this); | ||
std::string meta_value; | ||
uint32_t statistic = 0; | ||
const std::vector<std::string> keys = {key.ToString(), newkey.ToString()}; | ||
|
@@ -994,18 +995,44 @@ Status Redis::ListsRename(const Slice& key, Redis* new_inst, const Slice& newkey | |
// copy a new list with newkey | ||
ParsedListsMetaValue parsed_lists_meta_value(&meta_value); | ||
statistic = parsed_lists_meta_value.Count(); | ||
s = new_inst->GetDB()->Put(default_write_options_, handles_[kMetaCF], base_meta_newkey.Encode(), meta_value); | ||
|
||
// todo if value is too many, will slow to rename | ||
uint32_t version = parsed_lists_meta_value.Version(); | ||
uint64_t index = parsed_lists_meta_value.LeftIndex() + 1; | ||
uint64_t right_index = parsed_lists_meta_value.RightIndex() - 1; | ||
ListsDataKey base_lists_data_key(key, version, index); | ||
std::vector<std::string> list_nodes; | ||
rocksdb::Iterator* iter = db_->NewIterator(default_read_options_, handles_[kListsDataCF]); | ||
uint64_t current_index = index; | ||
for(iter->Seek(base_lists_data_key.Encode()); iter->Valid() && current_index <= right_index; iter->Next(), current_index++) { | ||
ParsedBaseDataValue parsed_value(iter->value()); | ||
list_nodes.push_back(parsed_value.UserValue().ToString()); | ||
} | ||
delete iter; | ||
|
||
// write new data value | ||
current_index = index; | ||
for (const auto& node : list_nodes) { | ||
ListsDataKey new_lists_data_key(newkey, version, current_index++); | ||
BaseDataValue n_val(node); | ||
batch->Put(kListsDataCF, new_lists_data_key.Encode(), n_val.Encode()); | ||
} | ||
// write new meta_key | ||
batch->Put(kMetaCF, base_meta_newkey.Encode(), meta_value); | ||
new_inst->UpdateSpecificKeyStatistics(DataType::kLists, newkey.ToString(), statistic); | ||
|
||
// ListsDel key | ||
parsed_lists_meta_value.InitialMetaValue(); | ||
s = db_->Put(default_write_options_, handles_[kMetaCF], base_meta_key.Encode(), meta_value); | ||
batch->Delete(kListsDataCF, base_meta_key.Encode()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure atomicity by including old key metadata update in the batch The update to the old key's metadata on line 1027 is performed outside of the batch using Apply this diff to include the metadata update in the batch: - s = db_->Put(default_write_options_, handles_[kMetaCF], base_meta_key.Encode(), meta_value);
+ batch->Put(kMetaCF, base_meta_key.Encode(), meta_value);
🧰 Tools🪛 GitHub Actions: kiwi[error] File has clang-format style issues. The formatting does not match the project's style guidelines. |
||
UpdateSpecificKeyStatistics(DataType::kLists, key.ToString(), statistic); | ||
|
||
return s; | ||
return batch->Commit(); | ||
} | ||
|
||
Status Redis::ListsRenamenx(const Slice& key, Redis* new_inst, const Slice& newkey) { | ||
auto batch = Batch::CreateBatch(this); | ||
|
||
std::string meta_value; | ||
uint32_t statistic = 0; | ||
const std::vector<std::string> keys = {key.ToString(), newkey.ToString()}; | ||
|
@@ -1029,22 +1056,47 @@ Status Redis::ListsRenamenx(const Slice& key, Redis* new_inst, const Slice& newk | |
ParsedListsMetaValue parsed_lists_meta_value(&meta_value); | ||
s = new_inst->GetDB()->Get(default_read_options_, handles_[kMetaCF], base_meta_newkey.Encode(), &new_meta_value); | ||
if (s.ok()) { | ||
if (IsStale(new_meta_value)) { | ||
ParsedListsMetaValue parsed_lists_new_meta_value(new_meta_value); | ||
if (parsed_lists_new_meta_value.Count() != 0 || parsed_lists_new_meta_value.IsStale()) { | ||
return Status::Corruption(); // newkey already exists. | ||
} | ||
} | ||
ParsedSetsMetaValue parsed_lists_new_meta_value(&new_meta_value); | ||
// ParsedListsMetaValue parsed_lists_new_meta_value(&new_meta_value); | ||
// copy a new list with newkey | ||
statistic = parsed_lists_meta_value.Count(); | ||
s = new_inst->GetDB()->Put(default_write_options_, handles_[kMetaCF], base_meta_newkey.Encode(), meta_value); | ||
|
||
// todo if value is too many, will slow to rename | ||
uint32_t version = parsed_lists_meta_value.Version(); | ||
uint64_t index = parsed_lists_meta_value.LeftIndex() + 1; | ||
uint64_t right_index = parsed_lists_meta_value.RightIndex() - 1; | ||
ListsDataKey base_lists_data_key(key, version, index); | ||
std::vector<std::string> list_nodes; | ||
rocksdb::Iterator* iter = db_->NewIterator(default_read_options_, handles_[kListsDataCF]); | ||
uint64_t current_index = index; | ||
for(iter->Seek(base_lists_data_key.Encode()); iter->Valid() && current_index <= right_index; iter->Next(), current_index++) { | ||
ParsedBaseDataValue parsed_value(iter->value()); | ||
list_nodes.push_back(parsed_value.UserValue().ToString()); | ||
} | ||
delete iter; | ||
|
||
// write new data value | ||
current_index = index; | ||
for (const auto& node : list_nodes) { | ||
ListsDataKey new_lists_data_key(newkey, version, current_index++); | ||
BaseDataValue n_val(node); | ||
batch->Put(kListsDataCF, new_lists_data_key.Encode(), n_val.Encode()); | ||
} | ||
// write new meta_key | ||
batch->Put(kMetaCF, base_meta_newkey.Encode(), meta_value); | ||
new_inst->UpdateSpecificKeyStatistics(DataType::kLists, newkey.ToString(), statistic); | ||
|
||
// ListsDel key | ||
parsed_lists_meta_value.InitialMetaValue(); | ||
s = db_->Put(default_write_options_, handles_[kMetaCF], base_meta_key.Encode(), meta_value); | ||
batch->Delete(kListsDataCF, base_meta_key.Encode()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure atomicity by including old key metadata update in the batch Similarly, in the Apply this diff to include the metadata update in the batch: - s = db_->Put(default_write_options_, handles_[kMetaCF], base_meta_key.Encode(), meta_value);
+ batch->Put(kMetaCF, base_meta_key.Encode(), meta_value);
🧰 Tools🪛 GitHub Actions: kiwi[error] File has clang-format style issues. The formatting does not match the project's style guidelines. |
||
UpdateSpecificKeyStatistics(DataType::kLists, key.ToString(), statistic); | ||
|
||
return s; | ||
return batch->Commit(); | ||
} | ||
|
||
void Redis::ScanLists() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Potential performance issues when renaming large lists
The current implementation loads all list nodes into memory by storing them in
list_nodes
, which could cause high memory usage and slow performance when renaming large lists. Consider processing the list data in a streaming fashion or using more efficient methods to handle large datasets.🧰 Tools
🪛 GitHub Actions: kiwi
[error] File has clang-format style issues. The formatting does not match the project's style guidelines.