-
Notifications
You must be signed in to change notification settings - Fork 59
refactor: move primary's learning preparation of cache into another function #368
Conversation
f561b4f
to
27cba8c
Compare
fb53b9b
to
738e588
Compare
src/replica/replica_learn.cpp
Outdated
count, | ||
response.state.meta.length()); | ||
} | ||
|
||
// learn delta state or checkpoint | ||
// in this case, the state on the PS is still incomplete |
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.
learner_state, | ||
response, | ||
delayed_replay_prepare_list); | ||
if (!should_learn_cache) { | ||
if (learn_start_decree > _app->last_durable_decree()) { |
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.
It seems that the comments are for condition if (learn_start_decree > _app->last_durable_decree())
in line 449, it is better to move comments between line 448 and line 449.
// learn delta state or checkpoint
// in this case, the state on the PS is still incomplete
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 think "delta state" stands for log and cache. So it's still reasonable to see this comment stands above L442. I don't know what's the initial intention of this comment.
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.
Sounds reasonable~ I recommend removing comment // in this case, the state on the PS is still incomplete
which is useless and confusing.
No description provided.