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.

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

feat: support user-defined log truncation index #2

Merged
merged 2 commits into from
Apr 27, 2024

Conversation

panlei-coder
Copy link
Collaborator

目前考虑的关于一般情况下日志截断的推进方式是:动态设置产生多少次flush后,就去调用snapshot接口截断日志。braft自己的定时快照我们可以设置成0,关闭掉它,只保留我们主动触发。
(1)在snapshot的接口函数中添加了一个self_snapshot_index的参数,默认值为0,上层应用可以根据自己的需求去设置这个self_snapshot_index参数,如果self_snapshot_index的的值为0,则会按照braft原本的逻辑进行日志截断并生成快照,如果是大于0的值,则会按照指定的index进行日志截断。(on_snapshot_save是一个空操作,只会创建快照目录,不会生成具体的快照数据)
(2)上层应用可以根据需要动态的设置FLAGS_raft_do_snapshot_min_index_gap值,即两次日志截断的log index的最小间隔,这里提供了set_raft_do_snapshot_min_index_gap接口,pikiwidb需要实现相关的命令来支持。
(3)测试:目前跑了一下braft/example/counter示例,能正常的生成快照文件。

@@ -175,7 +184,7 @@ void SnapshotExecutor::do_snapshot(Closure* done) {
}
_saving_snapshot = true;
SaveSnapshotDone* snapshot_save_done = new SaveSnapshotDone(this, writer, done);
if (_fsm_caller->on_snapshot_save(snapshot_save_done) != 0) {
if (_fsm_caller->on_snapshot_save(snapshot_save_done, self_snapshot_index) != 0) {
Copy link

Choose a reason for hiding this comment

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

on_snapshot_save收到self_snapshot_index的值好像没有使用场景?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

_fsm_caller->on_snapshot_save是将生成快照封装成一个ApplyTask传入到Apply队列中,等待去应用
int FSMCaller::on_snapshot_save(SaveSnapshotClosure* done, int64_t self_snapshot_index) {
ApplyTask task;
task.type = SNAPSHOT_SAVE;
task.self_snapshot_index = self_snapshot_index;
task.done = done;
return bthread::execution_queue_execute(_queue_id, task);
}

Copy link

Choose a reason for hiding this comment

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

懂了。是同名函数。

saved_fsm_applied_index = self_snapshot_index;
}
{
std::lock_guard<std::mutex> lock(raft_snapshot_gap_mutex);
Copy link

Choose a reason for hiding this comment

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

这个gap只是一个整形,不需要使用一个lock进行保护,即便偶尔读到较旧的值也不怎么影响结果。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

好的

@@ -967,15 +970,33 @@ butil::Status NodeImpl::reset_peers(const Configuration& new_peers) {
return butil::Status::OK();
}

void NodeImpl::snapshot(Closure* done) {
do_snapshot(done);
butil::Status NodeImpl::set_raft_do_snapshot_min_index_gap(int32_t raft_do_snapshot_min_index_gap) {
Copy link

Choose a reason for hiding this comment

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

BRPC_VALIDATE_GFLAG这个宏应该就可以用来校验,不需要自己实现一个函数,并且不需要使用mutex。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

set_raft_do_snapshot_min_index_gap这个接口不是用来给pikiwidb自己动态设置gap的大小么,校验有BRPC_VALIDATE_GFLAG这个的话那应该不需要了

Copy link

Choose a reason for hiding this comment

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

这里定义的那个Flags应该都是全局可见的,应该可以直接修改。braft还有很多参数可以修改,我们看下是都直接去修改还是怎么着。为每个都做一个接口好像也不现实。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

行,那这个先不加

return butil::Status();
}

void NodeImpl::snapshot(Closure* done, int64_t self_snapshot_index) {
Copy link

Choose a reason for hiding this comment

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

这里要不直接这做判断,直接让值穿透进去,判断条件就是> 0使用我们自己设置的值, <=0使用原来的逻辑。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

其实我感觉可以用重载函数,原函数一点不用变。重载的带log index 的函数,也不用做if判断。

@@ -150,6 +150,7 @@ friend class IteratorImpl;

struct ApplyTask {
TaskType type;
int64_t self_snapshot_index = 0; // Customize log truncation points
Copy link
Collaborator

Choose a reason for hiding this comment

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

可以考虑把这个值在 0 和非 0 下的表现写成注释放在上面?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

@longfar-ncy
Copy link
Collaborator

目前考虑的关于一般情况下日志截断的推进方式是:动态设置产生多少次flush后,就去调用snapshot接口截断日志。braft自己的定时快照我们可以设置成0,关闭掉它,只保留我们主动触发。 (1)在snapshot的接口函数中添加了一个self_snapshot_index的参数,默认值为0,上层应用可以根据自己的需求去设置这个self_snapshot_index参数,如果self_snapshot_index的的值为0,则会按照braft原本的逻辑进行日志截断并生成快照,如果是大于0的值,则会按照指定的index进行日志截断。(on_snapshot_save是一个空操作,只会创建快照目录,不会生成具体的快照数据) (2)上层应用可以根据需要动态的设置FLAGS_raft_do_snapshot_min_index_gap值,即两次日志截断的log index的最小间隔,这里提供了set_raft_do_snapshot_min_index_gap接口,pikiwidb需要实现相关的命令来支持。 (3)测试:目前跑了一下braft/example/counter示例,能正常的生成快照文件。

尝试在example/counter上运行时,是采用默认传值0,还是尝试传入自定义的截断日志索引?

@panlei-coder
Copy link
Collaborator Author

目前考虑的关于一般情况下日志截断的推进方式是:动态设置产生多少次flush后,就去调用snapshot接口截断日志。braft自己的定时快照我们可以设置成0,关闭掉它,只保留我们主动触发。 (1)在snapshot的接口函数中添加了一个self_snapshot_index的参数,默认值为0,上层应用可以根据自己的需求去设置这个self_snapshot_index参数,如果self_snapshot_index的的值为0,则会按照braft原本的逻辑进行日志截断并生成快照,如果是大于0的值,则会按照指定的index进行日志截断。(on_snapshot_save是一个空操作,只会创建快照目录,不会生成具体的快照数据) (2)上层应用可以根据需要动态的设置FLAGS_raft_do_snapshot_min_index_gap值,即两次日志截断的log index的最小间隔,这里提供了set_raft_do_snapshot_min_index_gap接口,pikiwidb需要实现相关的命令来支持。 (3)测试:目前跑了一下braft/example/counter示例,能正常的生成快照文件。

尝试在example/counter上运行时,是采用默认传值0,还是尝试传入自定义的截断日志索引?

目前是默认值,自定义的话可能需要我们那边整体差不多了才行,不然这块感觉不太好测

@AlexStocks AlexStocks merged commit 8d217f5 into pikiwidb:master Apr 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants