Merge branch 'release-5.4' into release-5.4-cp-4099

pingcap · Apr 28, 2022 · 88c461d · 88c461d
2 parents 0827ad7 + 4eb12de
commit 88c461d
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 35 deletions.
diff --git a/dbms/src/Server/DTTool/DTToolMigrate.cpp b/dbms/src/Server/DTTool/DTToolMigrate.cpp
@@ -176,6 +176,8 @@ int migrateServiceMain(DB::Context & context, const MigrateArgs & args)
         LOG_FMT_INFO(logger, "source version: {}", (src_file->getConfiguration() ? 2 : 1));
         LOG_FMT_INFO(logger, "source bytes: {}", src_file->getBytesOnDisk());
         LOG_FMT_INFO(logger, "migration temporary directory: {}", keeper.migration_temp_dir.path().c_str());
+        LOG_FMT_INFO(logger, "target version: {}", args.version);
+        LOG_FMT_INFO(logger, "target frame size: {}", args.frame);
         DB::DM::DMConfigurationOpt option{};
 
         // if new format is the target, we construct a config file.
@@ -341,30 +343,30 @@ int migrateEntry(const std::vector<std::string> & opts, RaftStoreFFIFunc ffi_fun
         if (args.version == 2)
         {
             args.frame = vm["frame"].as<size_t>();
-            auto algorithm_ = vm["algorithm"].as<std::string>();
-            if (algorithm_ == "xxh3")
+            auto raw_algorithm = vm["algorithm"].as<std::string>();
+            if (raw_algorithm == "xxh3")
             {
                 args.algorithm = DB::ChecksumAlgo::XXH3;
             }
-            else if (algorithm_ == "crc32")
+            else if (raw_algorithm == "crc32")
             {
                 args.algorithm = DB::ChecksumAlgo::CRC32;
             }
-            else if (algorithm_ == "crc64")
+            else if (raw_algorithm == "crc64")
             {
                 args.algorithm = DB::ChecksumAlgo::CRC64;
             }
-            else if (algorithm_ == "city128")
+            else if (raw_algorithm == "city128")
             {
                 args.algorithm = DB::ChecksumAlgo::City128;
             }
-            else if (algorithm_ == "none")
+            else if (raw_algorithm == "none")
             {
                 args.algorithm = DB::ChecksumAlgo::None;
             }
             else
             {
-                std::cerr << "invalid algorithm: " << algorithm_ << std::endl;
+                std::cerr << "invalid algorithm: " << raw_algorithm << std::endl;
                 return -EINVAL;
             }
         }

diff --git a/dbms/src/Server/tests/gtest_dttool.cpp b/dbms/src/Server/tests/gtest_dttool.cpp
@@ -218,20 +218,23 @@ TEST_F(DTToolTest, BlockwiseInvariant)
         stream->readSuffix();
     }
 
-    std::vector<std::tuple<size_t, DB::ChecksumAlgo, DB::CompressionMethod, int>> test_cases{
-        {2, DB::ChecksumAlgo::XXH3, DB::CompressionMethod::LZ4, -1},
-        {1, DB::ChecksumAlgo::XXH3, DB::CompressionMethod::ZSTD, 1},
-        {2, DB::ChecksumAlgo::City128, DB::CompressionMethod::LZ4HC, 0},
-        {2, DB::ChecksumAlgo::CRC64, DB::CompressionMethod::ZSTD, 22},
-        {1, DB::ChecksumAlgo::XXH3, DB::CompressionMethod::NONE, -1}};
-    for (auto [version, algo, comp, level] : test_cases)
+    std::vector<std::tuple<size_t, size_t, DB::ChecksumAlgo, DB::CompressionMethod, int>> test_cases{
+        {2, DBMS_DEFAULT_BUFFER_SIZE, DB::ChecksumAlgo::XXH3, DB::CompressionMethod::LZ4, -1},
+        {1, 64, DB::ChecksumAlgo::XXH3, DB::CompressionMethod::ZSTD, 1},
+        {2, DBMS_DEFAULT_BUFFER_SIZE * 2, DB::ChecksumAlgo::City128, DB::CompressionMethod::LZ4HC, 0},
+        {2, DBMS_DEFAULT_BUFFER_SIZE * 4, DB::ChecksumAlgo::City128, DB::CompressionMethod::LZ4HC, 0},
+        {2, 4, DB::ChecksumAlgo::CRC64, DB::CompressionMethod::ZSTD, 22},
+        {2, 13, DB::ChecksumAlgo::CRC64, DB::CompressionMethod::ZSTD, 22},
+        {2, 5261, DB::ChecksumAlgo::CRC64, DB::CompressionMethod::ZSTD, 22},
+        {1, DBMS_DEFAULT_BUFFER_SIZE, DB::ChecksumAlgo::XXH3, DB::CompressionMethod::NONE, -1}};
+    for (auto [version, frame_size, algo, comp, level] : test_cases)
     {
         auto a = DTTool::Migrate::MigrateArgs{
             .no_keep = false,
             .dry_mode = false,
             .file_id = 1,
             .version = version,
-            .frame = DBMS_DEFAULT_BUFFER_SIZE,
+            .frame = frame_size,
             .algorithm = algo,
             .workdir = getTemporaryPath(),
             .compression_method = comp,
@@ -245,6 +248,10 @@ TEST_F(DTToolTest, BlockwiseInvariant)
             0,
             getTemporaryPath(),
             DB::DM::DMFile::ReadMetaMode::all());
+        if (version == 2)
+        {
+            EXPECT_EQ(refreshed_file->getConfiguration()->getChecksumFrameLength(), frame_size);
+        }
         auto stream = DB::DM::createSimpleBlockInputStream(*db_context, refreshed_file);
         auto size_iter = size_info.begin();
         auto prop_iter = dmfile->getPackProperties().property().begin();

diff --git a/dbms/src/Storages/DeltaMerge/File/DMFile.cpp b/dbms/src/Storages/DeltaMerge/File/DMFile.cpp
@@ -193,43 +193,43 @@ bool DMFile::isColIndexExist(const ColId & col_id) const
     }
 }
 
-const String DMFile::encryptionBasePath() const
+String DMFile::encryptionBasePath() const
 {
     return getPathByStatus(parent_path, file_id, DMFile::Status::READABLE);
 }
 
 
-const EncryptionPath DMFile::encryptionDataPath(const FileNameBase & file_name_base) const
+EncryptionPath DMFile::encryptionDataPath(const FileNameBase & file_name_base) const
 {
     return EncryptionPath(encryptionBasePath(), isSingleFileMode() ? "" : file_name_base + details::DATA_FILE_SUFFIX);
 }
 
-const EncryptionPath DMFile::encryptionIndexPath(const FileNameBase & file_name_base) const
+EncryptionPath DMFile::encryptionIndexPath(const FileNameBase & file_name_base) const
 {
     return EncryptionPath(encryptionBasePath(), isSingleFileMode() ? "" : file_name_base + details::INDEX_FILE_SUFFIX);
 }
 
-const EncryptionPath DMFile::encryptionMarkPath(const FileNameBase & file_name_base) const
+EncryptionPath DMFile::encryptionMarkPath(const FileNameBase & file_name_base) const
 {
     return EncryptionPath(encryptionBasePath(), isSingleFileMode() ? "" : file_name_base + details::MARK_FILE_SUFFIX);
 }
 
-const EncryptionPath DMFile::encryptionMetaPath() const
+EncryptionPath DMFile::encryptionMetaPath() const
 {
     return EncryptionPath(encryptionBasePath(), isSingleFileMode() ? "" : metaFileName());
 }
 
-const EncryptionPath DMFile::encryptionPackStatPath() const
+EncryptionPath DMFile::encryptionPackStatPath() const
 {
     return EncryptionPath(encryptionBasePath(), isSingleFileMode() ? "" : packStatFileName());
 }
 
-const EncryptionPath DMFile::encryptionPackPropertyPath() const
+EncryptionPath DMFile::encryptionPackPropertyPath() const
 {
     return EncryptionPath(encryptionBasePath(), isSingleFileMode() ? "" : packPropertyFileName());
 }
 
-const EncryptionPath DMFile::encryptionConfigurationPath() const
+EncryptionPath DMFile::encryptionConfigurationPath() const
 {
     return EncryptionPath(encryptionBasePath(), isSingleFileMode() ? "" : configurationFileName());
 }
@@ -460,7 +460,7 @@ void DMFile::readPackStat(const FileProviderPtr & file_provider, const MetaPackI
             configuration->getChecksumAlgorithm(),
             configuration->getChecksumFrameLength());
         buf->seek(meta_pack_info.pack_stat_offset);
-        if (sizeof(PackStat) * packs != buf->readBig((char *)pack_stats.data(), sizeof(PackStat) * packs))
+        if (sizeof(PackStat) * packs != buf->readBig(reinterpret_cast<char *>(pack_stats.data()), sizeof(PackStat) * packs))
         {
             throw Exception("Cannot read all data", ErrorCodes::CANNOT_READ_ALL_DATA);
         }
@@ -469,7 +469,7 @@ void DMFile::readPackStat(const FileProviderPtr & file_provider, const MetaPackI
     {
         auto buf = openForRead(file_provider, path, encryptionPackStatPath(), meta_pack_info.pack_stat_size);
         buf.seek(meta_pack_info.pack_stat_offset);
-        if (sizeof(PackStat) * packs != buf.readBig((char *)pack_stats.data(), sizeof(PackStat) * packs))
+        if (sizeof(PackStat) * packs != buf.readBig(reinterpret_cast<char *>(pack_stats.data()), sizeof(PackStat) * packs))
         {
             throw Exception("Cannot read all data", ErrorCodes::CANNOT_READ_ALL_DATA);
         }
@@ -565,8 +565,9 @@ void DMFile::readMetadata(const FileProviderPtr & file_provider, const ReadMetaM
         auto recheck = [&](size_t size) {
             if (this->configuration)
             {
-                auto frame_count = size / this->configuration->getChecksumFrameLength()
-                    + (0 != size % this->configuration->getChecksumFrameLength());
+                auto total_size = this->configuration->getChecksumFrameLength() + this->configuration->getChecksumHeaderLength();
+                auto frame_count = size / total_size
+                    + (0 != size % total_size);
                 size -= frame_count * this->configuration->getChecksumHeaderLength();
             }
             return size;

diff --git a/dbms/src/Storages/DeltaMerge/File/DMFile.h b/dbms/src/Storages/DeltaMerge/File/DMFile.h
@@ -317,14 +317,14 @@ class DMFile : private boost::noncopyable
 
     bool isColIndexExist(const ColId & col_id) const;
 
-    const String encryptionBasePath() const;
-    const EncryptionPath encryptionDataPath(const FileNameBase & file_name_base) const;
-    const EncryptionPath encryptionIndexPath(const FileNameBase & file_name_base) const;
-    const EncryptionPath encryptionMarkPath(const FileNameBase & file_name_base) const;
-    const EncryptionPath encryptionMetaPath() const;
-    const EncryptionPath encryptionPackStatPath() const;
-    const EncryptionPath encryptionPackPropertyPath() const;
-    const EncryptionPath encryptionConfigurationPath() const;
+    String encryptionBasePath() const;
+    EncryptionPath encryptionDataPath(const FileNameBase & file_name_base) const;
+    EncryptionPath encryptionIndexPath(const FileNameBase & file_name_base) const;
+    EncryptionPath encryptionMarkPath(const FileNameBase & file_name_base) const;
+    EncryptionPath encryptionMetaPath() const;
+    EncryptionPath encryptionPackStatPath() const;
+    EncryptionPath encryptionPackPropertyPath() const;
+    EncryptionPath encryptionConfigurationPath() const;
 
     static FileNameBase getFileNameBase(ColId col_id, const IDataType::SubstreamPath & substream = {})
     {