Skip to content
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.

Sign up for GitHub

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

Remove req from lsn map when committing. #496

Merged
merged 3 commits into from
Aug 15, 2024
Merged

Remove req from lsn map when committing. #496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
463926a
Remove req from lsn map when committing.
xiaoxichen Aug 13, 2024
7d84734
Some debugging log
xiaoxichen Aug 14, 2024
29d5bda
Merge branch 'master' into testing
xiaoxichen Aug 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion conanfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

class HomestoreConan(ConanFile):
name = "homestore"
version = "6.4.45"
version = "6.4.46"

homepage = "https://github.com/eBay/Homestore"
description = "HomeStore Storage Engine"
Expand Down
3 changes: 2 additions & 1 deletion src/lib/replication/repl_dev/raft_repl_dev.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we do this , we will lose the entry cache in leader.
what`s the problem if remove this.

HomeStore/src/lib/replication/repl_dev/raft_repl_dev.cpp

Line 1092 in 72a355e

if (rreq->is_proposer()) {

can the reqs in the leader be removed as those in follower when they are expired?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not used anywhere except in end_of_append_batch ATM.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


auto cur_dsn = m_next_dsn.load(std::memory_order_relaxed);
while (cur_dsn <= rreq->dsn()) {
Expand All @@ -775,7 +777,6 @@ void RaftReplDev::handle_commit(repl_req_ptr_t rreq, bool recovery) {
auto prev_lsn = m_commit_upto_lsn.exchange(rreq->lsn());
RD_DBG_ASSERT_GT(rreq->lsn(), prev_lsn, "Out of order commit of lsns, it is not expected in RaftReplDev");
}

if (!rreq->is_proposer()) { rreq->clear(); }
}

Expand Down
2 changes: 2 additions & 0 deletions src/lib/replication/service/raft_repl_service.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,9 +338,11 @@ void RaftReplService::start_reaper_thread() {
m_reaper_fiber = iomanager.iofiber_self();

// Schedule the rdev garbage collector timer
LOGINFOMOD(replication, "Reaper Thread: scheduling GC every {} seconds", HS_DYNAMIC_CONFIG(generic.repl_dev_cleanup_interval_sec));
m_rdev_gc_timer_hdl = iomanager.schedule_thread_timer(
HS_DYNAMIC_CONFIG(generic.repl_dev_cleanup_interval_sec) * 1000 * 1000 * 1000, true /* recurring */,
nullptr, [this](void*) {
LOGINFOMOD(replication, "Reaper Thread: Doing GC");
gc_repl_reqs();
gc_repl_devs();
});
Expand Down
2 changes: 1 addition & 1 deletion src/tests/test_raft_repl_dev.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ class RaftReplDevTest : public testing::Test {

LOGINFO("Run on worker threads to schedule append on repldev for {} Bytes.", block_size);
g_helper->runner().set_task([this, block_size, db]() {
static std::normal_distribution<> num_blks_gen{3.0, 2.0};
static std::normal_distribution<> num_blks_gen{128.0, 0.0};
this->generate_writes(std::abs(std::round(num_blks_gen(g_re))) * block_size, block_size, db);
});
if (wait_for_commit) { g_helper->runner().execute().get(); }
Expand Down
Loading