diff --git a/programs/benchmark/Benchmark.cpp b/programs/benchmark/Benchmark.cpp index ce59f5cac7f..91c43160e0f 100644 --- a/programs/benchmark/Benchmark.cpp +++ b/programs/benchmark/Benchmark.cpp @@ -289,7 +289,7 @@ private: connection_entries.emplace_back(std::make_shared( connection->get(ConnectionTimeouts::getTCPTimeoutsWithoutFailover(settings)))); - pool.scheduleOrThrowOnError(std::bind(&Benchmark::thread, this, connection_entries)); + pool.scheduleOrThrowOnError([this, connection_entries]() mutable { thread(connection_entries); }); } } catch (...) diff --git a/programs/client/Client.cpp b/programs/client/Client.cpp index 53947283faf..d6cac7a7b02 100644 --- a/programs/client/Client.cpp +++ b/programs/client/Client.cpp @@ -485,7 +485,7 @@ private: history_file = config().getString("history_file"); else { - auto history_file_from_env = getenv("CLICKHOUSE_HISTORY_FILE"); + auto * history_file_from_env = getenv("CLICKHOUSE_HISTORY_FILE"); if (history_file_from_env) history_file = history_file_from_env; else if (!home_path.empty()) @@ -1480,7 +1480,7 @@ private: "\033[1m↗\033[0m", }; - auto indicator = indicators[increment % 8]; + const char * indicator = indicators[increment % 8]; if (!send_logs && written_progress_chars) message << '\r'; diff --git a/programs/client/ConnectionParameters.cpp b/programs/client/ConnectionParameters.cpp index 50cac3b7800..f0ef3ae5694 100644 --- a/programs/client/ConnectionParameters.cpp +++ b/programs/client/ConnectionParameters.cpp @@ -51,7 +51,7 @@ ConnectionParameters::ConnectionParameters(const Poco::Util::AbstractConfigurati { std::string prompt{"Password for user (" + user + "): "}; char buf[1000] = {}; - if (auto result = readpassphrase(prompt.c_str(), buf, sizeof(buf), 0)) + if (auto * result = readpassphrase(prompt.c_str(), buf, sizeof(buf), 0)) password = result; } diff --git a/programs/copier/ClusterCopier.cpp b/programs/copier/ClusterCopier.cpp index 8df55b63407..45cfb4963a3 100644 --- a/programs/copier/ClusterCopier.cpp +++ b/programs/copier/ClusterCopier.cpp @@ -442,7 +442,7 @@ bool ClusterCopier::checkPartitionPieceIsDone(const TaskTable & task_table, cons /// Collect all shards that contain partition piece number piece_number. Strings piece_status_paths; - for (auto & shard : shards_with_partition) + for (const auto & shard : shards_with_partition) { ShardPartition & task_shard_partition = shard->partition_tasks.find(partition_name)->second; ShardPartitionPiece & shard_partition_piece = task_shard_partition.pieces[piece_number]; @@ -702,7 +702,7 @@ ASTPtr ClusterCopier::removeAliasColumnsFromCreateQuery(const ASTPtr & query_ast auto new_columns_list = std::make_shared(); new_columns_list->set(new_columns_list->columns, new_columns); - if (auto indices = query_ast->as()->columns_list->indices) + if (const auto * indices = query_ast->as()->columns_list->indices) new_columns_list->set(new_columns_list->indices, indices->clone()); new_query.replace(new_query.columns_list, new_columns_list); diff --git a/programs/copier/ClusterCopierApp.cpp b/programs/copier/ClusterCopierApp.cpp index 52a37c75c72..5ab6064f0f3 100644 --- a/programs/copier/ClusterCopierApp.cpp +++ b/programs/copier/ClusterCopierApp.cpp @@ -94,7 +94,7 @@ void ClusterCopierApp::mainImpl() StatusFile status_file(process_path + "/status"); ThreadStatus thread_status; - auto log = &logger(); + auto * log = &logger(); LOG_INFO(log, "Starting clickhouse-copier (" << "id " << process_id << ", " << "host_id " << host_id << ", " diff --git a/programs/copier/Internals.cpp b/programs/copier/Internals.cpp index 545df2e779c..0613381a763 100644 --- a/programs/copier/Internals.cpp +++ b/programs/copier/Internals.cpp @@ -260,7 +260,7 @@ ShardPriority getReplicasPriority(const Cluster::Addresses & replicas, const std return res; res.is_remote = 1; - for (auto & replica : replicas) + for (const auto & replica : replicas) { if (isLocalAddress(DNSResolver::instance().resolveHost(replica.host_name))) { @@ -270,7 +270,7 @@ ShardPriority getReplicasPriority(const Cluster::Addresses & replicas, const std } res.hostname_difference = std::numeric_limits::max(); - for (auto & replica : replicas) + for (const auto & replica : replicas) { size_t difference = getHostNameDifference(local_hostname, replica.host_name); res.hostname_difference = std::min(difference, res.hostname_difference); diff --git a/programs/obfuscator/Obfuscator.cpp b/programs/obfuscator/Obfuscator.cpp index 8b5a8c73ca4..f3ac0549573 100644 --- a/programs/obfuscator/Obfuscator.cpp +++ b/programs/obfuscator/Obfuscator.cpp @@ -937,10 +937,10 @@ public: if (typeid_cast(&data_type)) return std::make_unique(seed); - if (auto type = typeid_cast(&data_type)) + if (const auto * type = typeid_cast(&data_type)) return std::make_unique(get(*type->getNestedType(), seed, markov_model_params)); - if (auto type = typeid_cast(&data_type)) + if (const auto * type = typeid_cast(&data_type)) return std::make_unique(get(*type->getNestedType(), seed, markov_model_params)); throw Exception("Unsupported data type", ErrorCodes::NOT_IMPLEMENTED); diff --git a/programs/server/HTTPHandler.cpp b/programs/server/HTTPHandler.cpp index bceeec306cf..701b5f7d735 100644 --- a/programs/server/HTTPHandler.cpp +++ b/programs/server/HTTPHandler.cpp @@ -195,7 +195,7 @@ void HTTPHandler::pushDelayedResults(Output & used_output) std::vector read_buffers; std::vector read_buffers_raw_ptr; - auto cascade_buffer = typeid_cast(used_output.out_maybe_delayed_and_compressed.get()); + auto * cascade_buffer = typeid_cast(used_output.out_maybe_delayed_and_compressed.get()); if (!cascade_buffer) throw Exception("Expected CascadeWriteBuffer", ErrorCodes::LOGICAL_ERROR); @@ -383,7 +383,7 @@ void HTTPHandler::processQuery( { auto push_memory_buffer_and_continue = [next_buffer = used_output.out_maybe_compressed] (const WriteBufferPtr & prev_buf) { - auto prev_memory_buffer = typeid_cast(prev_buf.get()); + auto * prev_memory_buffer = typeid_cast(prev_buf.get()); if (!prev_memory_buffer) throw Exception("Expected MemoryWriteBuffer", ErrorCodes::LOGICAL_ERROR); diff --git a/programs/server/HTTPHandlerFactory.cpp b/programs/server/HTTPHandlerFactory.cpp index 91cf9ddf25b..4caea1e92e8 100644 --- a/programs/server/HTTPHandlerFactory.cpp +++ b/programs/server/HTTPHandlerFactory.cpp @@ -28,7 +28,7 @@ HTTPRequestHandlerFactoryMain::HTTPRequestHandlerFactoryMain(const std::string & { } -Poco::Net::HTTPRequestHandler * HTTPRequestHandlerFactoryMain::createRequestHandler(const Poco::Net::HTTPServerRequest & request) // override +Poco::Net::HTTPRequestHandler * HTTPRequestHandlerFactoryMain::createRequestHandler(const Poco::Net::HTTPServerRequest & request) { LOG_TRACE(log, "HTTP Request for " << name << ". " << "Method: " << request.getMethod() @@ -40,7 +40,7 @@ Poco::Net::HTTPRequestHandler * HTTPRequestHandlerFactoryMain::createRequestHand for (auto & handler_factory : child_factories) { - auto handler = handler_factory->createRequestHandler(request); + auto * handler = handler_factory->createRequestHandler(request); if (handler != nullptr) return handler; } @@ -72,80 +72,96 @@ HTTPRequestHandlerFactoryMain::TThis * HTTPRequestHandlerFactoryMain::addHandler static inline auto createHandlersFactoryFromConfig(IServer & server, const std::string & name, const String & prefix) { - auto main_handler_factory = new HTTPRequestHandlerFactoryMain(name); + auto main_handler_factory = std::make_unique(name); - try + Poco::Util::AbstractConfiguration::Keys keys; + server.config().keys(prefix, keys); + + for (const auto & key : keys) { - Poco::Util::AbstractConfiguration::Keys keys; - server.config().keys(prefix, keys); + if (!startsWith(key, "rule")) + throw Exception("Unknown element in config: " + prefix + "." + key + ", must be 'rule'", ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); - for (const auto & key : keys) - { - if (!startsWith(key, "rule")) - throw Exception("Unknown element in config: " + prefix + "." + key + ", must be 'rule'", ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); + const auto & handler_type = server.config().getString(prefix + "." + key + ".handler.type", ""); - const auto & handler_type = server.config().getString(prefix + "." + key + ".handler.type", ""); - - if (handler_type == "static") - main_handler_factory->addHandler(createStaticHandlerFactory(server, prefix + "." + key)); - else if (handler_type == "dynamic_query_handler") - main_handler_factory->addHandler(createDynamicHandlerFactory(server, prefix + "." + key)); - else if (handler_type == "predefined_query_handler") - main_handler_factory->addHandler(createPredefinedHandlerFactory(server, prefix + "." + key)); - else if (handler_type.empty()) - throw Exception("Handler type in config is not specified here: " + - prefix + "." + key + ".handler.type", ErrorCodes::INVALID_CONFIG_PARAMETER); - else - throw Exception("Unknown handler type '" + handler_type +"' in config here: " + - prefix + "." + key + ".handler.type",ErrorCodes::INVALID_CONFIG_PARAMETER); - } - - return main_handler_factory; - } - catch (...) - { - delete main_handler_factory; - throw; + if (handler_type == "static") + main_handler_factory->addHandler(createStaticHandlerFactory(server, prefix + "." + key)); + else if (handler_type == "dynamic_query_handler") + main_handler_factory->addHandler(createDynamicHandlerFactory(server, prefix + "." + key)); + else if (handler_type == "predefined_query_handler") + main_handler_factory->addHandler(createPredefinedHandlerFactory(server, prefix + "." + key)); + else if (handler_type.empty()) + throw Exception("Handler type in config is not specified here: " + + prefix + "." + key + ".handler.type", ErrorCodes::INVALID_CONFIG_PARAMETER); + else + throw Exception("Unknown handler type '" + handler_type +"' in config here: " + + prefix + "." + key + ".handler.type",ErrorCodes::INVALID_CONFIG_PARAMETER); } + + return main_handler_factory.release(); } static const auto ping_response_expression = "Ok.\n"; static const auto root_response_expression = "config://http_server_default_response"; -static inline Poco::Net::HTTPRequestHandlerFactory * createHTTPHandlerFactory(IServer & server, const std::string & name, AsynchronousMetrics & async_metrics) +static inline Poco::Net::HTTPRequestHandlerFactory * createHTTPHandlerFactory( + IServer & server, const std::string & name, AsynchronousMetrics & async_metrics) { if (server.config().has("http_handlers")) return createHandlersFactoryFromConfig(server, name, "http_handlers"); else { - auto factory = (new HTTPRequestHandlerFactoryMain(name)) - ->addHandler((new HandlingRuleHTTPHandlerFactory(server, root_response_expression)) - ->attachStrictPath("/")->allowGetAndHeadRequest()) - ->addHandler((new HandlingRuleHTTPHandlerFactory(server, ping_response_expression)) - ->attachStrictPath("/ping")->allowGetAndHeadRequest()) - ->addHandler((new HandlingRuleHTTPHandlerFactory(server)) - ->attachNonStrictPath("/replicas_status")->allowGetAndHeadRequest()) - ->addHandler((new HandlingRuleHTTPHandlerFactory(server, "query"))->allowPostAndGetParamsRequest()); + auto factory = std::make_unique(name); + + auto root_handler = std::make_unique>(server, root_response_expression); + root_handler->attachStrictPath("/")->allowGetAndHeadRequest(); + factory->addHandler(root_handler.release()); + + auto ping_handler = std::make_unique>(server, ping_response_expression); + ping_handler->attachStrictPath("/ping")->allowGetAndHeadRequest(); + factory->addHandler(ping_handler.release()); + + auto replicas_status_handler = std::make_unique>(server); + replicas_status_handler->attachNonStrictPath("/replicas_status")->allowGetAndHeadRequest(); + factory->addHandler(replicas_status_handler.release()); + + auto query_handler = std::make_unique>(server, "query"); + query_handler->allowPostAndGetParamsRequest(); + factory->addHandler(query_handler.release()); if (server.config().has("prometheus") && server.config().getInt("prometheus.port", 0) == 0) - factory->addHandler((new HandlingRuleHTTPHandlerFactory( - server, PrometheusMetricsWriter(server.config(), "prometheus", async_metrics))) - ->attachStrictPath(server.config().getString("prometheus.endpoint", "/metrics"))->allowGetAndHeadRequest()); + { + auto prometheus_handler = std::make_unique>( + server, PrometheusMetricsWriter(server.config(), "prometheus", async_metrics)); + prometheus_handler->attachStrictPath(server.config().getString("prometheus.endpoint", "/metrics"))->allowGetAndHeadRequest(); + factory->addHandler(prometheus_handler.release()); + } - return factory; + return factory.release(); } } static inline Poco::Net::HTTPRequestHandlerFactory * createInterserverHTTPHandlerFactory(IServer & server, const std::string & name) { - return (new HTTPRequestHandlerFactoryMain(name)) - ->addHandler((new HandlingRuleHTTPHandlerFactory(server, root_response_expression)) - ->attachStrictPath("/")->allowGetAndHeadRequest()) - ->addHandler((new HandlingRuleHTTPHandlerFactory(server, ping_response_expression)) - ->attachStrictPath("/ping")->allowGetAndHeadRequest()) - ->addHandler((new HandlingRuleHTTPHandlerFactory(server)) - ->attachNonStrictPath("/replicas_status")->allowGetAndHeadRequest()) - ->addHandler((new HandlingRuleHTTPHandlerFactory(server))->allowPostAndGetParamsRequest()); + auto factory = std::make_unique(name); + + auto root_handler = std::make_unique>(server, root_response_expression); + root_handler->attachStrictPath("/")->allowGetAndHeadRequest(); + factory->addHandler(root_handler.release()); + + auto ping_handler = std::make_unique>(server, ping_response_expression); + ping_handler->attachStrictPath("/ping")->allowGetAndHeadRequest(); + factory->addHandler(ping_handler.release()); + + auto replicas_status_handler = std::make_unique>(server); + replicas_status_handler->attachNonStrictPath("/replicas_status")->allowGetAndHeadRequest(); + factory->addHandler(replicas_status_handler.release()); + + auto main_handler = std::make_unique>(server); + main_handler->allowPostAndGetParamsRequest(); + factory->addHandler(main_handler.release()); + + return factory.release(); } Poco::Net::HTTPRequestHandlerFactory * createHandlerFactory(IServer & server, AsynchronousMetrics & async_metrics, const std::string & name) @@ -155,9 +171,14 @@ Poco::Net::HTTPRequestHandlerFactory * createHandlerFactory(IServer & server, As else if (name == "InterserverIOHTTPHandler-factory" || name == "InterserverIOHTTPSHandler-factory") return createInterserverHTTPHandlerFactory(server, name); else if (name == "PrometheusHandler-factory") - return (new HTTPRequestHandlerFactoryMain(name))->addHandler((new HandlingRuleHTTPHandlerFactory( - server, PrometheusMetricsWriter(server.config(), "prometheus", async_metrics))) - ->attachStrictPath(server.config().getString("prometheus.endpoint", "/metrics"))->allowGetAndHeadRequest()); + { + auto factory = std::make_unique(name); + auto handler = std::make_unique>( + server, PrometheusMetricsWriter(server.config(), "prometheus", async_metrics)); + handler->attachStrictPath(server.config().getString("prometheus.endpoint", "/metrics"))->allowGetAndHeadRequest(); + factory->addHandler(handler.release()); + return factory.release(); + } throw Exception("LOGICAL ERROR: Unknown HTTP handler factory name.", ErrorCodes::LOGICAL_ERROR); } diff --git a/programs/server/ReplicasStatusHandler.cpp b/programs/server/ReplicasStatusHandler.cpp index 2f2aa5953b6..f2d1ffe2ee5 100644 --- a/programs/server/ReplicasStatusHandler.cpp +++ b/programs/server/ReplicasStatusHandler.cpp @@ -46,7 +46,7 @@ void ReplicasStatusHandler::handleRequest(Poco::Net::HTTPServerRequest & request for (auto iterator = db.second->getTablesIterator(); iterator->isValid(); iterator->next()) { - auto & table = iterator->table(); + const auto & table = iterator->table(); StorageReplicatedMergeTree * table_replicated = dynamic_cast(table.get()); if (!table_replicated) diff --git a/src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp b/src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp index 80e7e033525..2e71bc902e9 100644 --- a/src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp +++ b/src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp @@ -331,18 +331,13 @@ void ReplicatedMergeTreeQueue::updateTimesInZooKeeper( void ReplicatedMergeTreeQueue::removeProcessedEntry(zkutil::ZooKeeperPtr zookeeper, LogEntryPtr & entry) { - auto code = zookeeper->tryRemove(replica_path + "/queue/" + entry->znode_name); - - if (code) - LOG_ERROR(log, "Couldn't remove " << replica_path << "/queue/" << entry->znode_name << ": " - << zkutil::ZooKeeper::error2string(code) << ". This shouldn't happen often."); - std::optional min_unprocessed_insert_time_changed; std::optional max_processed_insert_time_changed; bool found = false; size_t queue_size = 0; + /// First remove from memory then from ZooKeeper { std::unique_lock lock(state_mutex); @@ -372,6 +367,11 @@ void ReplicatedMergeTreeQueue::removeProcessedEntry(zkutil::ZooKeeperPtr zookeep notifySubscribers(queue_size); + auto code = zookeeper->tryRemove(replica_path + "/queue/" + entry->znode_name); + if (code) + LOG_ERROR(log, "Couldn't remove " << replica_path << "/queue/" << entry->znode_name << ": " + << zkutil::ZooKeeper::error2string(code) << ". This shouldn't happen often."); + updateTimesInZooKeeper(zookeeper, min_unprocessed_insert_time_changed, max_processed_insert_time_changed); } diff --git a/utils/zookeeper-adjust-block-numbers-to-parts/main.cpp b/utils/zookeeper-adjust-block-numbers-to-parts/main.cpp index a896129f915..8550675cb9e 100644 --- a/utils/zookeeper-adjust-block-numbers-to-parts/main.cpp +++ b/utils/zookeeper-adjust-block-numbers-to-parts/main.cpp @@ -199,7 +199,7 @@ void setCurrentBlockNumber(zkutil::ZooKeeper & zk, const std::string & path, Int create_ephemeral_nodes(1); /// Firstly try to create just a single node. /// Create other nodes in batches of 50 nodes. - while (current_block_number + 50 <= new_current_block_number) + while (current_block_number + 50 <= new_current_block_number) // NOLINT: clang-tidy thinks that the loop is infinite create_ephemeral_nodes(50); create_ephemeral_nodes(new_current_block_number - current_block_number);