DM-55013: Add support for configuring the replication system's services via a config file by iagaponenko · Pull Request #1083 · lsst/qserv

iagaponenko · 2026-08-24T02:32:19Z

No description provided.

Copilot

🟡 Changes recommended

Unresolved configuration precedence and database forwarding defects, plus critical runtime and synchronization findings, block approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This pull request migrates replication services to schema-driven Config objects and JSON configuration files.

Changes:

Adds role-specific configuration schemas, parsers, defaults, and test data.
Routes database, authentication, request, and service settings through Config.
Updates applications, service APIs, CLI templates, and deployment configuration.

File summaries

File	Change
`src/replica/worker/WorkerUtils.h`	Adds Config-aware database helpers.
`src/replica/worker/WorkerUtils.cc`	Uses configured worker database parameters.
`src/replica/worker/WorkerSqlRequest.h`	Updates request configuration documentation.
`src/replica/worker/WorkerSqlRequest.cc`	Uses configured worker database services.
`src/replica/worker/WorkerSqlHttpRequest.h`	Updates request configuration documentation.
`src/replica/worker/WorkerSqlHttpRequest.cc`	Uses configured database parameters.
`src/replica/worker/WorkerServerConnection.h`	Validates configured instance IDs.
`src/replica/worker/WorkerServerConnection.cc`	Migrates configuration includes.
`src/replica/worker/WorkerServer.h`	Updates configuration documentation.
`src/replica/worker/WorkerServer.cc`	Migrates configuration includes.
`src/replica/worker/WorkerRequest.h`	Updates timeout documentation.
`src/replica/worker/WorkerRequest.cc`	Uses the worker request timeout.
`src/replica/worker/WorkerReplicationRequest.h`	Updates request configuration documentation.
`src/replica/worker/WorkerReplicationRequest.cc`	Migrates configuration includes.
`src/replica/worker/WorkerProcessorThread.cc`	Migrates configuration exception handling.
`src/replica/worker/WorkerProcessor.h`	Updates processor configuration documentation.
`src/replica/worker/WorkerProcessor.cc`	Uses configured database settings and reloads.
`src/replica/worker/WorkerHttpSvcMod.cc`	Uses configured authentication and instance IDs.
`src/replica/worker/WorkerHttpSvc.cc`	Migrates configuration includes.
`src/replica/worker/WorkerHttpRequest.h`	Updates request configuration documentation.
`src/replica/worker/WorkerHttpRequest.cc`	Uses the worker request timeout.
`src/replica/worker/WorkerHttpProcessorThread.cc`	Migrates configuration exception handling.
`src/replica/worker/WorkerHttpProcessor.h`	Updates processor configuration documentation.
`src/replica/worker/WorkerHttpProcessor.cc`	Uses configured database settings and reloads.
`src/replica/worker/WorkerFindRequest.h`	Updates request configuration documentation.
`src/replica/worker/WorkerFindRequest.cc`	Uses Config for scanning and database creation.
`src/replica/worker/WorkerFindReplicaHttpRequest.h`	Updates request configuration documentation.
`src/replica/worker/WorkerFindReplicaHttpRequest.cc`	Adds Config-aware database creation.
`src/replica/worker/WorkerFindAllRequest.h`	Updates request configuration documentation.
`src/replica/worker/WorkerFindAllRequest.cc`	Uses Config for scanning and database creation.
`src/replica/worker/WorkerFindAllReplicasHttpRequest.h`	Updates request configuration documentation.
`src/replica/worker/WorkerFindAllReplicasHttpRequest.cc`	Adds Config-aware database creation.
`src/replica/worker/WorkerExporterHttpSvcMod.cc`	Uses configured authentication.
`src/replica/worker/WorkerExporterHttpSvc.cc`	Reports the configured instance ID.
`src/replica/worker/WorkerEchoRequest.h`	Updates request configuration documentation.
`src/replica/worker/WorkerEchoHttpRequest.h`	Updates request configuration documentation.
`src/replica/worker/WorkerDirectorIndexRequest.h`	Updates request configuration documentation.
`src/replica/worker/WorkerDirectorIndexRequest.cc`	Migrates configuration includes.
`src/replica/worker/WorkerDirectorIndexHttpRequest.h`	Updates request configuration documentation.
`src/replica/worker/WorkerDirectorIndexHttpRequest.cc`	Migrates configuration includes.
`src/replica/worker/WorkerDeleteRequest.h`	Updates request configuration documentation.
`src/replica/worker/WorkerDeleteRequest.cc`	Migrates configuration includes.
`src/replica/worker/WorkerDeleteReplicaHttpRequest.h`	Updates request configuration documentation.
`src/replica/worker/WorkerDeleteReplicaHttpRequest.cc`	Migrates configuration includes.
`src/replica/worker/WorkerCreateReplicaHttpRequest.h`	Updates request configuration documentation.
`src/replica/worker/WorkerCreateReplicaHttpRequest.cc`	Migrates configuration includes.
`src/replica/worker/FileServerConnection.h`	Updates file-service configuration documentation.
`src/replica/worker/FileServerConnection.cc`	Uses the configured instance ID.
`src/replica/worker/FileServer.h`	Updates file-service configuration documentation.
`src/replica/worker/FileServer.cc`	Migrates configuration includes.
`src/replica/worker/FileClient.h`	Adds Config declarations and documentation.
`src/replica/worker/FileClient.cc`	Uses the configured instance ID.
`src/replica/util/FileUtils.cc`	Migrates configuration includes.
`src/replica/services/DatabaseServicesPool.h`	Uses shared Config pointers.
`src/replica/services/DatabaseServicesPool.cc`	Constructs services from Config.
`src/replica/services/DatabaseServicesMySQL.h`	Stores Config and database-generator dependencies.
`src/replica/services/DatabaseServices.cc`	Creates MySQL services from Config.
`src/replica/services/ChunkMap.cc`	Uses configured Czar database parameters.
`src/replica/requests/StopRequest.cc`	Uses Config instance IDs and Controller messaging.
`src/replica/requests/StatusRequest.cc`	Uses Config instance IDs and Controller messaging.
`src/replica/requests/SqlRequest.cc`	Uses Config instance IDs and Controller messaging.
`src/replica/requests/ServiceManagementRequestBase.cc`	Uses Config instance IDs and Controller messaging.
`src/replica/requests/Request.h`	Updates request configuration documentation.
`src/replica/requests/Request.cc`	Uses controller request settings and messaging.
`src/replica/requests/ReplicationRequest.cc`	Uses Config instance IDs and Controller messaging.
`src/replica/requests/MessengerConnector.h`	Uses Config pointers and controller settings.
`src/replica/requests/MessengerConnector.cc`	Uses Config and retry settings.
`src/replica/requests/Messenger.h`	Uses Config pointers.
`src/replica/requests/Messenger.cc`	Constructs connectors from Config.
`src/replica/requests/FindRequest.cc`	Uses Config instance IDs and Controller messaging.
`src/replica/requests/FindAllRequest.cc`	Uses Config instance IDs and Controller messaging.
`src/replica/requests/EchoRequest.cc`	Uses Config instance IDs and Controller messaging.
`src/replica/requests/DisposeRequest.cc`	Uses Config instance IDs and Controller messaging.
`src/replica/requests/DirectorIndexRequest.cc`	Uses Config instance IDs and Controller messaging.
`src/replica/requests/DeleteRequest.cc`	Uses Config instance IDs and Controller messaging.
`src/replica/requests/CMakeLists.txt`	Updates request sources.
`src/replica/registry/RegistryHttpSvcMod.cc`	Uses configured authentication and instance IDs.
`src/replica/registry/RegistryHttpSvc.h`	Updates registry configuration documentation.
`src/replica/registry/RegistryHttpSvc.cc`	Reports the configured instance ID.
`src/replica/registry/Registry.cc`	Uses configured security and registry settings.
`src/replica/qserv/XrdCmsgetVnId.cc`	Parses worker database parameters directly.
`src/replica/qserv/TestEchoQservMgtRequest.h`	Updates request configuration documentation.
`src/replica/qserv/SetReplicasQservMgtRequest.h`	Updates request configuration documentation.
`src/replica/qserv/SetReplicasQservMgtRequest.cc`	Migrates configuration includes.
`src/replica/qserv/RemoveReplicaQservMgtRequest.h`	Updates request configuration documentation.
`src/replica/qserv/QservWorkerMgtRequest.cc`	Migrates configuration includes.
`src/replica/qserv/QservMgtRequest.h`	Updates request configuration documentation.
`src/replica/qserv/QservMgtRequest.cc`	Uses Config security and authentication settings.
`src/replica/qserv/QservCzarMgtRequest.cc`	Migrates configuration includes.
`src/replica/qserv/PostEventQservCzarMgtRequest.h`	Updates request configuration documentation.
`src/replica/qserv/GetStatusQservMgtRequest.h`	Updates request configuration documentation.
`src/replica/qserv/GetStatusQservCzarMgtRequest.h`	Updates request configuration documentation.
`src/replica/qserv/GetResultFilesQservMgtRequest.h`	Updates request configuration documentation.
`src/replica/qserv/GetReplicasQservMgtRequest.h`	Updates request configuration documentation.
`src/replica/qserv/GetReplicasQservMgtRequest.cc`	Migrates configuration includes.
`src/replica/qserv/GetQueryProgressQservCzarMgtRequest.h`	Updates request configuration documentation.
`src/replica/qserv/GetDbStatusQservMgtRequest.h`	Updates request configuration documentation.
`src/replica/qserv/GetConfigQservMgtRequest.h`	Updates request configuration documentation.
`src/replica/qserv/GetConfigQservCzarMgtRequest.h`	Updates request configuration documentation.
`src/replica/qserv/AddReplicaQservMgtRequest.h`	Updates request configuration documentation.
`src/replica/mysql/DatabaseMySQL.cc`	Adds configurable connection defaults.
`src/replica/jobs/SqlRowStatsJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlRowStatsJob.cc`	Migrates configuration includes.
`src/replica/jobs/SqlRemoveTablePartitionsJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlQueryJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlQueryJob.cc`	Migrates configuration includes.
`src/replica/jobs/SqlJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlJob.cc`	Uses controller request concurrency settings.
`src/replica/jobs/SqlGrantAccessJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlGrantAccessJob.cc`	Migrates configuration includes.
`src/replica/jobs/SqlGetIndexesJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlEnableDbJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlEnableDbJob.cc`	Migrates configuration includes.
`src/replica/jobs/SqlDropIndexesJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlDisableDbJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlDisableDbJob.cc`	Migrates configuration includes.
`src/replica/jobs/SqlDeleteTablePartitionJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlDeleteTablePartitionJob.cc`	Migrates configuration includes.
`src/replica/jobs/SqlDeleteTableJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlDeleteDbJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlDeleteDbJob.cc`	Migrates configuration includes.
`src/replica/jobs/SqlCreateTablesJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlCreateTableJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlCreateTableJob.cc`	Migrates configuration includes.
`src/replica/jobs/SqlCreateIndexesJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlCreateDbJob.h`	Updates job configuration documentation.
`src/replica/jobs/SqlCreateDbJob.cc`	Migrates configuration includes.
`src/replica/jobs/SqlAlterTablesJob.h`	Updates job configuration documentation.
`src/replica/jobs/ServiceManagementJob.h`	Uses Config declarations and documentation.
`src/replica/jobs/ServiceManagementJob.cc`	Updates configuration documentation.
`src/replica/jobs/ReplicateJob.cc`	Uses controller request concurrency settings.
`src/replica/jobs/RebalanceJob.h`	Updates job configuration documentation.
`src/replica/jobs/RebalanceJob.cc`	Uses controller request concurrency settings.
`src/replica/jobs/QservSyncJob.h`	Updates job configuration documentation.
`src/replica/jobs/QservSyncJob.cc`	Uses Config exceptions and Controller services.
`src/replica/jobs/QservStatusJob.h`	Updates job configuration documentation.
`src/replica/jobs/QservStatusJob.cc`	Uses Controller Qserv management services.
`src/replica/jobs/QservGetReplicasJob.cc`	Uses Controller Qserv management services.
`src/replica/jobs/PurgeJob.cc`	Uses controller request concurrency settings.
`src/replica/jobs/MoveReplicaJob.cc`	Migrates configuration includes.
`src/replica/jobs/Job.h`	Updates job configuration documentation.
`src/replica/jobs/Job.cc`	Migrates configuration includes.
`src/replica/jobs/FixUpJob.cc`	Uses controller request concurrency settings.
`src/replica/jobs/FindAllJob.cc`	Updates configuration documentation.
`src/replica/jobs/DirectorIndexJob.h`	Updates job configuration documentation.
`src/replica/jobs/DirectorIndexJob.cc`	Uses controller database and request settings.
`src/replica/jobs/DeleteWorkerJob.cc`	Updates configuration documentation.
`src/replica/jobs/DeleteReplicaJob.cc`	Uses Controller Qserv management services.
`src/replica/jobs/CreateReplicaJob.cc`	Uses Controller Qserv management services.
`src/replica/jobs/ClusterHealthJob.h`	Updates job configuration documentation.
`src/replica/jobs/ClusterHealthJob.cc`	Uses Controller Qserv management services.
`src/replica/jobs/AbortTransactionJob.h`	Updates job configuration documentation.
`src/replica/jobs/AbortTransactionJob.cc`	Migrates configuration includes.
`src/replica/ingest/IngestRequestMgr.h`	Updates ingest configuration documentation.
`src/replica/ingest/IngestRequest.h`	Updates ingest configuration documentation.
`src/replica/ingest/IngestRequest.cc`	Migrates configuration includes.
`src/replica/ingest/IngestHttpSvcMod.cc`	Uses configured authentication.
`src/replica/ingest/IngestHttpSvc.h`	Updates ingest configuration documentation.
`src/replica/ingest/IngestHttpSvc.cc`	Reports the configured instance ID.
`src/replica/ingest/IngestFileSvc.h`	Uses Config declarations.
`src/replica/ingest/IngestFileSvc.cc`	Uses configured worker database parameters.
`src/replica/ingest/IngestFileHttpSvcMod.cc`	Uses configured authentication.
`src/replica/ingest/IngestDataHttpSvcMod.cc`	Uses configured authentication.
`src/replica/contr/Task.h`	Moves synchronization settings into Config.
`src/replica/contr/Task.cc`	Reads synchronization settings from Config.
`src/replica/contr/ReplicationTask.h`	Simplifies task construction around Config.
`src/replica/contr/HttpWorkerStatusModule.h`	Removes obsolete processor configuration.
`src/replica/contr/HttpWorkerStatusModule.cc`	Removes obsolete processor configuration.
`src/replica/contr/HttpSqlSchemaModule.h`	Removes obsolete processor configuration.
`src/replica/contr/HttpSqlSchemaModule.cc`	Removes obsolete processor configuration.
`src/replica/contr/HttpSqlIndexModule.h`	Removes obsolete processor configuration.
`src/replica/contr/HttpSqlIndexModule.cc`	Removes obsolete processor configuration.
`src/replica/contr/HttpRequestsModule.h`	Removes obsolete processor configuration.
`src/replica/contr/HttpRequestsModule.cc`	Removes obsolete processor configuration.
`src/replica/contr/HttpReplicationLevelsModule.h`	Removes obsolete processor configuration.
`src/replica/contr/HttpReplicationLevelsModule.cc`	Removes obsolete processor configuration.
`src/replica/contr/HttpQservSqlModule.h`	Removes obsolete processor configuration.
`src/replica/contr/HttpQservSqlModule.cc`	Removes obsolete processor configuration.
`src/replica/contr/HttpQservMonitorModule.h`	Uses Config and Czar database settings.
`src/replica/contr/HttpProcessorConfig.h`	Removes the obsolete processor configuration type.
`src/replica/contr/HttpProcessor.h`	Uses Config-backed processor settings.
`src/replica/contr/HttpModule.h`	Uses Config-backed HTTP settings.
`src/replica/contr/HttpJobsModule.h`	Removes obsolete processor configuration.
`src/replica/contr/HttpJobsModule.cc`	Removes obsolete processor configuration.
`src/replica/contr/HttpIngestTransModule.h`	Removes obsolete processor configuration.
`src/replica/contr/HttpIngestTransModule.cc`	Removes obsolete processor configuration.
`src/replica/contr/HttpIngestModule.h`	Removes obsolete processor configuration.
`src/replica/contr/HttpIngestConfigModule.h`	Removes obsolete processor configuration.
`src/replica/contr/HttpIngestConfigModule.cc`	Removes obsolete processor configuration.
`src/replica/contr/HttpIngestChunksModule.h`	Removes obsolete processor configuration.
`src/replica/contr/HttpIngestChunksModule.cc`	Removes obsolete processor configuration.
`src/replica/contr/HttpExportModule.h`	Removes obsolete processor configuration.
`src/replica/contr/HttpExportModule.cc`	Uses Config-backed export settings.
`src/replica/contr/HttpDirectorIndexModule.h`	Removes obsolete processor configuration.
`src/replica/contr/HttpDirectorIndexModule.cc`	Uses Config-backed index settings.
`src/replica/contr/HttpControllersModule.h`	Removes obsolete processor configuration.
`src/replica/contr/HttpControllersModule.cc`	Removes obsolete processor configuration.
`src/replica/contr/HttpCatalogsModule.h`	Removes obsolete processor configuration.
`src/replica/contr/HttpCatalogsModule.cc`	Removes obsolete processor configuration.
`src/replica/contr/DeleteWorkerTask.h`	Moves eviction settings into Config.
`src/replica/contr/DeleteWorkerTask.cc`	Reads eviction settings from Config.
`src/replica/contr/Controller.h`	Adds controller-owned services and synchronization.
`src/replica/contr/Controller.cc`	Implements lazy controller services and shutdown.
`src/replica/contr/CMakeLists.txt`	Renames the HTTP configuration module source.
`src/replica/config/ConfigTestDataWorker.h`	Adds worker configuration test declarations.
`src/replica/config/ConfigTestDataRegistry.h`	Adds registry configuration test declarations.
`src/replica/config/ConfigTestDataRegistry.cc`	Defines registry configuration test data.
`src/replica/config/ConfigTestDataController.h`	Adds controller configuration test declarations.
`src/replica/config/ConfigSchemaWorker.h`	Adds the worker configuration schema.
`src/replica/config/ConfigSchemaRegistry.h`	Adds the registry configuration schema.
`src/replica/config/ConfigSchemaRegistry.cc`	Defines registry schema defaults.
`src/replica/config/ConfigSchemaController.h`	Adds the controller configuration schema.
`src/replica/config/ConfigParserMySQL.h`	Simplifies MySQL configuration parsing.
`src/replica/config/ConfigParserMySQL.cc`	Simplifies MySQL configuration parsing.
`src/replica/config/ConfigParserJSON.h`	Adds schema-aware JSON validation.
`src/replica/config/ConfigParserJSON.cc`	Implements schema-aware JSON parsing.
`src/replica/config/ConfigExceptions.h`	Adds Config-specific exceptions.
`src/replica/config/ConfigDatabase.cc`	Migrates configuration exception handling.
`src/replica/config/CMakeLists.txt`	Builds Config schemas and test data.
`src/replica/CMakeLists.txt`	Renames the configuration test target.
`src/replica/apps/WorkerApp.h`	Removes legacy worker CLI state.
`src/replica/apps/WorkerApp.cc`	Initializes the worker Config schema.
`src/replica/apps/TestAwsS3App.cc`	Updates the application constructor API.
`src/replica/apps/RegistryHttpApp.cc`	Initializes the registry Config schema.
`src/replica/apps/QservXrootdSsiApp.cc`	Updates the application constructor API.
`src/replica/apps/MasterControllerHttpApp.h`	Removes legacy controller CLI state.
`src/http/Auth.h`	Adds authentication-context comparisons.
`python/lsst/qserv/admin/cli/options.py`	Removes obsolete replication options.
`python/lsst/qserv/admin/cli/entrypoint.py`	Updates replication service command templates.
`deploy/compose/docker-compose.yml`	Supplies schema-based service configuration.

Review details

Suppressed comments (15)

src/replica/apps/Application.cc:124

The command-line option values are initialized from every schema default, and OptionParser leaves them unchanged when an option is absent. The later loop therefore writes those defaults back after config->update(_configFile), overwriting non-empty values supplied by the JSON config file before the service starts. Track which options were explicitly provided (or otherwise apply only explicit overrides) so the documented config-file precedence is preserved.

                if (!value.empty()) {
                    config->setFromString(category, param, value);

src/replica/apps/MasterControllerHttpApp.cc:86

Passing the schema here enables --config, but Application::run initializes every generated option to its schema default and reapplies all non-empty values after config->update(_configFile). Since omitted options retain those defaults, values from a JSON config file are overwritten before the service starts, so the new file-based configuration cannot take effect. Track explicitly supplied options (or apply CLI overrides before the file) in the shared application setup.

        : Application(argc, argv, ::description, ::enableServiceProvider, ConfigSchemaController()) {}

src/replica/apps/WorkerApp.cc:69

This constructor enables the new schema-driven configuration path, but the shared Application::run() logic initializes every general option to its schema default and then writes every non-empty _generalParams value after loading --config. As a result, non-empty values from the JSON file (for example database.worker-db-conn or security.instance-id) are immediately replaced by defaults unless they are also supplied on the command line, so the advertised config-file support is ineffective. Track whether an option was explicitly provided before applying it.

        : Application(argc, argv, ::description, ::enableServiceProvider, ConfigSchemaWorker()) {}

src/replica/config/ConfigSchema.cc:27

This file calls min(8, num_threads) in sharedSchema() but does not include <algorithm>, relying on transitive headers to declare std::min. Add the direct standard-library include so the renamed schema implementation does not depend on include order.

#include <stdexcept>
#include <thread>

src/replica/config/ConfigSchemaController.cc:27

This translation unit uses min(8, num_threads) below, but it does not include <algorithm>. It currently relies on a transitive include to declare std::min; include the header directly so this new schema implementation remains portable and does not fail when those transitive includes change.

#include <thread>

src/replica/config/ConfigTestDataRegistry.h:37

This documentation still names the old ConfigTestData class even though the declaration below is ConfigTestDataRegistry, which makes generated/API documentation and diagnostics misleading. Update the class name in the description.
src/replica/mysql/DatabaseMySQL.cc:88
These atomics are initialized from the Config only once during application startup. The HTTP configuration API can later change database.allow-reconnect, database.connect-timeout-sec, or database.max-reconnects, but Connection::open() and execute() now read the stale atomics, so subsequent connections and retries ignore those runtime configuration updates. Keep the connector defaults synchronized whenever Config changes, or read the current Config at these use sites.
src/replica/worker/WorkerDeleteRequest.h:60
This factory documentation still says a zero timeout is taken from the Controller-side request configuration, but the worker implementation now reads worker.request-timeout-sec (WorkerRequest.cc:81). Update the description to identify the worker-side timeout.
src/replica/worker/WorkerDirectorIndexRequest.h:69
This factory documentation still says a zero timeout is taken from the Controller-side request configuration, but the worker implementation now uses worker.request-timeout-sec (WorkerRequest.cc:81). Update the description to identify the worker-side timeout.
src/replica/worker/WorkerEchoRequest.h:60
This factory documentation still says a zero timeout is taken from the Controller-side request configuration, but the worker implementation now reads worker.request-timeout-sec (WorkerRequest.cc:81). Update the description to identify the worker-side timeout.
src/replica/worker/WorkerFindAllRequest.h:59
This factory documentation still says a zero timeout is taken from the Controller-side request configuration, but the worker implementation now reads worker.request-timeout-sec (WorkerRequest.cc:81). Update the description to identify the worker-side timeout.
src/replica/worker/WorkerFindRequest.h:63
This factory documentation still says a zero timeout is taken from the Controller-side request configuration, but the worker implementation now reads worker.request-timeout-sec (WorkerRequest.cc:81). Update the description to identify the worker-side timeout.
src/replica/worker/WorkerReplicationRequest.h:70
This factory documentation still says a zero timeout is taken from the Controller-side request configuration, but the worker implementation now reads worker.request-timeout-sec (WorkerRequest.cc:81). Update the description to identify the worker-side timeout.
src/replica/worker/WorkerRequest.h:209
This factory documentation still says a zero timeout is taken from the Controller-side request configuration, but the implementation now reads worker.request-timeout-sec (WorkerRequest.cc:81). Update the description to say the worker-side timeout so callers are not misled about which configuration controls expiration.
src/replica/worker/WorkerSqlRequest.h:72
This factory documentation still says a zero timeout is taken from the Controller-side request configuration, but the worker implementation now reads worker.request-timeout-sec (WorkerRequest.cc:81). Update the description to identify the worker-side timeout.

Files reviewed: 259/259 changed files
Comments generated: 4
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

fritzm

LGTM -- thanks!

The older name of the parameter "--config" was replaced with a more appropriate name "--repl-db". The new name is consistent with similar parameters where the connection URLs are provided. The names of the variables in the Replication Framework's code were adjusted accordingly from "configUrl" to "replDbUrl". The integration test was updated to follow the change. Another reason for this refactoring was to reuse the command line parameter "--config" for passing the INI-formatted configuration files into the applications.

The previous implementation was made the worker services depend on the default value of the timeout. This assumption was incorrect since the timeout could be explicitly changed in the Controller causing inconsistent expectations on the lifetime of the requests. The new parameter addresses this problem by allowing consistent configurations of these services in the Qserv deployments.

The parameter sets the number of requests sent per worker. In the older implementation the Controller was depending on the number of the service threads at the worker. The new parameter breaks the configuration dependency on the worker and allows more flexibility in configuring the Controller.

The older implementation had an incorrect dependency on the worker-specific parameter.

Each service of the Replication-Ingest system is now configured with its own set of the configuration parameters.

Pushed the Messenger and QservMgtServices services from the provider down to the Controller which is better scope for the service since it's only used by the worker requests initiated via the Controller. This change also make the provider more suitable to use in applications (worker and Registry) where no communications with workers are required.

Simplified implementation of the Controller class.

The obsolete parameters are no longer used by the Controller or the Registry

This change is meant to reduce impedance when reading and understanding the Replication/Ingest system's code.

In the older representaion, the schemas were constructed from the very large initialization lists. This approach was eror prone. It was hard to read (by a human), and it was fragile (for modifications). Besides, there was agreat deal of duplication in the parameter definitions. The new code addresses these problem.

Fixed a bug in the JSON config parser.

Simplified the entry points of the Replication system's services. Note that the Registry servic eno longer depends on any database, and the worker service doesn't need to be blocked externally to wait before the Replication database is initialized. The implementation (its C++ code) of the service already does the tracking.

This fix eliminates the race condition caused by the worker status propagation delay from the Registry to the Controller.

iagaponenko

@fritzm I have left a few comments on your commit

iagaponenko · 2026-10-09T23:30:56Z

+        "http-root": "/usr/local/qserv/www/"
+      },
+      "registry": {
+        "host": {{ printf "%s-registry" (include "qserv.name" .) | quote }}


You may also add the port:

"port": 25082

Do you think the host names should be injected via the config files rather than be passed via the command-line parameters?

My intuition is telling me that static parameters (known a priori) could be configured in the config files. And parameters which are linking servics to each other (hosts and probably ports) should be configured via the command line (injected by the entry point).

Otherwise, you would have to treat the JSON config as a template and get it processed.

Perhaps I'm wrong.

I went ahead and did this -- I agree that it separates "deployment plumbing" to the chart, and "operational config" in the config files, which is nice.

iagaponenko · 2026-10-09T23:34:49Z

+        "schema-upgrade-wait-timeout": 600
+      },
+      "registry": {
+        "host": {{ printf "%s-registry" (include "qserv.name" .) | quote }}


Registry port?

Added explicit ports in all three places (czar, workers, and the registry itself)

iagaponenko force-pushed the tickets/DM-55013 branch 7 times, most recently from f18bbc0 to 08d47e7 Compare August 29, 2026 00:01

iagaponenko force-pushed the tickets/DM-55013 branch 3 times, most recently from 94a7483 to 38f4543 Compare September 9, 2026 04:21

iagaponenko force-pushed the tickets/DM-55013 branch from 38f4543 to ff54924 Compare September 14, 2026 17:39

iagaponenko requested a lite review from Copilot September 16, 2026 18:05

Copilot started reviewing on behalf of iagaponenko September 16, 2026 18:06 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

Comment thread src/replica/config/ConfigSchema.cc Outdated

Comment thread src/replica/contr/Controller.cc

Comment thread python/lsst/qserv/admin/cli/entrypoint.py

Comment thread python/lsst/qserv/admin/cli/entrypoint.py

iagaponenko force-pushed the tickets/DM-55013 branch 9 times, most recently from feb06d9 to 215de73 Compare September 22, 2026 00:39

fritzm approved these changes Sep 22, 2026

View reviewed changes

iagaponenko force-pushed the tickets/DM-55013 branch 4 times, most recently from 7ed720f to df25dc4 Compare October 1, 2026 04:19

iagaponenko force-pushed the tickets/DM-55013 branch 2 times, most recently from 058e89c to 758bd9a Compare October 5, 2026 21:26

iagaponenko force-pushed the tickets/DM-55013 branch from 758bd9a to 51e4927 Compare October 6, 2026 00:12

iagaponenko changed the base branch from xrd to main October 6, 2026 00:14

iagaponenko force-pushed the tickets/DM-55013 branch 2 times, most recently from 3331bda to bfa78b2 Compare October 8, 2026 03:12

iagaponenko added 21 commits October 9, 2026 14:08

Minor refactoring and refinements in the application classes

35435b8

A config parameter for the Controller for ingesting director indexes

09bc8f0

The older implementation had an incorrect dependency on the worker-specific parameter.

Moved a config parameter into a more appropriate category

fd1cf8c

Separate configuration schemas for different applications

b233a7e

Each service of the Replication-Ingest system is now configured with its own set of the configuration parameters.

Migrated security parameters into the configuration service

aec6e67

Changed the initialization protocol for the configuration service

4aa51e7

Simplified implementation of the Controller class.

Refactored schema version tracking parameters

ca3115f

Fixed the configuration of the Replication database connector

57e43d8

Eliminated unused config parameters

f71df0b

The obsolete parameters are no longer used by the Controller or the Registry

Refined config parameters of the Replication/Ingest services

badb90a

Reorganized parameters of the database connectors

d237af7

Streamlined and unified the naming convention for the config classes

d8e5a3b

This change is meant to reduce impedance when reading and understanding the Replication/Ingest system's code.

Remove an obsolete class

ed13cd1

Allow configuring the Replication/Ingest system from JSON file

7b813be

Fixed a bug in the JSON config parser.

Reinforced the cluster healthcheck in the integration test

78b2f72

This fix eliminates the race condition caused by the worker status propagation delay from the Registry to the Controller.

iagaponenko force-pushed the tickets/DM-55013 branch from bfa78b2 to 78b2f72 Compare October 9, 2026 21:08

iagaponenko commented Oct 9, 2026

View reviewed changes

Adjust helm chart for repl/ingest configuration changes

f7e790b

fritzm force-pushed the tickets/DM-55013 branch from 5e9fbb5 to f7e790b Compare October 10, 2026 04:23

Navigation Menu

Repository navigation

DM-55013: Add support for configuring the replication system's services via a config file - #1083

DM-55013: Add support for configuring the replication system's services via a config file#1083
iagaponenko wants to merge 22 commits into
mainfrom
tickets/DM-55013

iagaponenko commented Aug 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fritzm left a comment

Uh oh!

iagaponenko left a comment

Uh oh!

iagaponenko Oct 9, 2026

Uh oh!

iagaponenko Oct 9, 2026

Uh oh!

fritzm Oct 10, 2026

Uh oh!

iagaponenko Oct 9, 2026

Uh oh!

fritzm Oct 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

iagaponenko commented Aug 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (15)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fritzm left a comment

Choose a reason for hiding this comment

Uh oh!

iagaponenko left a comment

Choose a reason for hiding this comment

Uh oh!

iagaponenko Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

iagaponenko Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

fritzm Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

iagaponenko Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

fritzm Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants