From c9ab4047b4c598584c69fc59d989fd9d48899fff Mon Sep 17 00:00:00 2001 From: Igor Gaponenko Date: Sat, 21 Mar 2026 21:10:29 -0700 Subject: [PATCH 1/4] Eliminated obsolete code The eliminated code appears to be some left over experiment that was never finished, nor it matured into any reasonable idea. Another problem with the removed code was that it was making the package qana dependent on the Qserv frontend. This was not right from the dependency management standpoint. --- src/cconfig/CzarConfig.h | 4 ---- src/czar/Czar.cc | 1 - src/czar/Czar.h | 6 ------ src/qana/CMakeLists.txt | 1 - src/qana/ScanTablePlugin.cc | 13 +------------ 5 files changed, 1 insertion(+), 24 deletions(-) diff --git a/src/cconfig/CzarConfig.h b/src/cconfig/CzarConfig.h index 201a89602f..9b05bbe2ee 100644 --- a/src/cconfig/CzarConfig.h +++ b/src/cconfig/CzarConfig.h @@ -135,8 +135,6 @@ class CzarConfig { */ int getXrootdCBThreadsInit() const { return _xrootdCBThreadsInit->getVal(); } - bool getQueryDistributionTestVer() const { return _queryDistributionTestVer->getVal(); } - /* * @return A value of the "spread" parameter. This may improve a performance * of xrootd for catalogs with the large number of chunks. The default value @@ -364,8 +362,6 @@ class CzarConfig { util::ConfigValTInt::create(_configValMap, "tuning", "xrootdCBThreadsMax", notReq, 500); CVTIntPtr _xrootdCBThreadsInit = util::ConfigValTInt::create(_configValMap, "tuning", "xrootdCBThreadsInit", notReq, 50); - CVTIntPtr _queryDistributionTestVer = - util::ConfigValTInt::create(_configValMap, "tuning", "queryDistributionTestVer", notReq, 0); CVTBoolPtr _notifyWorkersOnQueryFinish = util::ConfigValTBool::create(_configValMap, "tuning", "notifyWorkersOnQueryFinish", notReq, 1); CVTBoolPtr _notifyWorkersOnCzarRestart = diff --git a/src/czar/Czar.cc b/src/czar/Czar.cc index bf44d539bb..a8d41f8098 100644 --- a/src/czar/Czar.cc +++ b/src/czar/Czar.cc @@ -152,7 +152,6 @@ Czar::Czar(string const& configFilePath, string const& czarName) int const xrootdSpread = _czarConfig->getXrootdSpread(); LOGS(_log, LOG_LVL_INFO, "config xrootdSpread=" << xrootdSpread); XrdSsiProviderClient->SetSpread(xrootdSpread); - _queryDistributionTestVer = _czarConfig->getQueryDistributionTestVer(); LOGS(_log, LOG_LVL_INFO, "Creating czar instance with name " << czarName); LOGS(_log, LOG_LVL_INFO, "Czar config: " << *_czarConfig); diff --git a/src/czar/Czar.h b/src/czar/Czar.h index 6c7917c776..e25bb0f833 100644 --- a/src/czar/Czar.h +++ b/src/czar/Czar.h @@ -113,10 +113,6 @@ class Czar { /// Return a pointer to QdispSharedResources qdisp::SharedResources::Ptr getQdispSharedResources() { return _qdispSharedResources; } - /// @return true if trivial queries should be treated as - /// interactive queries to stress test the czar. - bool getQueryDistributionTestVer() { return _queryDistributionTestVer; } - /// @param queryId The unique identifier of the previously submitted user query /// @return The reconstructed info for the query SubmitResult getQueryInfo(QueryId queryId) const; @@ -161,8 +157,6 @@ class Czar { /// and any other resources for use by query executives. qdisp::SharedResources::Ptr _qdispSharedResources; - bool _queryDistributionTestVer; ///< True if config says this is distribution test version. - /// Reloads the log configuration file on log config file change. std::shared_ptr _logFileMonitor; diff --git a/src/qana/CMakeLists.txt b/src/qana/CMakeLists.txt index d59c10bf0c..a49dd9e9bb 100644 --- a/src/qana/CMakeLists.txt +++ b/src/qana/CMakeLists.txt @@ -31,7 +31,6 @@ FUNCTION(qana_tests) target_link_libraries(${TEST} PUBLIC cconfig ccontrol - czar parser qana qdisp diff --git a/src/qana/ScanTablePlugin.cc b/src/qana/ScanTablePlugin.cc index 8c3fcde007..0b4b197f1d 100644 --- a/src/qana/ScanTablePlugin.cc +++ b/src/qana/ScanTablePlugin.cc @@ -40,7 +40,6 @@ #include "lsst/log/Log.h" // Qserv headers -#include "czar/Czar.h" #include "global/stringTypes.h" #include "proto/ScanTableInfo.h" #include "query/ColumnRef.h" @@ -173,18 +172,8 @@ proto::ScanInfo ScanTablePlugin::_findScanTables(query::SelectStmt& stmt, query: } } - auto czar = czar::Czar::getCzar(); - bool queryDistributionTestVer = (czar == nullptr) ? 0 : czar->getQueryDistributionTestVer(); - StringPairVector scanTables; - // Even trivial queries need to use full table scans to avoid crippling the czar - // with heaps of high priority, but very simple queries. Unless there are - // factors that greatly restrict how many chunks need to be read, it needs to be - // a table scan. - // For system testing, it is useful to see how the system handles large numbers - // trivial queries as the amount of work done by the workers is minimal. This - // highlights the cost of czar-worker communications. - if (!queryDistributionTestVer || hasSelectColumnRef) { + if (hasSelectColumnRef) { if (hasSecondaryKey) { LOGS(_log, LOG_LVL_TRACE, "**** Not a scan ****"); // Not a scan? Leave scanTables alone From 054ad2bcea84b58482768370aead2c67e599b1f1 Mon Sep 17 00:00:00 2001 From: Igor Gaponenko Date: Sat, 11 Apr 2026 21:06:44 -0700 Subject: [PATCH 2/4] Remove QuerySession::_isFinal This member has no actual effect on the query processing --- src/qproc/QuerySession.cc | 4 ---- src/qproc/QuerySession.h | 1 - 2 files changed, 5 deletions(-) diff --git a/src/qproc/QuerySession.cc b/src/qproc/QuerySession.cc index 875816da39..d6f06009ef 100644 --- a/src/qproc/QuerySession.cc +++ b/src/qproc/QuerySession.cc @@ -128,7 +128,6 @@ std::shared_ptr QuerySession::parseQuery(std::string const& s void QuerySession::analyzeQuery(std::string const& sql, std::shared_ptr const& stmt) { _original = sql; _stmt = stmt; - _isFinal = false; _initContext(); assert(_context.get()); @@ -288,9 +287,6 @@ std::shared_ptr QuerySession::getMergeStmt() const { } void QuerySession::finalize() { - if (_isFinal) { - return; - } QueryPluginPtrVector::iterator i; for (i = _plugins->begin(); i != _plugins->end(); ++i) { (**i).applyFinal(*_context); diff --git a/src/qproc/QuerySession.h b/src/qproc/QuerySession.h index 0b5c8ea7c5..32951002b2 100644 --- a/src/qproc/QuerySession.h +++ b/src/qproc/QuerySession.h @@ -272,7 +272,6 @@ class QuerySession { std::string _tmpTable; std::string _resultTable; std::string _error; - int _isFinal = 0; ///< Has query analysis/optimization completed? ChunkSpecVector _chunks; ///< Chunk coverage std::shared_ptr _plugins; ///< Analysis plugin chain From 1ccaa9caa338533e7d0f8128ac194ac75bd4e8e8 Mon Sep 17 00:00:00 2001 From: Igor Gaponenko Date: Sat, 11 Apr 2026 21:10:07 -0700 Subject: [PATCH 3/4] Remove unnecessary serialization at Czar during query submittion --- src/czar/Czar.cc | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/czar/Czar.cc b/src/czar/Czar.cc index a8d41f8098..19f8775b9a 100644 --- a/src/czar/Czar.cc +++ b/src/czar/Czar.cc @@ -222,13 +222,8 @@ SubmitResult Czar::submitQuery(string const& query, map const& h } // make new UserQuery - // this is atomic - ccontrol::UserQuery::Ptr uq; - { - lock_guard lock(_mutex); - uq = _uqFactory->newUserQuery(query, defaultDb, getQdispSharedResources(), userQueryId, msgTableName, - resultDb); - } + ccontrol::UserQuery::Ptr const uq = _uqFactory->newUserQuery(query, defaultDb, getQdispSharedResources(), + userQueryId, msgTableName, resultDb); // Add logging context with query ID QSERV_LOGCONTEXT_QUERY(uq->getQueryId()); From 3c7a874ef6d970a88552373da23c768bb0b87b81 Mon Sep 17 00:00:00 2001 From: Igor Gaponenko Date: Mon, 13 Apr 2026 11:02:07 -0700 Subject: [PATCH 4/4] Eliminated code duplication Also refactored to the code to evaluate the empty chunks list for queries which involve chunked tables. --- src/ccontrol/UserQuerySelect.cc | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/src/ccontrol/UserQuerySelect.cc b/src/ccontrol/UserQuerySelect.cc index 933096686f..9d8b42cf93 100644 --- a/src/ccontrol/UserQuerySelect.cc +++ b/src/ccontrol/UserQuerySelect.cc @@ -423,33 +423,26 @@ void UserQuerySelect::_expandSelectStarInMergeStatment(std::shared_ptrsaveResultQuery(_queryId, getResultQuery()); } void UserQuerySelect::_setupChunking() { - LOGS(_log, LOG_LVL_TRACE, "Setup chunking"); - std::shared_ptr im; - std::shared_ptr eSet = _qSession->getEmptyChunks(); - { - eSet = _qSession->getEmptyChunks(); - if (!eSet) { - eSet = std::make_shared(); - LOGS(_log, LOG_LVL_WARN, "Missing empty chunks info for dominantDbs"); - } - } - // FIXME add operator<< for QuerySession - LOGS(_log, LOG_LVL_TRACE, "_qSession: " << _qSession); + LOGS(_log, LOG_LVL_TRACE, "Setup chunking _qSession: " << _qSession); if (_qSession->hasChunks()) { auto areaRestrictors = _qSession->getAreaRestrictors(); auto secIdxRestrictors = _qSession->getSecIdxRestrictors(); css::StripingParams partStriping = _qSession->getDbStriping(); - - im = std::make_shared(partStriping, _secondaryIndex); + auto const im = std::make_shared(partStriping, _secondaryIndex); qproc::ChunkSpecVector csv; if (areaRestrictors != nullptr || secIdxRestrictors != nullptr) { csv = im->getChunks(areaRestrictors, secIdxRestrictors); } else { // Unrestricted: full-sky csv = im->getAllChunks(); } - LOGS(_log, LOG_LVL_TRACE, "Chunk specs: " << util::printable(csv)); + // Filter out empty chunks + std::shared_ptr eSet = _qSession->getEmptyChunks(); + if (!eSet) { + eSet = std::make_shared(); + LOGS(_log, LOG_LVL_WARN, "Missing empty chunks info for dominantDbs"); + } for (qproc::ChunkSpecVector::const_iterator i = csv.begin(), e = csv.end(); i != e; ++i) { if (eSet->count(i->chunkId) == 0) { // chunk not in empty? _qSession->addChunk(*i);