diff --git a/include/rsync_client.h b/include/rsync_client.h index 657407218..4d6fce140 100644 --- a/include/rsync_client.h +++ b/include/rsync_client.h @@ -112,7 +112,11 @@ class RsyncWriter { public: RsyncWriter(const std::string& filepath) { filepath_ = filepath; - fd_ = open(filepath.c_str(), O_RDWR | O_APPEND | O_CREAT, 0644); + // NOTE: do NOT use O_APPEND here. rsync writes each chunk to an explicit + // offset via pwrite(). O_APPEND would force every write to the end of the + // file, ignoring the offset, which corrupts the file on retry / resumed + // transfer / stale-file scenarios (holes, duplicated or misplaced data). + fd_ = open(filepath.c_str(), O_RDWR | O_CREAT, 0644); } ~RsyncWriter() {} Status Write(uint64_t offset, size_t n, const char* data) { @@ -120,7 +124,7 @@ class RsyncWriter { size_t left = n; Status s; while (left != 0) { - ssize_t done = write(fd_, ptr, left); + ssize_t done = pwrite(fd_, ptr, left, offset); if (done < 0) { if (errno == EINTR) { continue; diff --git a/include/rsync_server.h b/include/rsync_server.h index 560585f3c..7873550c3 100644 --- a/include/rsync_server.h +++ b/include/rsync_server.h @@ -115,6 +115,10 @@ class RsyncReader { memcpy(data, block_data_ + offset_in_block, copy_count); *bytes_read = copy_count; *is_eof = (offset + copy_count == total_size_); + // Compute a per-chunk checksum so the client can detect corruption of this + // response (e.g. a bit flip that slips past the TCP checksum). Previously + // this was left as an empty string, so the client had nothing to verify. + *checksum = pstd::md5(std::string(data, copy_count)); return pstd::Status::OK(); } diff --git a/src/rsync_client.cc b/src/rsync_client.cc index 61fab0e0d..ad90532ac 100644 --- a/src/rsync_client.cc +++ b/src/rsync_client.cc @@ -239,7 +239,22 @@ Status RsyncClient::CopyRemoteFile(const std::string& filename, int index) { return s; } - s = writer->Write((uint64_t)offset, ret_count, resp->file_resp().data().c_str()); + // Verify the per-chunk checksum before writing to disk. Older masters may + // return an empty checksum; in that case we keep backward compatibility by + // skipping verification. A mismatch means this response was corrupted in + // transit, so retry the same offset instead of persisting bad data. + const std::string& expected_checksum = resp->file_resp().checksum(); + if (!expected_checksum.empty()) { + std::string actual_checksum = pstd::md5(std::string(resp->file_resp().data().data(), ret_count)); + if (actual_checksum != expected_checksum) { + LOG(WARNING) << "rsync client checksum mismatch, filename: " << filename + << ", offset: " << offset << ", retry"; + retries++; + continue; + } + } + + s = writer->Write((uint64_t)offset, ret_count, resp->file_resp().data().data()); if (!s.ok()) { LOG(WARNING) << "rsync client write file error"; break;