Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

reload auxiliary zookeepers configuration #16627

Merged
merged 1 commit into from
Nov 11, 2020
Merged

reload auxiliary zookeepers configuration #16627

merged 1 commit into from
Nov 11, 2020

Conversation

amosbird
Copy link
Collaborator

@amosbird amosbird commented Nov 3, 2020 •
edited by alesapin
Loading

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Improvement

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

Now, <auxiliary_zookeepers> configuration can be changed in config.xml and reloaded without server startup.

@robot-clickhouse robot-clickhouse added the pr-improvement Pull request with some product improvements label Nov 3, 2020
@amosbird amosbird changed the title reload auxiliar zookeepers configuration reload auxiliary zookeepers configuration Nov 3, 2020
@sundy-li sundy-li mentioned this pull request Nov 3, 2020
Merged
@alesapin alesapin self-assigned this Nov 9, 2020
alesapin
alesapin reviewed Nov 9, 2020
View reviewed changes
Copy link
Member

@alesapin alesapin left a comment

Choose a reason for hiding this comment

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

I think we should make this code more consistent with "main" ZooKeeper.

src/Interpreters/Context.cpp Outdated
{
if (!zk || zk->configChanged(*config, config_name))
zk = std::make_shared<zkutil::ZooKeeper>(*config, config_name);
if (zk->expired())
Copy link
Member

Choose a reason for hiding this comment

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

But how we can get into this branch? Seems like this logic is not consistent with function name?

Copy link
Member

Choose a reason for hiding this comment

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

Still don't understand why we need it.

programs/server/Server.cpp Outdated
@@ -555,6 +555,7 @@ int Server::main(const std::vector<std::string> & /*args*/)
global_context->setClustersConfig(config);
global_context->setMacros(std::make_unique<Macros>(*config, "macros", log));
global_context->setExternalAuthenticatorsConfig(*config);
global_context->setAuxiliaryZooKeepersConfig(config);
Copy link
Member

Choose a reason for hiding this comment

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

I think we should consistent with the main "zookeeper" config logic: https://github.com/ClickHouse/ClickHouse/pull/16627/files#diff-330af1a9f5e0bc78a4831acfc2b01918e1a906d8d4d95ec22f8434dd1c0b4743R567-R568. So instead of setAuxiliaryZooKeepersConfig it's will be better to add reloadAuxiliaryZooKeeperIfChanged.

@alesapin
Copy link
Member 

2020-11-09 15:27:11 [e2119a3fd15d] 2020.11.09 15:27:11.086007 [ 6886 ] {fe931347-6954-4169-be82-2fdfefcc7c1b} <Error> executeQuery: Code: 529, e.displayText() = DB::Exception: OPTIMIZE cannot be done on this replica because it is not a leader (version 20.12.1.5136) (from [::1]:37216) (in query: OPTIMIZE TABLE r_prop_table2 FINAL; ), Stack trace (when copying this message, always include the lines below):
2020-11-09 15:27:11 
2020-11-09 15:27:11 0. /build/obj-x86_64-linux-gnu/../contrib/libcxx/include/exception:129: Poco::Exception::Exception(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int) @ 0x197b4562 in /usr/bin/clickhouse
2020-11-09 15:27:11 1. /build/obj-x86_64-linux-gnu/../src/Common/Exception.cpp:40: DB::Exception::Exception(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int) @ 0xd6e8d54 in /usr/bin/clickhouse
2020-11-09 15:27:11 2. /build/obj-x86_64-linux-gnu/../src/Storages/StorageReplicatedMergeTree.cpp:0: DB::StorageReplicatedMergeTree::optimize(std::__1::shared_ptr<DB::IAST> const&, std::__1::shared_ptr<DB::StorageInMemoryMetadata const> const&, std::__1::shared_ptr<DB::IAST> const&, bool, bool, DB::Context const&) @ 0x1840fc7f in /usr/bin/clickhouse
2020-11-09 15:27:11 3. /build/obj-x86_64-linux-gnu/../src/Interpreters/InterpreterOptimizeQuery.cpp:0: DB::InterpreterOptimizeQuery::execute() @ 0x17b1e346 in /usr/bin/clickhouse
2020-11-09 15:27:11 4. /build/obj-x86_64-linux-gnu/../src/Interpreters/executeQuery.cpp:0: DB::executeQueryImpl(char const*, char const*, DB::Context&, bool, DB::QueryProcessingStage::Enum, bool, DB::ReadBuffer*) @ 0x1804ffb5 in /usr/bin/clickhouse
2020-11-09 15:27:11 5. /build/obj-x86_64-linux-gnu/../src/Interpreters/executeQuery.cpp:815: DB::executeQuery(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, DB::Context&, bool, DB::QueryProcessingStage::Enum, bool) @ 0x1804ed5f in /usr/bin/clickhouse
2020-11-09 15:27:11 6. /build/obj-x86_64-linux-gnu/../src/Server/TCPHandler.cpp:254: DB::TCPHandler::runImpl() @ 0x18995ff8 in /usr/bin/clickhouse
2020-11-09 15:27:11 7. /build/obj-x86_64-linux-gnu/../src/Server/TCPHandler.cpp:0: DB::TCPHandler::run() @ 0x189abfba in /usr/bin/clickhouse
2020-11-09 15:27:11 8. /build/obj-x86_64-linux-gnu/../contrib/poco/Net/src/TCPServerConnection.cpp:57: Poco::Net::TCPServerConnection::start() @ 0x1973388c in /usr/bin/clickhouse
2020-11-09 15:27:11 9. /build/obj-x86_64-linux-gnu/../contrib/libcxx/include/atomic:1036: Poco::Net::TCPServerDispatcher::run() @ 0x19733cce in /usr/bin/clickhouse
2020-11-09 15:27:11 10. /build/obj-x86_64-linux-gnu/../contrib/poco/Foundation/include/Poco/ScopedLock.h:36: Poco::PooledThread::run() @ 0x1980f5ba in /usr/bin/clickhouse
2020-11-09 15:27:11 11. /build/obj-x86_64-linux-gnu/../contrib/poco/Foundation/include/Poco/SharedPtr.h:156: Poco::ThreadImpl::runnableEntry(void*) @ 0x1980d56c in /usr/bin/clickhouse
2020-11-09 15:27:11 12. start_thread @ 0x9669 in /usr/lib/x86_64-linux-gnu/libpthread-2.30.so
2020-11-09 15:27:11 13. clone @ 0x1222b3 in /usr/lib/x86_64-linux-gnu/libc-2.30.so
2020-11-09 15:27:11 
2020-11-09 15:27:11 Received exception from server (version 20.12.1):
2020-11-09 15:27:11 Code: 529. DB::Exception: Received from localhost:9000. DB::Exception: OPTIMIZE cannot be done on this replica because it is not a leader.

And what should we do?

alesapin
alesapin reviewed Nov 10, 2020
View reviewed changes
programs/server/Server.cpp Show resolved Hide resolved
src/Interpreters/Context.cpp Outdated
{
if (!zk || zk->configChanged(*config, config_name))
zk = std::make_shared<zkutil::ZooKeeper>(*config, config_name);
if (zk->expired())
Copy link
Member

Choose a reason for hiding this comment

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

Still don't understand why we need it.

@alesapin
Copy link
Member

Test failures are not related to changes. Need to fix integration tests...

@alesapin alesapin merged commit 637e3dc into ClickHouse:master Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alesapin alesapin alesapin approved these changes

Assignees

@alesapin alesapin

Labels
pr-improvement Pull request with some product improvements
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@amosbird @alesapin @robot-clickhouse