test: launch the sync operator app - #17
Conversation
36c2ce1 to
681ff2f
Compare
Adds the apps/syncoperator module: config, store, automation host and health, ingesting the MemberTraffic purchases the operator observes for its own synchronizer and holding the sequencer admin connection the reconciliation will grant on. Ingestion is deliberately not filtered by the node's own migration id, since a registered synchronizer is pinned to migration id 0. Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
Drop the synchronizer id and sequencer list from the config and take the synchronizer id from the sequencer instead. Share the MemberTraffic sum query with the DSO store. Also fixes the store test, which never ran: the operator was missing as an observer on the ingested contracts, and the suite was absent from the non-integration test list. Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
681ff2f to
75cbcb9
Compare
e1baa3a to
e8f3872
Compare
Add package_name to the sync_operator_acs_store index, matching the shape V049 rebuilt the dso and scan indexes into; the shared MemberTraffic query filters on it. Drop trafficBalanceReconciliationDelay, which nothing reads, and the scalapb runtime deps, which the module has no generated code for. Add the module to clean-splice. Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
Wires the app into apps-app (config, environment, console references, metrics, config transforms) so it can be configured and started like the other apps. Adds an integration test that starts it against a base topology, checks it takes its synchronizer id from the sequencer it is configured with, then stops and restarts it. The operator runs against the splitwell synchronizer's sequencer, which stands in for a dedicated synchronizer until the test topologies include one. Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
Splitwell and the validator both cover start and stop with a plain restart block and a separate liveness/readiness block, on an auto-started environment. Match that rather than hand-rolling a manual-start variant. Keeps one behaviour test of our own: that the operator takes its synchronizer id from the sequencer, which the log check cannot catch because a wrong id still starts cleanly. Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
Wait on the sync operator admin port in WaitForPorts, and bump both its participant and its sequencer admin API in bumpCantonPortsBy, so a test that composes the app with a port bump does not point at unbumped nodes. Move the admin port to 5115, alongside the other Splice app admin APIs, rather than the 57xx band that belongs to the splitwell Canton node. Note that the sequencer port is wall clock only. Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
6226693 to
5f1ff0f
Compare
| storage.config.properties.databaseName = "splice_apps" | ||
| instance-lock-enabled = false | ||
| admin-api.address = 0.0.0.0 | ||
| admin-api.port = 5115 |
There was a problem hiding this comment.
in apps/app/src/test/resources/README.md, looks like 5115 is reserved as API index 15. Doesnt seem like anything binds it today, but reservation gets consumed silently and registry not updated.
Maybe for Sync Operator Admin API, use
admin-api.port = 5116
plus a line in README.md: - 16: Sync Operator, Admin API
| .logical | ||
| syncOperatorBackend.appState.store.key.synchronizerId shouldBe served | ||
| } | ||
| } |
There was a problem hiding this comment.
Something that came up was that dont think that any of the three tests would fail if ACS ingestion were broken. They all pass as soon as the app has initialized.
The ingestion service does get started during initialize (SyncOperatorApp.scala:148 builds the automation service, and its superclass registers UpdateIngestionService in the constructor), so that part is fine. What nothing waits on is ingestion actually completing where DbMultiDomainAcsStore.finishedAcsIngestion is a separate promise that isHealthy never looks at.
So startSync(), httpLive, httpReady and the store.key.synchronizerId check would all still go green if the ingestion stream were stuck retrying, or if the operator party didn't have ledger read rights.
The store unit test doesn't cover it either, since SyncOperatorStoreTest constructs the store directly and drives testIngestionSink without a live participant.
I think one assertion closes the gap, since DbSyncOperatorStore.getTotalPurchasedMemberTraffic already wraps its query in waitUntilAcsIngested:
"ingest its ACS and answer traffic queries" in { implicit env =>
val member = splitwellValidatorBackend.participantClientWithAdminToken.id
syncOperatorBackend.appState.store
.getTotalPurchasedMemberTraffic(member)
.futureValue shouldBe 0L
}
The topology already gives you everything needed for it, and 0 is the expected answer since no MemberTraffic names this operator.
Looks like the first two tests are the same pair SplitwellIntegrationTest.scala:31-38 has, so you're following the existing pattern anyway so I may be wrong.
There was a problem hiding this comment.
I think I will just add all traffic related cases in a seperate PR
There was a problem hiding this comment.
I'm fine with leaving that for a separate PR
| storage.config.properties.databaseName = "splice_apps" | ||
| instance-lock-enabled = false | ||
| admin-api.address = 0.0.0.0 | ||
| admin-api.port = 5115 |
| .logical | ||
| syncOperatorBackend.appState.store.key.synchronizerId shouldBe served | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm fine with leaving that for a separate PR
#16 was squash-merged, so its commits are not ancestors of the base branch and this could not simply be retargeted. Merging brings in its last two commits, the init step wording and the pinned store migration id, neither of which this branch had. SyncOperatorApp and SyncOperatorAutomationService conflicted add/add for the same reason; both belong to #16 and are taken from the base unchanged. Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
Worked on PR #16, this PR aims to add a simple start and stop test for the sync operator app introduced on PR #16.
This PR doesn't launch any new dedicated syncronizer at all, rather uses the existing setup already in the ci. Proper setup will be introduced in future.