Skip to content

Add logs_to_keep to replicated database settings#84183

Merged
tuanpach merged 7 commits intoClickHouse:masterfrom
Khatskevich:khatskevich/add_logs_to_keep
Aug 22, 2025
Merged

Add logs_to_keep to replicated database settings#84183
tuanpach merged 7 commits intoClickHouse:masterfrom
Khatskevich:khatskevich/add_logs_to_keep

Conversation

@Khatskevich
Copy link
Copy Markdown
Contributor

@Khatskevich Khatskevich commented Jul 22, 2025

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Added a server setting, logs_to_keep to database replicated settings, that allows changing the default logs_to_keep parameter for replicated databases. Lower values reduce the number of ZNodes (especially if there are many databases), while higher values allow a missing replica to catch up after a longer period of time.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)
    (short docs are written directly into src/Core/ServerSettings.cpp

@bharatnc bharatnc added the can be tested Allows running workflows for external contributors label Jul 22, 2025
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Jul 22, 2025

Workflow [PR], commit [8786630]

@clickhouse-gh clickhouse-gh bot added the pr-improvement Pull request with some product improvements label Jul 22, 2025
ops.emplace_back(zkutil::makeCreateRequest(zookeeper_path + "/metadata", "", zkutil::CreateMode::Persistent));
ops.emplace_back(zkutil::makeCreateRequest(zookeeper_path + "/max_log_ptr", "1", zkutil::CreateMode::Persistent));
ops.emplace_back(zkutil::makeCreateRequest(zookeeper_path + "/logs_to_keep", "1000", zkutil::CreateMode::Persistent));
auto logs_to_keep = getContext()->getServerSettings().database_replicated_logs_to_keep;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must declare the setting before using it:

namespace ServerSetting
{
extern const ServerSettingsBool database_replicated_allow_detach_permanently;
extern const ServerSettingsUInt32 max_database_replicated_create_table_thread_pool_size;
}

auto allow_concurrent_table_creation = getContext()->getServerSettings()[ServerSetting::max_database_replicated_create_table_thread_pool_size] != 1;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Declared.

@tuanpach tuanpach self-assigned this Jul 22, 2025
Reducing this value might save many ZNodes in
a setup with many databases.
@Khatskevich Khatskevich force-pushed the khatskevich/add_logs_to_keep branch from 626774d to ad9d1c9 Compare July 23, 2025 22:26
@Khatskevich Khatskevich requested a review from tuanpach July 23, 2025 22:27
@tuanpach
Copy link
Copy Markdown
Member

We should move this setting to DatabaseReplicatedSettings

https://github.com/ClickHouse/ClickHouse/blob/master/src/Databases/DatabaseReplicatedSettings.cpp#L10

@tuanpach
Copy link
Copy Markdown
Member

@Khatskevich, I made some changes. Please take a look. Thanks

@Khatskevich
Copy link
Copy Markdown
Contributor Author

@tuanpach looks good, thank you!
The only possibility to improve it would be to make the logs_to_keep variable dynamic and adjustable after a database creation, but to me it looks like overkill. It makes sense to leave it as is.

@tuanpach tuanpach added this pull request to the merge queue Aug 22, 2025
@tuanpach tuanpach changed the title Allow adjusting logs_to_keep in replicated database Add logs_to_keep to replicated database settings Aug 22, 2025
Merged via the queue into ClickHouse:master with commit 418f5d8 Aug 22, 2025
119 of 122 checks passed
@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-synced-to-cloud The PR is synced to the cloud repo label Aug 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

can be tested Allows running workflows for external contributors pr-improvement Pull request with some product improvements pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants