Skip to content

Commit 312508b

Browse files
Merge pull request ClickHouse#89968 from ClickHouse/backport/25.8/89259
Backport ClickHouse#89259 to 25.8: Only change S3Queue metadata in Keeper on the initial replica
2 parents b11eaf6 + 634725d commit 312508b

File tree

1 file changed

+22
-11
lines changed

1 file changed

+22
-11
lines changed

src/Storages/ObjectStorageQueue/ObjectStorageQueueMetadata.cpp

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <Common/getRandomASCIIString.h>
1919
#include <Common/randomSeed.h>
2020
#include <Common/DNSResolver.h>
21+
#include <Interpreters/DDLTask.h>
2122
#include <shared_mutex>
2223
#include <Core/ServerUUID.h>
2324

@@ -244,24 +245,30 @@ ObjectStorageQueueMetadata::tryAcquireBucket(const Bucket & bucket, const Proces
244245

245246
void ObjectStorageQueueMetadata::alterSettings(const SettingsChanges & changes, const ContextPtr & context)
246247
{
248+
bool is_initial_query = context->getClientInfo().query_kind == ClientInfo::QueryKind::INITIAL_QUERY ||
249+
(context->getZooKeeperMetadataTransaction() && context->getZooKeeperMetadataTransaction()->isInitialQuery());
250+
247251
const fs::path alter_settings_lock_path = zookeeper_path / "alter_settings_lock";
248252
zkutil::EphemeralNodeHolder::Ptr alter_settings_lock;
249253
auto zookeeper = getZooKeeper();
250254

251-
/// We will retry taking alter_settings_lock for the duration of 5 seconds.
252-
/// Do we need to add a setting for this?
253-
const size_t num_tries = 100;
254-
for (size_t i = 0; i < num_tries; ++i)
255+
if (is_initial_query)
255256
{
256-
alter_settings_lock = zkutil::EphemeralNodeHolder::tryCreate(alter_settings_lock_path, *zookeeper, toString(getCurrentTime()));
257+
/// We will retry taking alter_settings_lock for the duration of 5 seconds.
258+
/// Do we need to add a setting for this?
259+
const size_t num_tries = 100;
260+
for (size_t i = 0; i < num_tries; ++i)
261+
{
262+
alter_settings_lock = zkutil::EphemeralNodeHolder::tryCreate(alter_settings_lock_path, *zookeeper, toString(getCurrentTime()));
257263

258-
if (alter_settings_lock)
259-
break;
264+
if (alter_settings_lock)
265+
break;
260266

261-
if (i == num_tries - 1)
262-
throw Exception(ErrorCodes::LOGICAL_ERROR, "Failed to take alter setting lock");
267+
if (i == num_tries - 1)
268+
throw Exception(ErrorCodes::LOGICAL_ERROR, "Failed to take alter setting lock");
263269

264-
sleepForMilliseconds(50);
270+
sleepForMilliseconds(50);
271+
}
265272
}
266273

267274
Coordination::Stat stat;
@@ -373,7 +380,11 @@ void ObjectStorageQueueMetadata::alterSettings(const SettingsChanges & changes,
373380
LOG_TRACE(log, "New metadata: {}", new_metadata_str);
374381

375382
const fs::path table_metadata_path = zookeeper_path / "metadata";
376-
zookeeper->set(table_metadata_path, new_metadata_str, stat.version);
383+
/// Here we intentionally do not add zk retries,
384+
/// because we modify metadata under ephemeral metadata lock,
385+
/// so we do not want to retry if it expires.
386+
if (is_initial_query)
387+
zookeeper->set(table_metadata_path, new_metadata_str, stat.version);
377388

378389
table_metadata.syncChangeableSettings(new_table_metadata);
379390
}

0 commit comments

Comments
 (0)