This is an automated email from the ASF dual-hosted git repository.

yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/master by this push:
     new d2b199955a [bugfix](deserialize ) pack struct to avoid parse wrong 
content for file header (#21907)
d2b199955a is described below

commit d2b199955a460df40bd010e504d2d826c21a70a6
Author: AlexYue <yj976240...@gmail.com>
AuthorDate: Tue Jul 18 22:32:41 2023 +0800

    [bugfix](deserialize ) pack struct to avoid parse wrong content for file 
header (#21907)
    
    Recently we encountered one strange bug where the log is file length is not 
match. 
file=/mnt/hdd01/master/NO_AVX2/doris.HDD/snapshot/20230713122303.26.72000/45832/536215111/45832.hdr,
 file_length=, real_file_length=0 when running restore P2 case, after checking 
the file on the remote storage we doubt it's the local file deserialize who 
caused this situation.
    Then we analyzed the layout for the struct and the content of the hdr file 
then we found out that it must be the wrong layout which cause reading wrong 
content.
---
 be/src/olap/file_header.h | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/be/src/olap/file_header.h b/be/src/olap/file_header.h
index c764031205..ef31bba23f 100644
--- a/be/src/olap/file_header.h
+++ b/be/src/olap/file_header.h
@@ -44,7 +44,7 @@ using FixedFileHeader = struct _FixedFileHeader {
     uint32_t protobuf_length;
     // Checksum of Protobuf part
     uint32_t protobuf_checksum;
-};
+} __attribute__((packed));
 
 using FixedFileHeaderV2 = struct _FixedFileHeaderV2 {
     uint64_t magic_number;
@@ -57,7 +57,7 @@ using FixedFileHeaderV2 = struct _FixedFileHeaderV2 {
     uint64_t protobuf_length;
     // Checksum of Protobuf part
     uint32_t protobuf_checksum;
-};
+} __attribute__((packed));
 
 template <typename MessageType, typename ExtraType = uint32_t>
 class FileHeader {
@@ -158,6 +158,9 @@ Status FileHeader<MessageType, ExtraType>::deserialize() {
     size_t bytes_read = 0;
     RETURN_IF_ERROR(file_reader->read_at(
             0, {(const uint8_t*)&_fixed_file_header, _fixed_file_header_size}, 
&bytes_read));
+    DCHECK(_fixed_file_header_size == bytes_read)
+            << " deserialize read bytes dismatch, request bytes " << 
_fixed_file_header_size
+            << " actual read " << bytes_read;
 
     //Status read_at(size_t offset, Slice result, size_t* bytes_read,
     //             const IOContext* io_ctx = nullptr);
@@ -167,6 +170,9 @@ Status FileHeader<MessageType, ExtraType>::deserialize() {
         FixedFileHeader tmp_header;
         RETURN_IF_ERROR(file_reader->read_at(0, {(const uint8_t*)&tmp_header, 
sizeof(tmp_header)},
                                              &bytes_read));
+        DCHECK(sizeof(tmp_header) == bytes_read)
+                << " deserialize read bytes dismatch, request bytes " << 
sizeof(tmp_header)
+                << " actual read " << bytes_read;
         _fixed_file_header.file_length = tmp_header.file_length;
         _fixed_file_header.checksum = tmp_header.checksum;
         _fixed_file_header.protobuf_length = tmp_header.protobuf_length;
@@ -200,7 +206,7 @@ Status FileHeader<MessageType, ExtraType>::deserialize() {
 
     if (file_length() != static_cast<uint64_t>(real_file_length)) {
         return Status::Error<ErrorCode::FILE_DATA_ERROR>(
-                "file length is not match. file={}, file_length=, 
real_file_length={}",
+                "file length is not match. file={}, file_length={}, 
real_file_length={}",
                 file_reader->path().native(), file_length(), real_file_length);
     }
 
@@ -209,9 +215,13 @@ Status FileHeader<MessageType, ExtraType>::deserialize() {
             olap_adler32(olap_adler32_init(), buf.get(), 
_fixed_file_header.protobuf_length);
 
     if (real_protobuf_checksum != _fixed_file_header.protobuf_checksum) {
+        // When compiling using gcc there woule be error like:
+        // Cannot bind packed field '_FixedFileHeaderV2::protobuf_checksum' to 
'unsigned int&'
+        // so we need to using unary operator+ to evaluate one value to pass
+        // to status to successfully compile.
         return Status::Error<ErrorCode::CHECKSUM_ERROR>(
                 "checksum is not match. file={}, expect={}, actual={}",
-                file_reader->path().native(), 
_fixed_file_header.protobuf_checksum,
+                file_reader->path().native(), 
+_fixed_file_header.protobuf_checksum,
                 real_protobuf_checksum);
     }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to