From 4eb12de1b5d80b8366b58495308413c1923da50d Mon Sep 17 00:00:00 2001 From: Ti Chi Robot Date: Thu, 28 Apr 2022 16:10:53 +0800 Subject: [PATCH] fix: ut and more idx error (#4777) (#4790) close pingcap/tiflash#4778 --- dbms/src/Server/DTTool/DTToolMigrate.cpp | 16 +++++++------ dbms/src/Server/tests/gtest_dttool.cpp | 23 +++++++++++------- dbms/src/Storages/DeltaMerge/File/DMFile.cpp | 25 ++++++++++---------- dbms/src/Storages/DeltaMerge/File/DMFile.h | 16 ++++++------- 4 files changed, 45 insertions(+), 35 deletions(-) diff --git a/dbms/src/Server/DTTool/DTToolMigrate.cpp b/dbms/src/Server/DTTool/DTToolMigrate.cpp index 28c9586807f..c1d0a8c6597 100644 --- 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 & opts, RaftStoreFFIFunc ffi_fun if (args.version == 2) { args.frame = vm["frame"].as(); - auto algorithm_ = vm["algorithm"].as(); - if (algorithm_ == "xxh3") + auto raw_algorithm = vm["algorithm"].as(); + 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 index bb1e2c1daa9..f209896f043 100644 --- 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> 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> 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 index aefb018a68e..ac0d08df531 100644 --- 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(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(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 index 16479b7bb15..4db79e0e2b8 100644 --- 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 = {}) {