diff --git a/contrib/client-c b/contrib/client-c index ccc3e3b9671..0be225561ef 160000 --- a/contrib/client-c +++ b/contrib/client-c @@ -1 +1 @@ -Subproject commit ccc3e3b967148a3544294d43d49ba1b6516ceaa1 +Subproject commit 0be225561ef79e675d947c82a718ebe76d473dc1 diff --git a/dbms/src/Storages/Page/V3/Remote/RemoteDataLocation.h b/dbms/src/Storages/Page/V3/Remote/RemoteDataLocation.h index dcd806b845c..cb0b1b0e4e5 100644 --- a/dbms/src/Storages/Page/V3/Remote/RemoteDataLocation.h +++ b/dbms/src/Storages/Page/V3/Remote/RemoteDataLocation.h @@ -1,3 +1,17 @@ +// Copyright 2023 PingCAP, Ltd. +// +// 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 namespace DB::Remote diff --git a/dbms/src/Storages/S3/S3GCManager.cpp b/dbms/src/Storages/S3/S3GCManager.cpp index 0ac2e37f89a..271a529aeb2 100644 --- a/dbms/src/Storages/S3/S3GCManager.cpp +++ b/dbms/src/Storages/S3/S3GCManager.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -49,10 +50,12 @@ namespace DB::S3 S3GCManager::S3GCManager( pingcap::pd::ClientPtr pd_client_, std::shared_ptr client_, + OwnerManagerPtr gc_owner_manager_, S3LockClientPtr lock_client_, S3GCConfig config_) : pd_client(std::move(pd_client_)) , client(std::move(client_)) + , gc_owner_manager(std::move(gc_owner_manager_)) , lock_client(std::move(lock_client_)) , shutdown_called(false) , config(config_) @@ -77,8 +80,7 @@ bool S3GCManager::runOnAllStores() { // Only the GC Manager node run the GC logic // TODO: keep a pointer of OwnerManager and check it here - bool is_gc_owner = true; - if (!is_gc_owner) + if (bool is_gc_owner = gc_owner_manager->isOwner(); !is_gc_owner) { return false; } @@ -437,12 +439,13 @@ String S3GCManager::getTemporaryDownloadFile(String s3_key) S3GCManagerService::S3GCManagerService( Context & context, pingcap::pd::ClientPtr pd_client, + OwnerManagerPtr gc_owner_manager_, S3LockClientPtr lock_client, const S3GCConfig & config) : global_ctx(context.getGlobalContext()) { auto s3_client = S3::ClientFactory::instance().createWithBucket(); - manager = std::make_unique(std::move(pd_client), std::move(s3_client), std::move(lock_client), config); + manager = std::make_unique(std::move(pd_client), std::move(s3_client), std::move(gc_owner_manager_), std::move(lock_client), config); timer = global_ctx.getBackgroundPool().addTask( [this]() { diff --git a/dbms/src/Storages/S3/S3GCManager.h b/dbms/src/Storages/S3/S3GCManager.h index 5400abc65c4..ca209a2b0ed 100644 --- a/dbms/src/Storages/S3/S3GCManager.h +++ b/dbms/src/Storages/S3/S3GCManager.h @@ -45,6 +45,8 @@ namespace DB class Context; class Logger; using LoggerPtr = std::shared_ptr; +class OwnerManager; +using OwnerManagerPtr = std::shared_ptr; } // namespace DB namespace DB::S3 @@ -76,6 +78,7 @@ class S3GCManager explicit S3GCManager( pingcap::pd::ClientPtr pd_client_, std::shared_ptr client_, + OwnerManagerPtr gc_owner_manager_, S3LockClientPtr lock_client_, S3GCConfig config_); @@ -122,6 +125,7 @@ class S3GCManager const std::shared_ptr client; + const OwnerManagerPtr gc_owner_manager; const S3LockClientPtr lock_client; std::atomic shutdown_called; @@ -137,6 +141,7 @@ class S3GCManagerService explicit S3GCManagerService( Context & context, pingcap::pd::ClientPtr pd_client, + OwnerManagerPtr gc_owner_manager_, S3LockClientPtr lock_client, const S3GCConfig & config); diff --git a/dbms/src/Storages/S3/tests/gtest_s3gcmanager.cpp b/dbms/src/Storages/S3/tests/gtest_s3gcmanager.cpp index f0a697605d2..813e05b5641 100644 --- a/dbms/src/Storages/S3/tests/gtest_s3gcmanager.cpp +++ b/dbms/src/Storages/S3/tests/gtest_s3gcmanager.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -42,6 +43,22 @@ class S3GCManagerTest : public ::testing::Test { } + void SetUp() override + { + S3GCConfig config{ + .manifest_expired_hour = 1, + .delmark_expired_hour = 1, + .temp_path = tests::TiFlashTestEnv::getTemporaryPath(), + }; + mock_s3_client = std::make_shared(); + auto mock_gc_owner = OwnerManager::createMockOwner("owner_0"); + auto mock_lock_client = std::make_shared(mock_s3_client); + auto mock_pd_client = std::make_shared(); + gc_mgr = std::make_unique(mock_pd_client, mock_s3_client, mock_gc_owner, mock_lock_client, config); + } + + std::shared_ptr mock_s3_client; + std::unique_ptr gc_mgr; LoggerPtr log; }; @@ -74,18 +91,9 @@ try ASSERT_EQ(set.latestUploadSequence(), 81); ASSERT_EQ(set.latestManifestKey(), S3Filename::newCheckpointManifest(store_id, 81).toFullKey()); - S3GCConfig config{ - .manifest_expired_hour = 1, - .delmark_expired_hour = 1, - .temp_path = tests::TiFlashTestEnv::getTemporaryPath(), - }; - auto mock_client = std::make_shared(); - auto mock_lock_client = std::make_shared(mock_client); - auto mock_pd_client = std::make_shared(); - S3GCManager gc_mgr(mock_pd_client, mock_client, mock_lock_client, config); - gc_mgr.removeOutdatedManifest(set, &timepoint); + gc_mgr->removeOutdatedManifest(set, &timepoint); - auto delete_keys = mock_client->delete_keys; + auto delete_keys = mock_s3_client->delete_keys; ASSERT_EQ(delete_keys.size(), 2); ASSERT_EQ(delete_keys[0], S3Filename::newCheckpointManifest(store_id, 4).toFullKey()); ASSERT_EQ(delete_keys[1], S3Filename::newCheckpointManifest(store_id, 5).toFullKey()); @@ -96,23 +104,13 @@ CATCH TEST_F(S3GCManagerTest, RemoveDataFile) try { - S3GCConfig config{ - .manifest_expired_hour = 1, - .delmark_expired_hour = 1, - .temp_path = tests::TiFlashTestEnv::getTemporaryPath(), - }; - auto mock_client = std::make_shared(); - auto mock_lock_client = std::make_shared(mock_client); - auto mock_pd_client = std::make_shared(); - S3GCManager gc_mgr(mock_pd_client, mock_client, mock_lock_client, config); - auto timepoint = Aws::Utils::DateTime("2023-02-01T08:00:00Z", Aws::Utils::DateFormat::ISO_8601); { // delmark expired auto delmark_mtime = timepoint - std::chrono::milliseconds(3601 * 1000); - gc_mgr.removeDataFileIfDelmarkExpired("datafile_key", "datafile_key.del", timepoint, delmark_mtime); + gc_mgr->removeDataFileIfDelmarkExpired("datafile_key", "datafile_key.del", timepoint, delmark_mtime); - auto delete_keys = mock_client->delete_keys; + auto delete_keys = mock_s3_client->delete_keys; ASSERT_EQ(delete_keys.size(), 2); ASSERT_EQ(delete_keys[0], "datafile_key"); ASSERT_EQ(delete_keys[1], "datafile_key.del"); @@ -120,9 +118,9 @@ try { // delmark not expired auto delmark_mtime = timepoint - std::chrono::milliseconds(3599 * 1000); - gc_mgr.removeDataFileIfDelmarkExpired("datafile_key", "datafile_key.del", timepoint, delmark_mtime); + gc_mgr->removeDataFileIfDelmarkExpired("datafile_key", "datafile_key.del", timepoint, delmark_mtime); - auto delete_keys = mock_client->delete_keys; + auto delete_keys = mock_s3_client->delete_keys; ASSERT_EQ(delete_keys.size(), 2); ASSERT_EQ(delete_keys[0], "datafile_key"); ASSERT_EQ(delete_keys[1], "datafile_key.del"); @@ -134,16 +132,6 @@ CATCH TEST_F(S3GCManagerTest, RemoveLock) try { - S3GCConfig config{ - .manifest_expired_hour = 1, - .delmark_expired_hour = 1, - .temp_path = tests::TiFlashTestEnv::getTemporaryPath(), - }; - auto mock_client = std::make_shared(); - auto mock_lock_client = std::make_shared(mock_client); - auto mock_pd_client = std::make_shared(); - S3GCManager gc_mgr(mock_pd_client, mock_client, mock_lock_client, config); - StoreID store_id = 20; auto df = S3Filename::newCheckpointData(store_id, 300, 1); @@ -153,59 +141,59 @@ try auto timepoint = Aws::Utils::DateTime("2023-02-01T08:00:00Z", Aws::Utils::DateFormat::ISO_8601); { // delmark not exist, and no more lockfile - mock_client->clear(); - gc_mgr.cleanOneLock(lock_key, lock_view, timepoint); + mock_s3_client->clear(); + gc_mgr->cleanOneLock(lock_key, lock_view, timepoint); // lock is deleted and delmark is created - auto delete_keys = mock_client->delete_keys; + auto delete_keys = mock_s3_client->delete_keys; ASSERT_EQ(delete_keys.size(), 1); ASSERT_EQ(delete_keys[0], lock_key); - auto put_keys = mock_client->put_keys; + auto put_keys = mock_s3_client->put_keys; ASSERT_EQ(put_keys.size(), 1); ASSERT_EQ(put_keys[0], df.toView().getDelMarkKey()); } { // delmark not exist, but still locked by another lockfile - mock_client->clear(); + mock_s3_client->clear(); auto another_lock_key = df.toView().getLockKey(store_id + 1, 450); - mock_client->list_result = {another_lock_key}; - gc_mgr.cleanOneLock(lock_key, lock_view, timepoint); + mock_s3_client->list_result = {another_lock_key}; + gc_mgr->cleanOneLock(lock_key, lock_view, timepoint); // lock is deleted and delmark is created - auto delete_keys = mock_client->delete_keys; + auto delete_keys = mock_s3_client->delete_keys; ASSERT_EQ(delete_keys.size(), 1); ASSERT_EQ(delete_keys[0], lock_key); - auto put_keys = mock_client->put_keys; + auto put_keys = mock_s3_client->put_keys; ASSERT_EQ(put_keys.size(), 0); } { // delmark exist, not expired - mock_client->clear(); + mock_s3_client->clear(); auto delmark_mtime = timepoint - std::chrono::milliseconds(3599 * 1000); - mock_client->head_result_mtime = delmark_mtime; - gc_mgr.cleanOneLock(lock_key, lock_view, timepoint); + mock_s3_client->head_result_mtime = delmark_mtime; + gc_mgr->cleanOneLock(lock_key, lock_view, timepoint); // lock is deleted, datafile and delmark remain - auto delete_keys = mock_client->delete_keys; + auto delete_keys = mock_s3_client->delete_keys; ASSERT_EQ(delete_keys.size(), 1); ASSERT_EQ(delete_keys[0], lock_key); - auto put_keys = mock_client->put_keys; + auto put_keys = mock_s3_client->put_keys; ASSERT_EQ(put_keys.size(), 0); } { // delmark exist, expired - mock_client->clear(); + mock_s3_client->clear(); auto delmark_mtime = timepoint - std::chrono::milliseconds(3601 * 1000); - mock_client->head_result_mtime = delmark_mtime; - gc_mgr.cleanOneLock(lock_key, lock_view, timepoint); + mock_s3_client->head_result_mtime = delmark_mtime; + gc_mgr->cleanOneLock(lock_key, lock_view, timepoint); // lock datafile and delmark are deleted - auto delete_keys = mock_client->delete_keys; + auto delete_keys = mock_s3_client->delete_keys; ASSERT_EQ(delete_keys.size(), 3); ASSERT_EQ(delete_keys[0], lock_key); ASSERT_EQ(delete_keys[1], df.toFullKey()); ASSERT_EQ(delete_keys[2], df.toView().getDelMarkKey()); - auto put_keys = mock_client->put_keys; + auto put_keys = mock_s3_client->put_keys; ASSERT_EQ(put_keys.size(), 0); } } @@ -214,16 +202,6 @@ CATCH TEST_F(S3GCManagerTest, ScanLocks) try { - S3GCConfig config{ - .manifest_expired_hour = 1, - .delmark_expired_hour = 1, - .temp_path = tests::TiFlashTestEnv::getTemporaryPath(), - }; - auto mock_client = std::make_shared(); - auto mock_lock_client = std::make_shared(mock_client); - auto mock_pd_client = std::make_shared(); - S3GCManager gc_mgr(mock_pd_client, mock_client, mock_lock_client, config); - StoreID store_id = 20; StoreID lock_store_id = 21; UInt64 safe_sequence = 100; @@ -258,19 +236,19 @@ try expected_created_delmark = df.toView().getDelMarkKey(); keys.emplace_back(lock_key); } - mock_client->clear(); - mock_client->list_result = keys; // set for `LIST` + mock_s3_client->clear(); + mock_s3_client->list_result = keys; // set for `LIST` } { auto timepoint = Aws::Utils::DateTime("2023-02-01T08:00:00Z", Aws::Utils::DateFormat::ISO_8601); - gc_mgr.cleanUnusedLocks(lock_store_id, S3Filename::getLockPrefix(), safe_sequence, valid_lock_files, timepoint); + gc_mgr->cleanUnusedLocks(lock_store_id, S3Filename::getLockPrefix(), safe_sequence, valid_lock_files, timepoint); // lock is deleted and delmark is created - auto delete_keys = mock_client->delete_keys; + auto delete_keys = mock_s3_client->delete_keys; ASSERT_EQ(delete_keys.size(), 1); ASSERT_EQ(delete_keys[0], expected_deleted_lock_key); - auto put_keys = mock_client->put_keys; + auto put_keys = mock_s3_client->put_keys; ASSERT_EQ(put_keys.size(), 1); ASSERT_EQ(put_keys[0], expected_created_delmark); } diff --git a/dbms/src/Storages/Transaction/TMTContext.cpp b/dbms/src/Storages/Transaction/TMTContext.cpp index 4d2cf93d88c..a67b1bb9f6e 100644 --- a/dbms/src/Storages/Transaction/TMTContext.cpp +++ b/dbms/src/Storages/Transaction/TMTContext.cpp @@ -102,7 +102,7 @@ TMTContext::TMTContext(Context & context_, const TiFlashRaftConfig & raft_config S3::S3GCConfig gc_config; gc_config.temp_path = context.getTemporaryPath(); // TODO: add suffix for it? - s3gc_manager = std::make_unique(context, cluster->pd_client, s3_lock_client, gc_config); + s3gc_manager = std::make_unique(context, cluster->pd_client, s3gc_owner, s3_lock_client, gc_config); } } @@ -146,6 +146,12 @@ void TMTContext::shutdown() s3gc_owner = nullptr; } + if (s3gc_manager) + { + s3gc_manager->shutdown(); + s3gc_manager = nullptr; + } + if (background_service) { background_service->shutdown(); diff --git a/dbms/src/TestUtils/MockS3Client.cpp b/dbms/src/TestUtils/MockS3Client.cpp index ee66eacb752..cab95bf6ed4 100644 --- a/dbms/src/TestUtils/MockS3Client.cpp +++ b/dbms/src/TestUtils/MockS3Client.cpp @@ -1,3 +1,17 @@ +// Copyright 2023 PingCAP, Ltd. +// +// 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 diff --git a/dbms/src/TestUtils/MockS3Client.h b/dbms/src/TestUtils/MockS3Client.h index bfca6320bad..ab84e3e432c 100644 --- a/dbms/src/TestUtils/MockS3Client.h +++ b/dbms/src/TestUtils/MockS3Client.h @@ -1,3 +1,17 @@ +// Copyright 2023 PingCAP, Ltd. +// +// 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 diff --git a/dbms/src/TiDB/OwnerManager.h b/dbms/src/TiDB/OwnerManager.h index 0efa4f274e7..81a9dd82735 100644 --- a/dbms/src/TiDB/OwnerManager.h +++ b/dbms/src/TiDB/OwnerManager.h @@ -51,9 +51,6 @@ using LeaderKey = v3electionpb::LeaderKey; } // namespace Etcd class OwnerManager; -// Now owner manager is created in TMTContext, but -// used in S3LockService. It is hard to find out -// which will be shutdown first. So use shared_ptr now. using OwnerManagerPtr = std::shared_ptr; namespace tests