feat(tiering): Faster small bins serialization #3340
Conversation
dranikpg
force-pushed
the
tiering-offload-opt
branch
2 times, most recently
from
549f0a0 to
150256b
Compare
Signed-off-by: Vladislav Oleshko <[email protected]>
| void FlushDelayed(bool force) { | ||
| // Flush pages with most records accumulated first, or all, if forced. | ||
| // It's enough just to issue reads, because they are collapsed by the tiered storage internally | ||
| while ((force && !delayed_.empty()) || delayed_.size() > kMaxPageAccum) { |
There was a problem hiding this comment.
We need to choose some kind of limit here... by the number of pages? by the total sum of enties? 🤷🏻♂️ We have to take into account that once we flush all of those, we can get a memory spike
| } | ||
|
|
||
| // Flush tiered delayed entries to avoid reordering with journal | ||
| tiered_serializer_->FlushDelayed(true); | ||
| } |
There was a problem hiding this comment.
Ideally we pass the value with ChangeReq& and find out whether it was tiered, only if it's tiered and part of a relevant page, we should flush it
| int64_t memory_margin_ = 0; | ||
| std::vector<tiering::DiskSegment> delayed_deletes_; |
There was a problem hiding this comment.
todo: better naming
dranikpg
changed the title
|
@romange Please look only at the idea, I left some open questions |
| : db_slice_(slice), dest_(dest), compression_mode_(compression_mode) { | ||
| : db_slice_(slice), | ||
| dest_(dest), | ||
| tiered_serializer_(new TieredSerializer{}), |
There was a problem hiding this comment.
do you want to initialize it unconditionally?
There was a problem hiding this comment.
🤷🏻♂️ I don't think it's expensive
| auto entries = delayed_.extract(page); | ||
| for (auto& [base, value] : entries.mapped()) { | ||
| DCHECK(value.IsExternal()); | ||
| pending_.push_back(Read(std::move(base), value)); |
There was a problem hiding this comment.
what guarantess that by the time you flush delayed, their segment are still correct?
maybe the pages were freed and repurposed for other values?
There was a problem hiding this comment.
Inside tiered storage, I don't delete small bin pages while we're serializing
| delayed_sizes_.insert({entries.size(), page}); | ||
| } | ||
| } else { | ||
| pending_.push_back(Read(std::move(base), pv)); |
|
I thought about this more. Master is the more fragile piece compared to slave or an instance loading the snapshot. |
|
Closed in favour of #3396 |
No description provided.