From 6f1e96d3d0f8353c78ee9caac23f2949d76fd407 Mon Sep 17 00:00:00 2001 From: Nobuhiro Ban Date: Fri, 6 Sep 2024 23:36:09 +0900 Subject: [PATCH 1/2] tglogutil-compaction: quick fix for Ubuntu 24.04 LTS (boost filesystem 1.83) --- src/limestone/dblogutil/dblogutil.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/limestone/dblogutil/dblogutil.cpp b/src/limestone/dblogutil/dblogutil.cpp index c1b0d524..0142e2fe 100644 --- a/src/limestone/dblogutil/dblogutil.cpp +++ b/src/limestone/dblogutil/dblogutil.cpp @@ -164,7 +164,17 @@ void repair(dblog_scan &ds, std::optional epoch) { } static boost::filesystem::path make_tmp_dir_next_to(const boost::filesystem::path& target_dir, const char* suffix) { - auto tmpdirname = boost::filesystem::canonical(target_dir).string() + suffix; + auto canonicalpath = boost::filesystem::canonical(target_dir); + // some versions of boost::filesystem::canonical do not remove trailing directory-separators ('/') + std::string targetdirstring = canonicalpath.string(); + std::size_t prev_len{}; + do { + prev_len = targetdirstring.size(); + canonicalpath.remove_trailing_separator(); // remove only one char + targetdirstring = canonicalpath.string(); + } while (targetdirstring.size() < prev_len); + + auto tmpdirname = targetdirstring + suffix; if (::mkdtemp(tmpdirname.data()) == nullptr) { LOG_LP(ERROR) << "mkdtemp failed, errno = " << errno; throw std::runtime_error("I/O error"); From 1f9521650ebcd739485c8242e60ca16469de1fcd Mon Sep 17 00:00:00 2001 From: Nobuhiro Ban Date: Mon, 16 Sep 2024 22:15:35 +0900 Subject: [PATCH 2/2] tglogutil-compaction: move a function to a separate file, and add test --- src/limestone/dblogutil/dblogutil.cpp | 19 --------- src/limestone/filepath.cpp | 44 +++++++++++++++++++ src/limestone/internal.h | 4 ++ test/limestone/utils/make_tmp_test.cpp | 58 ++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 19 deletions(-) create mode 100644 src/limestone/filepath.cpp create mode 100644 test/limestone/utils/make_tmp_test.cpp diff --git a/src/limestone/dblogutil/dblogutil.cpp b/src/limestone/dblogutil/dblogutil.cpp index 0142e2fe..bcb544de 100644 --- a/src/limestone/dblogutil/dblogutil.cpp +++ b/src/limestone/dblogutil/dblogutil.cpp @@ -163,25 +163,6 @@ void repair(dblog_scan &ds, std::optional epoch) { } } -static boost::filesystem::path make_tmp_dir_next_to(const boost::filesystem::path& target_dir, const char* suffix) { - auto canonicalpath = boost::filesystem::canonical(target_dir); - // some versions of boost::filesystem::canonical do not remove trailing directory-separators ('/') - std::string targetdirstring = canonicalpath.string(); - std::size_t prev_len{}; - do { - prev_len = targetdirstring.size(); - canonicalpath.remove_trailing_separator(); // remove only one char - targetdirstring = canonicalpath.string(); - } while (targetdirstring.size() < prev_len); - - auto tmpdirname = targetdirstring + suffix; - if (::mkdtemp(tmpdirname.data()) == nullptr) { - LOG_LP(ERROR) << "mkdtemp failed, errno = " << errno; - throw std::runtime_error("I/O error"); - } - return {tmpdirname}; -} - static boost::filesystem::path make_work_dir_next_to(const boost::filesystem::path& target_dir) { // assume: already checked existence and is_dir return make_tmp_dir_next_to(target_dir, ".work_XXXXXX"); diff --git a/src/limestone/filepath.cpp b/src/limestone/filepath.cpp new file mode 100644 index 00000000..b4d04fde --- /dev/null +++ b/src/limestone/filepath.cpp @@ -0,0 +1,44 @@ +/* + * Copyright 2024-2024 Project Tsurugi. + * + * 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 "logging_helper.h" + +#include "internal.h" + +namespace limestone::internal { + +boost::filesystem::path make_tmp_dir_next_to(const boost::filesystem::path& target_dir, const char* suffix) { + auto canonicalpath = boost::filesystem::canonical(target_dir); + // some versions of boost::filesystem::canonical do not remove trailing directory-separators ('/') + std::string targetdirstring = canonicalpath.string(); + std::size_t prev_len{}; + do { + prev_len = targetdirstring.size(); + canonicalpath.remove_trailing_separator(); // remove only one char + targetdirstring = canonicalpath.string(); + } while (targetdirstring.size() < prev_len); + + auto tmpdirname = targetdirstring + suffix; + if (::mkdtemp(tmpdirname.data()) == nullptr) { + LOG_LP(ERROR) << "mkdtemp failed, errno = " << errno; + throw std::runtime_error("I/O error"); + } + return {tmpdirname}; +} + +} diff --git a/src/limestone/internal.h b/src/limestone/internal.h index 3de4e6af..1aea9c6d 100644 --- a/src/limestone/internal.h +++ b/src/limestone/internal.h @@ -68,4 +68,8 @@ void create_compact_pwal( int num_worker, const std::set& file_names = std::set()); +// filepath.cpp + +boost::filesystem::path make_tmp_dir_next_to(const boost::filesystem::path& target_dir, const char* suffix); + } diff --git a/test/limestone/utils/make_tmp_test.cpp b/test/limestone/utils/make_tmp_test.cpp new file mode 100644 index 00000000..2cd008c8 --- /dev/null +++ b/test/limestone/utils/make_tmp_test.cpp @@ -0,0 +1,58 @@ + +#include + +#include "dblog_scan.h" +#include "internal.h" +#include "log_entry.h" + +#include "test_root.h" + +namespace limestone::testing { + +using namespace std::literals; +using namespace limestone::api; +using namespace limestone::internal; + +class make_tmp_test : public ::testing::Test { +public: +static constexpr const char* location = "/tmp/make_tmp_test"; + + void SetUp() { + boost::filesystem::remove_all(location); + if (!boost::filesystem::create_directory(location)) { + std::cerr << "cannot make directory" << std::endl; + } + } + + void TearDown() { + boost::filesystem::remove_all(location); + } + + bool starts_with(std::string a, std::string b) { return a.substr(0, b.length()) == b; } + +}; + +TEST_F(make_tmp_test, make_tmp_dir_next_to_0slash) { + std::string p = std::string(location) + "/test0"; + boost::filesystem::create_directory(p); + auto tmp = make_tmp_dir_next_to({p}, ".suffix_XXXXXX"); + ASSERT_TRUE(starts_with(tmp.filename().string(), "test0.suffix_")); +} + +// check removing trailig slashes + +TEST_F(make_tmp_test, make_tmp_dir_next_to_1slash) { + std::string p = std::string(location) + "/test1/"; + boost::filesystem::create_directory(p); + auto tmp = make_tmp_dir_next_to({p}, ".suffix_XXXXXX"); + ASSERT_TRUE(starts_with(tmp.filename().string(), "test1.suffix_")); +} + +TEST_F(make_tmp_test, make_tmp_dir_next_to_2slash) { + std::string p = std::string(location) + "/test2//"; + boost::filesystem::create_directory(p); + auto tmp = make_tmp_dir_next_to({p}, ".suffix_XXXXXX"); + ASSERT_TRUE(starts_with(tmp.filename().string(), "test2.suffix_")); +} + +}