Skip to content

Commit e481794

Browse files
alesapinalesapin
authored
Merge pull request ClickHouse#45650 from ClickHouse/backport/22.8/38262-fix-system-unfreeze-for-ordinary-database
Merge pull request ClickHouse#38262 from PolyProgrammist/fix-ordinary-system-un…
2 parents ce99631 + aa9f39a commit e481794

File tree

14 files changed

+121
-88
lines changed

14 files changed

+121
-88
lines changed

‎src/Interpreters/InterpreterSystemQuery.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ BlockIO InterpreterSystemQuery::execute()
539539
{
540540
getContext()->checkAccess(AccessType::SYSTEM_UNFREEZE);
541541
/// The result contains information about deleted parts as a table. It is for compatibility with ALTER TABLE UNFREEZE query.
542-
result = Unfreezer().unfreeze(query.backup_name, getContext());
542+
result = Unfreezer(getContext()).systemUnfreeze(query.backup_name);
543543
break;
544544
}
545545
default:

‎src/Parsers/ASTSystemQuery.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,10 @@ void ASTSystemQuery::formatImpl(const FormatSettings & settings, FormatState &,
201201
if (!filesystem_cache_path.empty())
202202
settings.ostr << (settings.hilite ? hilite_none : "") << " " << filesystem_cache_path;
203203
}
204+
else if (type == Type::UNFREEZE)
205+
{
206+
settings.ostr << (settings.hilite ? hilite_identifier : "") << backQuoteIfNeed(backup_name);
207+
}
204208
}
205209

206210

‎src/Storages/Freeze.cpp‎

Lines changed: 85 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,29 @@
55
#include <Common/escapeForFileName.h>
66
#include <Common/logger_useful.h>
77

8+
/**
9+
* When ClickHouse has frozen data on remote storage it required 'smart' data removing during UNFREEZE.
10+
* For remote storage actually frozen not remote data but local metadata with referrers on remote data.
11+
* So remote data can be referred from working and frozen data sets (or two frozen) at same time.
12+
* In this case during UNFREEZE ClickHouse should remove only local metadata and keep remote data.
13+
* But when data was already removed from working data set ClickHouse should remove remote data too.
14+
* To detect is current data used or not in some other place ClickHouse uses
15+
* - ref_count from metadata to check if data used in some other metadata on the same replica;
16+
* - Keeper record to check if data used on other replica.
17+
* StorageReplicatedMergeTree::removeSharedDetachedPart makes required checks, so here this method
18+
* called for each frozen part.
19+
*/
20+
821
namespace DB
922
{
23+
24+
namespace ErrorCodes
25+
{
26+
extern const int SUPPORT_IS_DISABLED;
27+
}
28+
1029
void FreezeMetaData::fill(const StorageReplicatedMergeTree & storage)
1130
{
12-
is_replicated = storage.supportsReplication();
13-
is_remote = storage.isRemote();
1431
replica_name = storage.getReplicaName();
1532
zookeeper_name = storage.getZooKeeperName();
1633
table_shared_id = storage.getTableSharedID();
@@ -26,11 +43,17 @@ void FreezeMetaData::save(DiskPtr data_disk, const String & path) const
2643

2744
writeIntText(version, buffer);
2845
buffer.write("\n", 1);
29-
writeBoolText(is_replicated, buffer);
30-
buffer.write("\n", 1);
31-
writeBoolText(is_remote, buffer);
32-
buffer.write("\n", 1);
33-
writeString(replica_name, buffer);
46+
if (version == 1)
47+
{
48+
/// is_replicated and is_remote are not used
49+
bool is_replicated = true;
50+
writeBoolText(is_replicated, buffer);
51+
buffer.write("\n", 1);
52+
bool is_remote = true;
53+
writeBoolText(is_remote, buffer);
54+
buffer.write("\n", 1);
55+
}
56+
writeString(escapeForFileName(replica_name), buffer);
3457
buffer.write("\n", 1);
3558
writeString(zookeeper_name, buffer);
3659
buffer.write("\n", 1);
@@ -51,17 +74,25 @@ bool FreezeMetaData::load(DiskPtr data_disk, const String & path)
5174
auto metadata_str = metadata_storage->readFileToString(file_path);
5275
ReadBufferFromString buffer(metadata_str);
5376
readIntText(version, buffer);
54-
if (version != 1)
77+
if (version < 1 || version > 2)
5578
{
56-
LOG_ERROR(&Poco::Logger::get("FreezeMetaData"), "Unknown freezed metadata version: {}", version);
79+
LOG_ERROR(&Poco::Logger::get("FreezeMetaData"), "Unknown frozen metadata version: {}", version);
5780
return false;
5881
}
5982
DB::assertChar('\n', buffer);
60-
readBoolText(is_replicated, buffer);
61-
DB::assertChar('\n', buffer);
62-
readBoolText(is_remote, buffer);
63-
DB::assertChar('\n', buffer);
64-
readString(replica_name, buffer);
83+
if (version == 1)
84+
{
85+
/// is_replicated and is_remote are not used
86+
bool is_replicated;
87+
readBoolText(is_replicated, buffer);
88+
DB::assertChar('\n', buffer);
89+
bool is_remote;
90+
readBoolText(is_remote, buffer);
91+
DB::assertChar('\n', buffer);
92+
}
93+
std::string unescaped_replica_name;
94+
readString(unescaped_replica_name, buffer);
95+
replica_name = unescapeForFileName(unescaped_replica_name);
6596
DB::assertChar('\n', buffer);
6697
readString(zookeeper_name, buffer);
6798
DB::assertChar('\n', buffer);
@@ -87,43 +118,62 @@ String FreezeMetaData::getFileName(const String & path)
87118
return fs::path(path) / "frozen_metadata.txt";
88119
}
89120

90-
BlockIO Unfreezer::unfreeze(const String & backup_name, ContextPtr local_context)
121+
Unfreezer::Unfreezer(ContextPtr context) : local_context(context)
122+
{
123+
if (local_context->hasZooKeeper())
124+
zookeeper = local_context->getZooKeeper();
125+
}
126+
127+
BlockIO Unfreezer::systemUnfreeze(const String & backup_name)
91128
{
92-
LOG_DEBUG(log, "Unfreezing backup {}", backup_name);
129+
LOG_DEBUG(log, "Unfreezing backup {}", escapeForFileName(backup_name));
130+
131+
const auto & config = local_context->getConfigRef();
132+
static constexpr auto config_key = "enable_system_unfreeze";
133+
if (!config.getBool(config_key, false))
134+
{
135+
throw Exception(ErrorCodes::SUPPORT_IS_DISABLED, "Support for SYSTEM UNFREEZE query is disabled. You can enable it via '{}' server setting", config_key);
136+
}
137+
93138
auto disks_map = local_context->getDisksMap();
94139
Disks disks;
95140
for (auto & [name, disk]: disks_map)
96141
{
97142
disks.push_back(disk);
98143
}
99144
auto backup_path = fs::path(backup_directory_prefix) / escapeForFileName(backup_name);
100-
auto store_path = backup_path / "store";
145+
auto store_paths = {backup_path / "store", backup_path / "data"};
101146

102147
PartitionCommandsResultInfo result_info;
103148

104149
for (const auto & disk: disks)
105150
{
106-
if (!disk->exists(store_path))
107-
continue;
108-
for (auto prefix_it = disk->iterateDirectory(store_path); prefix_it->isValid(); prefix_it->next())
151+
for (const auto& store_path: store_paths)
109152
{
110-
auto prefix_directory = store_path / prefix_it->name();
111-
for (auto table_it = disk->iterateDirectory(prefix_directory); table_it->isValid(); table_it->next())
153+
if (!disk->exists(store_path))
154+
continue;
155+
for (auto prefix_it = disk->iterateDirectory(store_path); prefix_it->isValid(); prefix_it->next())
112156
{
113-
auto table_directory = prefix_directory / table_it->name();
114-
auto current_result_info = unfreezePartitionsFromTableDirectory([] (const String &) { return true; }, backup_name, {disk}, table_directory, local_context);
115-
for (auto & command_result : current_result_info)
157+
auto prefix_directory = store_path / prefix_it->name();
158+
for (auto table_it = disk->iterateDirectory(prefix_directory); table_it->isValid(); table_it->next())
116159
{
117-
command_result.command_type = "SYSTEM UNFREEZE";
160+
auto table_directory = prefix_directory / table_it->name();
161+
auto current_result_info = unfreezePartitionsFromTableDirectory(
162+
[](const String &) { return true; }, backup_name, {disk}, table_directory);
163+
for (auto & command_result : current_result_info)
164+
{
165+
command_result.command_type = "SYSTEM UNFREEZE";
166+
}
167+
result_info.insert(
168+
result_info.end(),
169+
std::make_move_iterator(current_result_info.begin()),
170+
std::make_move_iterator(current_result_info.end()));
118171
}
119-
result_info.insert(
120-
result_info.end(),
121-
std::make_move_iterator(current_result_info.begin()),
122-
std::make_move_iterator(current_result_info.end()));
123172
}
124173
}
125174
if (disk->exists(backup_path))
126175
{
176+
/// After unfreezing we need to clear revision.txt file and empty directories
127177
disk->removeRecursive(backup_path);
128178
}
129179
}
@@ -136,18 +186,15 @@ BlockIO Unfreezer::unfreeze(const String & backup_name, ContextPtr local_context
136186
return result;
137187
}
138188

139-
bool Unfreezer::removeFreezedPart(DiskPtr disk, const String & path, const String & part_name, ContextPtr local_context)
189+
bool Unfreezer::removeFreezedPart(DiskPtr disk, const String & path, const String & part_name, ContextPtr local_context, zkutil::ZooKeeperPtr zookeeper)
140190
{
141191
if (disk->supportZeroCopyReplication())
142192
{
143193
FreezeMetaData meta;
144194
if (meta.load(disk, path))
145195
{
146-
if (meta.is_replicated)
147-
{
148-
FreezeMetaData::clean(disk, path);
149-
return StorageReplicatedMergeTree::removeSharedDetachedPart(disk, path, part_name, meta.table_shared_id, meta.zookeeper_name, meta.replica_name, "", local_context);
150-
}
196+
FreezeMetaData::clean(disk, path);
197+
return StorageReplicatedMergeTree::removeSharedDetachedPart(disk, path, part_name, meta.table_shared_id, meta.zookeeper_name, meta.replica_name, "", local_context, zookeeper);
151198
}
152199
}
153200

@@ -156,7 +203,7 @@ bool Unfreezer::removeFreezedPart(DiskPtr disk, const String & path, const Strin
156203
return false;
157204
}
158205

159-
PartitionCommandsResultInfo Unfreezer::unfreezePartitionsFromTableDirectory(MergeTreeData::MatcherFn matcher, const String & backup_name, const Disks & disks, const fs::path & table_directory, ContextPtr local_context)
206+
PartitionCommandsResultInfo Unfreezer::unfreezePartitionsFromTableDirectory(MergeTreeData::MatcherFn matcher, const String & backup_name, const Disks & disks, const fs::path & table_directory)
160207
{
161208
PartitionCommandsResultInfo result;
162209

@@ -180,7 +227,7 @@ PartitionCommandsResultInfo Unfreezer::unfreezePartitionsFromTableDirectory(Merg
180227

181228
const auto & path = it->path();
182229

183-
bool keep_shared = removeFreezedPart(disk, path, partition_directory, local_context);
230+
bool keep_shared = removeFreezedPart(disk, path, partition_directory, local_context, zookeeper);
184231

185232
result.push_back(PartitionCommandResultInfo{
186233
.partition_id = partition_id,

‎src/Storages/Freeze.h‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,7 @@ struct FreezeMetaData
2323
static String getFileName(const String & path);
2424

2525
public:
26-
int version = 1;
27-
bool is_replicated{false};
28-
bool is_remote{false};
26+
int version = 2;
2927
String replica_name;
3028
String zookeeper_name;
3129
String table_shared_id;
@@ -34,12 +32,15 @@ struct FreezeMetaData
3432
class Unfreezer
3533
{
3634
public:
37-
PartitionCommandsResultInfo unfreezePartitionsFromTableDirectory(MergeTreeData::MatcherFn matcher, const String & backup_name, const Disks & disks, const fs::path & table_directory, ContextPtr local_context);
38-
BlockIO unfreeze(const String & backup_name, ContextPtr local_context);
35+
Unfreezer(ContextPtr context);
36+
PartitionCommandsResultInfo unfreezePartitionsFromTableDirectory(MergeTreeData::MatcherFn matcher, const String & backup_name, const Disks & disks, const fs::path & table_directory);
37+
BlockIO systemUnfreeze(const String & backup_name);
3938
private:
39+
ContextPtr local_context;
40+
zkutil::ZooKeeperPtr zookeeper;
4041
Poco::Logger * log = &Poco::Logger::get("Unfreezer");
4142
static constexpr std::string_view backup_directory_prefix = "shadow";
42-
static bool removeFreezedPart(DiskPtr disk, const String & path, const String & part_name, ContextPtr local_context);
43+
static bool removeFreezedPart(DiskPtr disk, const String & path, const String & part_name, ContextPtr local_context, zkutil::ZooKeeperPtr zookeeper);
4344
};
4445

4546
}

‎src/Storages/MergeTree/MergeTreeData.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1941,7 +1941,7 @@ size_t MergeTreeData::clearOldBrokenPartsFromDetachedDirecory()
19411941

19421942
for (auto & [old_name, new_name, disk] : renamed_parts.old_and_new_names)
19431943
{
1944-
removeDetachedPart(disk, fs::path(relative_data_path) / "detached" / new_name / "", old_name, false);
1944+
removeDetachedPart(disk, fs::path(relative_data_path) / "detached" / new_name / "", old_name);
19451945
LOG_DEBUG(log, "Removed broken detached part {} due to a timeout for broken detached parts", old_name);
19461946
old_name.clear();
19471947
}
@@ -4629,7 +4629,7 @@ void MergeTreeData::dropDetached(const ASTPtr & partition, bool part, ContextPtr
46294629

46304630
for (auto & [old_name, new_name, disk] : renamed_parts.old_and_new_names)
46314631
{
4632-
bool keep_shared = removeDetachedPart(disk, fs::path(relative_data_path) / "detached" / new_name / "", old_name, false);
4632+
bool keep_shared = removeDetachedPart(disk, fs::path(relative_data_path) / "detached" / new_name / "", old_name);
46334633
LOG_DEBUG(log, "Dropped detached part {}, keep shared data: {}", old_name, keep_shared);
46344634
old_name.clear();
46354635
}
@@ -6285,7 +6285,7 @@ PartitionCommandsResultInfo MergeTreeData::unfreezeAll(
62856285
return unfreezePartitionsByMatcher([] (const String &) { return true; }, backup_name, local_context);
62866286
}
62876287

6288-
bool MergeTreeData::removeDetachedPart(DiskPtr disk, const String & path, const String &, bool)
6288+
bool MergeTreeData::removeDetachedPart(DiskPtr disk, const String & path, const String &)
62896289
{
62906290
disk->removeRecursive(path);
62916291

@@ -6300,7 +6300,7 @@ PartitionCommandsResultInfo MergeTreeData::unfreezePartitionsByMatcher(MatcherFn
63006300

63016301
auto disks = getStoragePolicy()->getDisks();
63026302

6303-
return Unfreezer().unfreezePartitionsFromTableDirectory(matcher, backup_name, disks, backup_path, local_context);
6303+
return Unfreezer(local_context).unfreezePartitionsFromTableDirectory(matcher, backup_name, disks, backup_path);
63046304
}
63056305

63066306
bool MergeTreeData::canReplacePartition(const DataPartPtr & src_part) const

‎src/Storages/MergeTree/MergeTreeData.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -982,7 +982,7 @@ class MergeTreeData : public IStorage, public WithMutableContext
982982

983983
/// Check shared data usage on other replicas for detached/freezed part
984984
/// Remove local files and remote files if needed
985-
virtual bool removeDetachedPart(DiskPtr disk, const String & path, const String & part_name, bool is_freezed);
985+
virtual bool removeDetachedPart(DiskPtr disk, const String & path, const String & part_name);
986986

987987
virtual String getTableSharedID() const { return ""; }
988988

‎src/Storages/StorageReplicatedMergeTree.cpp‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8291,25 +8291,12 @@ void StorageReplicatedMergeTree::createZeroCopyLockNode(
82918291
}
82928292
}
82938293

8294-
bool StorageReplicatedMergeTree::removeDetachedPart(DiskPtr disk, const String & path, const String & part_name, bool is_freezed)
8294+
bool StorageReplicatedMergeTree::removeDetachedPart(DiskPtr disk, const String & path, const String & part_name)
82958295
{
82968296
if (disk->supportZeroCopyReplication())
82978297
{
8298-
if (is_freezed)
8299-
{
8300-
FreezeMetaData meta;
8301-
if (meta.load(disk, path))
8302-
{
8303-
FreezeMetaData::clean(disk, path);
8304-
return removeSharedDetachedPart(disk, path, part_name, meta.table_shared_id, meta.zookeeper_name, meta.replica_name, "", getContext());
8305-
}
8306-
}
8307-
else
8308-
{
8309-
String table_id = getTableSharedID();
8310-
8311-
return removeSharedDetachedPart(disk, path, part_name, table_id, zookeeper_name, replica_name, zookeeper_path, getContext());
8312-
}
8298+
String table_id = getTableSharedID();
8299+
return removeSharedDetachedPart(disk, path, part_name, table_id, zookeeper_name, replica_name, zookeeper_path, getContext(), current_zookeeper);
83138300
}
83148301

83158302
disk->removeRecursive(path);
@@ -8319,11 +8306,10 @@ bool StorageReplicatedMergeTree::removeDetachedPart(DiskPtr disk, const String &
83198306

83208307

83218308
bool StorageReplicatedMergeTree::removeSharedDetachedPart(DiskPtr disk, const String & path, const String & part_name, const String & table_uuid,
8322-
const String &, const String & detached_replica_name, const String & detached_zookeeper_path, ContextPtr local_context)
8309+
const String &, const String & detached_replica_name, const String & detached_zookeeper_path, ContextPtr local_context, const zkutil::ZooKeeperPtr & zookeeper)
83238310
{
83248311
bool keep_shared = false;
83258312

8326-
zkutil::ZooKeeperPtr zookeeper = local_context->getZooKeeper();
83278313
NameSet files_not_to_remove;
83288314

83298315
fs::path checksums = fs::path(path) / IMergeTreeDataPart::FILE_FOR_REFERENCES_CHECK;

‎src/Storages/StorageReplicatedMergeTree.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ class StorageReplicatedMergeTree final : public MergeTreeData
326326
void checkBrokenDisks();
327327

328328
static bool removeSharedDetachedPart(DiskPtr disk, const String & path, const String & part_name, const String & table_uuid,
329-
const String & zookeeper_name, const String & replica_name, const String & zookeeper_path, ContextPtr local_context);
329+
const String & zookeeper_name, const String & replica_name, const String & zookeeper_path, ContextPtr local_context, const zkutil::ZooKeeperPtr & zookeeper);
330330

331331
private:
332332
std::atomic_bool are_restoring_replica {false};
@@ -824,7 +824,7 @@ class StorageReplicatedMergeTree final : public MergeTreeData
824824
int32_t mode = zkutil::CreateMode::Persistent, bool replace_existing_lock = false,
825825
const String & path_to_set_hardlinked_files = "", const NameSet & hardlinked_files = {});
826826

827-
bool removeDetachedPart(DiskPtr disk, const String & path, const String & part_name, bool is_freezed) override;
827+
bool removeDetachedPart(DiskPtr disk, const String & path, const String & part_name) override;
828828

829829
/// Create freeze metadata for table and save in zookeeper. Required only if zero-copy replication enabled.
830830
void createAndStoreFreezeMetadata(DiskPtr disk, DataPartPtr part, String backup_part_path) const override;

‎tests/config/config.d/system_unfreeze.xml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
<?xml version="1.0"?>
2+
<clickhouse>
3+
<enable_system_unfreeze>true</enable_system_unfreeze>
4+
</clickhouse>

‎tests/config/install.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ ln -sf $SRC_PATH/config.d/named_collection.xml $DEST_SERVER_PATH/config.d/
4848
ln -sf $SRC_PATH/config.d/ssl_certs.xml $DEST_SERVER_PATH/config.d/
4949
ln -sf $SRC_PATH/config.d/filesystem_cache_log.xml $DEST_SERVER_PATH/config.d/
5050
ln -sf $SRC_PATH/config.d/session_log.xml $DEST_SERVER_PATH/config.d/
51+
ln -sf $SRC_PATH/config.d/system_unfreeze.xml $DEST_SERVER_PATH/config.d/
5152

5253
ln -sf $SRC_PATH/users.d/log_queries.xml $DEST_SERVER_PATH/users.d/
5354
ln -sf $SRC_PATH/users.d/readonly.xml $DEST_SERVER_PATH/users.d/

0 commit comments

Comments
 (0)