baidu / braft

An industrial-grade C++ implementation of RAFT consensus algorithm based on brpc, widely used inside Baidu to build highly-available distributed systems.
Apache License 2.0
3.99k stars 886 forks source link

errno导致raft_meta 判断错误 #319

Open lintanghui opened 3 years ago

lintanghui commented 3 years ago

NodeImpl::init()的时候有多个地方都会导致可能出现errno = 2,包括log_meta 不存在 ,raft_snapshot/tmp 目录不存在。 在 LocalSnapshotStorage::init() 这个函数初始化的时候,https://github.com/baidu/braft/blob/master/src/braft/snapshot.cpp#L464 执行删除tmp目录会导致errno返回2. 同时这个errno不会被清理。 在执行https://github.com/baidu/braft/blob/master/src/braft/raft_meta.cpp#L507 的时候会如果meta问价格式错误返回-1,那么会导致出现误判

 int ret = pb_file.load(&meta); // pb文件格式错误返回 ret =-1
    if (ret == 0) {
        _term = meta.term();
        ret = _votedfor.parse(meta.votedfor());
    } else if (errno == ENOENT) { // 由于之前初始化snapshot的时候errno被设置过这里出出现误判
        ret = 0;
    } else {
        PLOG(ERROR) << "Fail to load meta from " << path;
    }
PFZheng commented 3 years ago

ret != 0 的 case errno 一定会重新设置吧?

lintanghui commented 3 years ago

ret != 0 的 case errno 一定会重新设置吧?

不会的 如果是pb_file.read()返回失败的情况,比如pb文件格式错误的时候,这个时候errno是不会被设置的。只有出现操作系统级别的io错误的时候才会设置errno。 mock raft_meta文件corrupt的时候 https://github.com/baidu/braft/blob/master/src/braft/protobuf_file.cpp#L111 这一行会报错read body fail.返回ret = -1. 但是errno可能在前面的其他逻辑里被设置成了 ENOENT 比如node.init的时候初始化 snapshot. 删除snapshot/tmp 目录不存在会导致errno = ENOENT,这个errno会一直残留到load_meta的地方导致误报

lintanghui commented 3 years ago

一个简单的复现方法

TEST_F(TestUsageSuits, mock_raft_meta_courrupt){

    system("rm -rf stable");
    system("mkdir -p stable");
    braft::FileBasedSingleMetaStorage* storage = new braft::FileBasedSingleMetaStorage("./stable");
    std::string name =   "./stable/raft_meta";
    std::ofstream f;
    f.open(name);
    f << "mock_err_raft_meta";  // 写入脏数据mock raft_meta文件损坏
    f.close();
    errno = 2; // 如果注释掉这一行就会失败
    ASSERT_TRUE(storage->init().ok());
}

在test_meta.cpp 里添加如上单元测试。 预期预期结果应该是解析pb文件失败。但是实际日志

[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from TestUsageSuits
[ RUN      ] TestUsageSuits.mock_raft_meta_courrupt
WARNING: Logging before InitGoogleLogging() is written to STDERR
W0820 16:45:09.617503 16597 protobuf_file.cpp:111] read body failed, path: ./stable/raft_meta
I0820 16:45:09.617626 16597 raft_meta.cpp:520] Loaded single stable meta, path ./stable term 1 votedfor 0.0.0.0:0:0 time: 349
[       OK ] TestUsageSuits.mock_raft_meta_courrupt (7 ms)
[----------] 1 test from TestUsageSuits (7 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (7 ms total)
[  PASSED  ] 1 test.

ret 在这里https://github.com/baidu/braft/blob/master/src/braft/raft_meta.cpp#L511 被设置成0了

PFZheng commented 3 years ago

我理解你说的意思了,protobuf_file 还有一些其他的问题,save 和 load 使用了两种风格在传递错误