github-actions[bot] commented on code in PR #67204:
URL: https://github.com/apache/doris/pull/67204#discussion_r3870460184
##########
be/src/load/group_commit/wal/wal_file_reader.cpp:
##########
@@ -92,18 +99,34 @@ Status WalFileReader::read_block(PBlock& block) {
if (block_len == 0) {
return Status::DataQualityError("fail to read wal {} ,block is empty",
_file_name);
}
- if (_offset == file_reader->size()) {
- LOG(WARNING) << "need read block with length=" << block_len << ", but
offset=" << _offset
- << " reached end of WAL (path=" << _file_name
- << ", size=" << file_reader->size() << ")";
+ const size_t remaining_bytes = file_size - _offset;
+ if (block_len > remaining_bytes) {
+ LOG(WARNING) << "ignore incomplete wal tail, path=" << _file_name
+ << ", file_size=" << file_size << ", read_offset=" <<
_offset
+ << ", block_bytes=" << block_len << ", available_bytes="
<< remaining_bytes;
return Status::EndOfFile("end of wal file");
}
// read block
std::string block_buf;
block_buf.resize(block_len);
RETURN_IF_ERROR(file_reader->read_at(_offset, {block_buf.c_str(),
block_len}, &bytes_read));
+ if (bytes_read != block_len) {
+ LOG(WARNING) << "ignore incomplete wal tail, path=" << _file_name
+ << ", file_size=" << file_size << ", read_offset=" <<
_offset
+ << ", block_bytes=" << block_len << ", read_block_bytes="
<< bytes_read;
+ return Status::EndOfFile("end of wal file");
+ }
RETURN_IF_ERROR(_deserialize(block, block_buf, block_len, bytes_read));
_offset += block_len;
+ const size_t checksum_bytes = file_size - _offset;
+ if (checksum_bytes < WalWriter::CHECKSUM_SIZE) {
Review Comment:
**[P1] Do not publish a record whose checksum append never completed**
`append_blocks` writes length, payload, and CRC in separate appends, so a
crash—or a partial/error return while writing the CRC—can leave exactly
`block_len` payload bytes plus 0-3 checksum bytes. In that case `write_wal`
never returned success, but this branch still returns the parsed block as `OK`;
replay can commit those unaccepted, unchecked rows and then delete the WAL.
Please treat `block_len + CHECKSUM_SIZE` as the record boundary: return EOF
without publishing when the full CRC is absent, and verify the CRC before
returning the block.
##########
be/src/load/group_commit/wal/wal_file_reader.cpp:
##########
@@ -83,6 +83,13 @@ Status WalFileReader::read_block(PBlock& block) {
if (_offset >= file_reader->size()) {
return Status::EndOfFile("end of wal file");
}
+ const size_t file_size = file_reader->size();
+ if (file_size - _offset < WalWriter::LENGTH_SIZE) {
Review Comment:
**[P1] Handle a torn WAL header at startup too**
This guards partial block framing, but the WAL header has the same crash
boundary: `VWalWriter::init` creates the file before `append_header`, which
writes magic, version, length, and column IDs separately. After a crash on one
of those appends, startup scans the residual file; `read_header` only handles
size 0 and can decode a zero-byte/uninitialized version or length buffer, then
return a non-terminal error that is retried forever. Please bounds-check the
complete header before each fixed-width decode and classify a truncated header
as a discardable incomplete WAL (with prefix-boundary tests).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
