From 881a80de4d9000779f47f903538523aebe50beba Mon Sep 17 00:00:00 2001 From: Ti Chi Robot Date: Thu, 15 Aug 2024 00:06:07 +0800 Subject: [PATCH] Compression: fix zstd compression level be negative number (#9323) (#9325) close pingcap/tiflash#9322 Compression: fix zstd compression level could be a negative number Signed-off-by: Lloyd-Pottiger Co-authored-by: Lloyd-Pottiger --- .../Compression/CompressionCodecFactory.cpp | 34 +++-- .../Compression/tests/gtest_codec_factory.cpp | 144 ++++++++++++++++++ 2 files changed, 162 insertions(+), 16 deletions(-) create mode 100644 dbms/src/IO/Compression/tests/gtest_codec_factory.cpp diff --git a/dbms/src/IO/Compression/CompressionCodecFactory.cpp b/dbms/src/IO/Compression/CompressionCodecFactory.cpp index 925d838863a..23891fef1ec 100644 --- a/dbms/src/IO/Compression/CompressionCodecFactory.cpp +++ b/dbms/src/IO/Compression/CompressionCodecFactory.cpp @@ -94,6 +94,7 @@ CompressionCodecPtr CompressionCodecFactory::getStaticCodec if (lz4_map.size() >= MAX_LZ4_MAP_SIZE) lz4_map.clear(); auto [it, inserted] = lz4_map.emplace(setting.level, std::make_shared(setting.level)); + assert(inserted); return it->second; } @@ -113,27 +114,28 @@ CompressionCodecPtr CompressionCodecFactory::getStaticCodec= MAX_LZ4HC_MAP_SIZE) lz4hc_map.clear(); auto [it, inserted] = lz4hc_map.emplace(setting.level, std::make_shared(setting.level)); + assert(inserted); return it->second; } template <> CompressionCodecPtr CompressionCodecFactory::getStaticCodec(const CompressionSetting & setting) { - // ZSTD level is in the range [1, 22], the maximum size of the map is 22. - static std::vector zstd_map = { - std::make_shared(1), std::make_shared(2), - std::make_shared(3), std::make_shared(4), - std::make_shared(5), std::make_shared(6), - std::make_shared(7), std::make_shared(8), - std::make_shared(9), std::make_shared(10), - std::make_shared(11), std::make_shared(12), - std::make_shared(13), std::make_shared(14), - std::make_shared(15), std::make_shared(16), - std::make_shared(17), std::make_shared(18), - std::make_shared(19), std::make_shared(20), - std::make_shared(21), std::make_shared(22), - }; - return zstd_map[setting.level - 1]; + static constexpr auto MAX_ZSTD_MAP_SIZE = 10; + static std::shared_mutex zstd_mutex; + static std::unordered_map zstd_map; + { + std::shared_lock lock(zstd_mutex); + auto it = zstd_map.find(setting.level); + if (it != zstd_map.end()) + return it->second; + } + std::unique_lock lock(zstd_mutex); + if (zstd_map.size() >= MAX_ZSTD_MAP_SIZE) + zstd_map.clear(); + auto [it, inserted] = zstd_map.emplace(setting.level, std::make_shared(setting.level)); + assert(inserted); + return it->second; } template <> @@ -234,7 +236,7 @@ Codecs CompressionCodecFactory::createCodecs(const CompressionSettings & setting if (auto codec = create(setting); codec) codecs.push_back(std::move(codec)); } - assert(!codecs.empty()); + RUNTIME_CHECK(!codecs.empty()); return codecs; } diff --git a/dbms/src/IO/Compression/tests/gtest_codec_factory.cpp b/dbms/src/IO/Compression/tests/gtest_codec_factory.cpp new file mode 100644 index 00000000000..c0371193dd7 --- /dev/null +++ b/dbms/src/IO/Compression/tests/gtest_codec_factory.cpp @@ -0,0 +1,144 @@ +// Copyright 2024 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include +#include +#include + +#include + + +namespace DB::tests +{ + +TEST(TestCodecFactory, TestBasic) +try +{ + { + auto codec = CompressionCodecFactory::create( + CompressionSettings(CompressionMethod::LZ4, std::numeric_limits::min())); + ASSERT_TRUE(codec != nullptr); + } + { + auto codec = CompressionCodecFactory::create(CompressionSettings(CompressionMethod::LZ4, -1)); + ASSERT_TRUE(codec != nullptr); + } + { + auto codec = CompressionCodecFactory::create(CompressionSettings(CompressionMethod::LZ4, 0)); + ASSERT_TRUE(codec != nullptr); + } + { + auto codec = CompressionCodecFactory::create(CompressionSettings(CompressionMethod::LZ4, 1)); + ASSERT_TRUE(codec != nullptr); + } + { + auto codec = CompressionCodecFactory::create( + CompressionSettings(CompressionMethod::LZ4, std::numeric_limits::max())); + ASSERT_TRUE(codec != nullptr); + } + { + auto codec = CompressionCodecFactory::create( + CompressionSettings(CompressionMethod::LZ4HC, std::numeric_limits::min())); + ASSERT_TRUE(codec != nullptr); + } + { + auto codec = CompressionCodecFactory::create(CompressionSettings(CompressionMethod::LZ4HC, -1)); + ASSERT_TRUE(codec != nullptr); + } + { + auto codec = CompressionCodecFactory::create(CompressionSettings(CompressionMethod::LZ4HC, 0)); + ASSERT_TRUE(codec != nullptr); + } + { + auto codec = CompressionCodecFactory::create(CompressionSettings(CompressionMethod::LZ4HC, 1)); + ASSERT_TRUE(codec != nullptr); + } + { + auto codec = CompressionCodecFactory::create( + CompressionSettings(CompressionMethod::LZ4HC, std::numeric_limits::max())); + ASSERT_TRUE(codec != nullptr); + } + { + auto codec = CompressionCodecFactory::create( + CompressionSettings(CompressionMethod::ZSTD, std::numeric_limits::min())); + ASSERT_TRUE(codec != nullptr); + } + { + auto codec = CompressionCodecFactory::create(CompressionSettings(CompressionMethod::ZSTD, -1)); + ASSERT_TRUE(codec != nullptr); + } + { + auto codec = CompressionCodecFactory::create(CompressionSettings(CompressionMethod::ZSTD, 0)); + ASSERT_TRUE(codec != nullptr); + } + { + auto codec = CompressionCodecFactory::create(CompressionSettings(CompressionMethod::ZSTD, 1)); + ASSERT_TRUE(codec != nullptr); + } + { + auto codec = CompressionCodecFactory::create( + CompressionSettings(CompressionMethod::ZSTD, std::numeric_limits::max())); + ASSERT_TRUE(codec != nullptr); + } + { + auto codec = CompressionCodecFactory::create(CompressionSettings(CompressionMethod::NONE, 1)); + ASSERT_TRUE(codec != nullptr); + } +#if USE_QPL + { + auto codec = CompressionCodecFactory::create(CompressionSettings(CompressionMethod::DEFLATE_QPL, 1)); + ASSERT_TRUE(codec != nullptr); + } +#endif +} +CATCH + +TEST(TestCodecFactory, TestMultipleCodec) +try +{ + { + std::vector settings{ + CompressionSetting(CompressionMethodByte::FOR), + }; + ASSERT_ANY_THROW(CompressionCodecFactory::create(CompressionSettings(settings))); + } + { + std::vector settings{ + CompressionSetting(CompressionMethodByte::DeltaFOR), + CompressionSetting(CompressionMethodByte::RunLength), + }; + ASSERT_ANY_THROW(CompressionCodecFactory::create(CompressionSettings(settings))); + } + { + std::vector settings{ + CompressionSetting(CompressionMethodByte::DeltaFOR), + CompressionSetting(CompressionMethodByte::LZ4), + }; + auto codec = CompressionCodecFactory::create(CompressionSettings(settings)); + ASSERT_TRUE(codec != nullptr); + ASSERT_TRUE(codec->isCompression()); + } + { + std::vector settings{ + CompressionSetting(CompressionMethodByte::ZSTD), + }; + auto codec = CompressionCodecFactory::create(CompressionSettings(settings)); + ASSERT_TRUE(codec != nullptr); + ASSERT_TRUE(codec->isCompression()); + } +} +CATCH + +} // namespace DB::tests