-
Notifications
You must be signed in to change notification settings - Fork 15
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
Issue: 109: Checkpointing for HomeObject #115
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #115 +/- ##
==========================================
- Coverage 78.06% 77.88% -0.18%
==========================================
Files 29 30 +1
Lines 1217 1266 +49
Branches 127 130 +3
==========================================
+ Hits 950 986 +36
- Misses 199 208 +9
- Partials 68 72 +4 ☔ View full report in Codecov by Sentry. |
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.
@yamingk
1 we already have a timer for flushing pg super block periodically, if we have CP for homeobject, we could remove that timer.
2 the dirty list can not guarantee that it will be flushed to disk(crash before trigger_cp_flush), so we need a separate log to make sure all the updates will be recovered when restart. if we use cp for homeobject, we should implement that log, right?
@JacksonYao287 | >>>>> 2 the dirty list can not guarantee that it will be flushed to disk(crash before trigger_cp_flush), so we need a separate log to make sure all the updates will be recovered when restart. if we use cp for homeobject, we should implement that log, right? |
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.
Minor comment, just for knowledge purpose, LG otherwise
replica_set_uuid = rhs.replica_set_uuid; | ||
index_table_uuid = rhs.index_table_uuid; | ||
blob_sequence_num = rhs.blob_sequence_num; | ||
memcpy(members, rhs.members, sizeof(pg_members) * num_members); |
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.
is this safe to copy sizeof(pg_members) * num_members
bytes to members
, which can hold only one pg_members
?
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.
The idea behind the original author is that members
is always come from the heap, and its pointed size is always the size of num_members.
cache_pg_sb_ = (pg_info_superblk*)malloc(pg_sb_->size()); | ||
cache_pg_sb_->copy(*(pg_sb_.get())); |
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.
i prefer to remove copy
, and overwrite operator new for pg_info_superblk
. i think it will be more readable
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.
LG!
Changes.
Testing: