curvefs/metaserver: recover s3ChunkInfoRemove field for GetOrModifyS3… #1404
Conversation
| } | ||
|
|
||
| auto list2del = item.second; | ||
| S3ChunkInfoList dummy; |
There was a problem hiding this comment.
it's wired that you add a empty list, even now it's correct because ModifyInodeS3ChunkInfoList will just return ok if list is empty.
There was a problem hiding this comment.
it's wired that you add a empty list, even now it's correct because
ModifyInodeS3ChunkInfoListwill just return ok if list is empty.
yep, i will use pointer to improve it.
| std::unordered_map<uint64_t, bool> deleted; | ||
| for (const auto& item : map2add) { | ||
| uint64_t chunkIndex = item.first; | ||
| auto list2add = item.second; |
There was a problem hiding this comment.
avoid meaningless copy, const auto& list2add
There was a problem hiding this comment.
avoid meaningless copy,
const auto& list2add
yep, i will improve it.
| for (const auto& item : map2add) { | ||
| uint64_t chunkIndex = item.first; | ||
| auto list2add = item.second; | ||
| S3ChunkInfoList list2del; |
There was a problem hiding this comment.
ditto, use a pointer
const S3ChunkInfoList* list2del = nullptr;
line 333: list2der = &iter->second;
after line 341: line2del = nullptr;There was a problem hiding this comment.
ditto, use a pointer
const S3ChunkInfoList* list2del = nullptr; line 333: list2der = &iter->second; after line 341: line2del = nullptr;
fixed.
| continue; | ||
| } | ||
|
|
||
| auto list2del = item.second; |
| if (rc != MetaStatusCode::OK) { | ||
| return rc; | ||
| } | ||
| std::unordered_map<uint64_t, bool> deleted; |
There was a problem hiding this comment.
no need to use a map in here, a set is enough, and just store the chunk index that has been deleted.
There was a problem hiding this comment.
no need to use a map in here, a set is enough, and just store the chunk index that has been deleted.
yep, i will improve it.
| bool succ = SplitS3ChunkInfoList( | ||
| iterator->Value(), delFirstChunkId, &list); |
There was a problem hiding this comment.
why pass an empty list?
There was a problem hiding this comment.
why pass an empty list?
fixed.
|
|
||
| for (const auto& item : map2del) { | ||
| uint64_t chunkIndex = item.first; | ||
| if (deleted[chunkIndex]) { |
There was a problem hiding this comment.
deleted[chunkIndex] will add an element (key:chunkIndex, value:false)to map, which it's strange there...
using deleted.count(chunkIndex) instead
There was a problem hiding this comment.
deleted[chunkIndex] will add an element (key:chunkIndex, value:false)to map, which it's strange there... using deleted.count(chunkIndex) instead
yep, i will improve it.
| std::unordered_map<uint64_t, bool> deleted; | ||
| for (const auto& item : map2add) { | ||
| uint64_t chunkIndex = item.first; | ||
| auto list2add = item.second; |
There was a problem hiding this comment.
const auto& ?
fixed.
| continue; | ||
| } | ||
|
|
||
| auto list2del = item.second; |
There was a problem hiding this comment.
const auto&?
fixed, i will use pointer to avoid memory copying.
| bool returnS3ChunkInfoMap, | ||
| bool compaction); | ||
| std::shared_ptr<Iterator>* iterator); |
There was a problem hiding this comment.
"iterator" should be more meaningful name. it's too generic to understand
There was a problem hiding this comment.
"iterator" should be more meaningful name. it's too generic to understand
fixed.
| // firstChunkId < minChunkId | ||
| key2del.push_back(skey); | ||
| *size4del += key.size; | ||
| // delete list range : [ ] |
There was a problem hiding this comment.
logic need change maybe
I adjusted the logic according based on our discussion.
|
recheck |
What problem does this PR solve?
Issue Number: #1304, #1308
Problem Summary: