Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

refactor: introduce mutation_log::replay_block #302

Merged
merged 8 commits into from
Sep 16, 2019

Conversation

neverchanje
Copy link
Contributor

@neverchanje neverchanje commented Aug 27, 2019

mutation_log_utils is a set of utilities for mutation log operations. Some of them are used in the refactoring of mutation_log, but some are not. Leave them here for future usage (for duplication).

mutation_log::replay was used simply for replica recovering, that is, iterating through the whole set of log files and replaying. However, for duplication, the replay of mutation_log becomes a block by block operation. Because it will impose much pressure on this replica to read the entire file once. So we add a new function mutation_log::replay_block. Actually, replay is originally implemented on a block by block behavior, but replaying a block is not taken as a function.

The unit test is committed formerly, see this file.
https://github.com/XiaoMi/rdsn/blob/master/src/dist/replication/test/replica_test/unit_test/mutation_log_test.cpp

mutation_log::replay_block may start from a specified offset, not from the offset point to the last replayed mutation. Therefore we modified mutation_log::reset_stream, to make it able to reset the start_offset.

src/dist/replication/lib/mutation_log.cpp Outdated Show resolved Hide resolved
replay_callback callback,
/*out*/ int64_t &end_offset)
{
end_offset = log->start_offset();
Copy link
Contributor

Choose a reason for hiding this comment

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

end_offset的值是在replay_block里面被修改的,初值设为0更直观一些吧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个是原来的代码,这里是照顾到 code review

Copy link
Contributor

Choose a reason for hiding this comment

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

我看了一下现在replay的用法,其实没有用到end_offset,可以写个todo考虑后续重构这块

Copy link
Contributor Author

Choose a reason for hiding this comment

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

小改动,就没必要写 todo 了

src/dist/replication/lib/mutation_log.cpp Outdated Show resolved Hide resolved
src/dist/replication/lib/mutation_log.cpp Outdated Show resolved Hide resolved
src/dist/replication/lib/mutation_log.cpp Outdated Show resolved Hide resolved
src/dist/replication/lib/mutation_log.cpp Outdated Show resolved Hide resolved
src/dist/replication/lib/mutation_log.h Outdated Show resolved Hide resolved
src/dist/replication/lib/mutation_log.cpp Show resolved Hide resolved
src/dist/replication/lib/mutation_log.h Outdated Show resolved Hide resolved
return err;
}

// TODO(wutao1): move it to mutation_log_replay.cpp
Copy link
Contributor

Choose a reason for hiding this comment

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

为什么要把replay函数都移到mutation_log_replay.cpp中?如果要移,就这个pr移过去吧,现在两个在mutation_log.cpp,两个在mutation_log_replay.cpp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个 PR 重构关键路径,另外两个虽然没有移但已经加了 TODO。移是因为这个代码已经两千行了。

Copy link
Contributor

@hycdong hycdong Sep 12, 2019

Choose a reason for hiding this comment

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

mutation_log.cpp文件有2000多行了,replay_log的代码最多500行,如果是为了精简文件行数,可以后面来拆,可能有更好的精简方法,这个pr就专注重构

Copy link
Contributor Author

Choose a reason for hiding this comment

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

嗯,这里拆成两个文件也是方便对比,我发现直接在原来文件改反而更不容易看

@hycdong
Copy link
Contributor

hycdong commented Sep 12, 2019

我觉得这个pr的描述和代码有点不一致,我理解原来的replay其实就是以block的方式replay log中的mutation的,这个pr只是重构写法,并不是把replay改成以block的方式replay log,没有大改

@neverchanje neverchanje changed the title refactor: reimplement mutation_log::replay in a block by block way refactor: introduce mutation_log::replay_block Sep 12, 2019
@neverchanje
Copy link
Contributor Author

@hycdong 我修改了标题,你可以再看

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants