Remove req from lsn map when committing. #496
Conversation
Signed-off-by: Xiaoxi Chen <[email protected]>
Signed-off-by: Xiaoxi Chen <[email protected]>
|
Did your test shows memory usage not increasing via grafana ? Whats the current usage ? |
|
yes the homestore long running UT shows stable memory usage. Will continue to check on SM level. |
| @@ -758,6 +758,8 @@ void RaftReplDev::handle_commit(repl_req_ptr_t rreq, bool recovery) { | |||
|
|
|||
| // Remove the request from repl_key map. | |||
| m_repl_key_req_map.erase(rreq->rkey()); | |||
| // Remove the request from lsn map. | |||
| m_state_machine->unlink_lsn_to_req(rreq->lsn()); | |||
There was a problem hiding this comment.
if we do this , we will lose the entry cache in leader.
what`s the problem if remove this.
can the reqs in the leader be removed as those in follower when they are expired?
There was a problem hiding this comment.
this is not used anywhere except in end_of_append_batch ATM.
There was a problem hiding this comment.
I think the entry cache is separated into two part - log and data.
Log is on logstore, as discussed in your logstore PR if the cache you add can be extend and cache last N records, regardless of flush.
Data is on the path of RaftReplDev::on_fetch_data_received, right now it just read from drive based on blkid. As #414 stated this approach is wrong after we introduce GC. If the fetch need to go through Index it goes through HO, I think we may have a unified cache in HO which can serve customer IO as well.
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #496 +/- ##
==========================================
+ Coverage 56.51% 56.58% +0.07%
==========================================
Files 108 109 +1
Lines 10300 10515 +215
Branches 1402 1428 +26
==========================================
+ Hits 5821 5950 +129
- Misses 3894 3961 +67
- Partials 585 604 +19 ☔ View full report in Codecov by Sentry. |
No description provided.