疑似bug确认

[promoon]

fsm_caller.cpp 306行，
_last_applied_index.store(committed_index, butil::memory_order_release);
这一步为什么设置_last_applied_index为committed_index，而不是last_index。

正常情况下两者应该都是相等的，但是如果iter_impl出现error的情况下呢？

[promoon]

另外请问什么情况下会导致iter_impl出现error

[killme2008]

我尝试回答下：

1. 问题 1：有没有 bug？
   iter_impl 出现 error 的时候，会调用 set_error ，阻止后续状态机的执行：

    if (iter_impl.has_error()) {
        set_error(iter_impl.error());
        iter_impl.run_the_rest_closure_with_error();
    }

do_commit 的时候会判断 fsm_caller 当前是否处于 error 状态：

void FSMCaller::do_committed(int64_t committed_index) {
    if (!_error.status().ok()) {
        return;
    }

FSMCaller 的 set_error 还会通知到 state machine 的 on_error 方法，表示一个不可恢复的错误发生，需要人工介入处理。整个状态机将停止，因此不会有后续的数据安全问题。并且需要强调 _last_applied_index 是内存状态， do_commit 最终存储的还是上一次的 index:

    int64_t last_applied_index = _last_applied_index.load(
                                        butil::memory_order_relaxed);
     .....
    _log_manager->set_applied_id(last_applied_id);

因此并不存在数据安全问题。

2. 什么情况下会导致 iter_impl 出现 error ？

用户在 apply task 到状态机的时候可能遇到不可控的异常（比如网络或者文件磁盘沾满等 IO 问题），这种时候用户可以主动终止状态机继续执行，调用 set_error_and_rollback 方法回滚，人工介入处理，待错误恢复后继续执行状态机。

[promoon]

我尝试回答下：

1. 问题 1：有没有 bug？
   iter_impl 出现 error 的时候，会调用 set_error ，阻止后续状态机的执行：

    if (iter_impl.has_error()) {
        set_error(iter_impl.error());
        iter_impl.run_the_rest_closure_with_error();
    }

do_commit 的时候会判断 fsm_caller 当前是否处于 error 状态：

void FSMCaller::do_committed(int64_t committed_index) {
    if (!_error.status().ok()) {
        return;
    }

FSMCaller 的 set_error 还会通知到 state machine 的 on_error 方法，表示一个不可恢复的错误发生，需要人工介入处理。整个状态机将停止，因此不会有后续的数据安全问题。并且需要强调 _last_applied_index 是内存状态， do_commit 最终存储的还是上一次的 index:

    int64_t last_applied_index = _last_applied_index.load(
                                        butil::memory_order_relaxed);
     .....
    _log_manager->set_applied_id(last_applied_id);

因此并不存在数据安全问题。

1. 什么情况下会导致 iter_impl 出现 error ？

用户在 apply task 到状态机的时候可能遇到不可控的异常（比如网络或者文件磁盘沾满等 IO 问题），这种时候用户可以主动终止状态机继续执行，调用 set_error_and_rollback 方法回滚，人工介入处理，待错误恢复后继续执行状态机。

感谢回复。
如你所说，do_commit时候在前面有个error状态判断，确实可以阻断后续的日志commit。
另外我看了下 _last_applied_index 在do_snapshot_save和do_snapshot_load中会用到，这两个函数调用前有个pass_by_status来阻断error状态，应该是没有问题的。

bool FSMCaller::pass_by_status(Closure* done) {
    brpc::ClosureGuard done_guard(done);
    if (!_error.status().ok()) {
        if (done) {
            done->status().set_error(
                        EINVAL, "FSMCaller is in bad status=`%s'",
                                _error.status().error_cstr());
        }
        return false;
    }
    done_guard.release();
    return true;
}

但是如果从语义理解上，在error情况下，committed_index并不一定等于last_index，那意味着committed_index的entry的term也不一定等于last_index对应entry的term；而 _last_applied_index和_last_applied_term我理解是对应同一个entry的，如果是这样的话，这里用_last_applied_index设为last_index是否更加合适。

    const int64_t last_index = iter_impl.index() - 1;
    const int64_t last_term = _log_manager->get_term(last_index);
    LogId last_applied_id(last_index, last_term);
    _last_applied_index.store(committed_index, butil::memory_order_release);
    _last_applied_term = last_term;
    _log_manager->set_applied_id(last_applied_id);

[killme2008]

@promoon 是设置为 last_index，我看错了，注意这行：

 LogId last_applied_id(last_index, last_term);

取的是当前 iterator 的 index，set_error_and_rollback 会重新计算 current_index 的。

[promoon]

@killme2008
其实我比较纠结于306行中的这个committed_index，虽然应该不会出现bug，但是从语义上来讲改为last_index感觉更合理一些🤔：

_last_applied_index.store(committed_index, butil::memory_order_release);

[killme2008]

@promoon 了解了，error 情况下，用 last_index 确实更合适，虽然不影响正确性，可以等官方来回答下。