feat: sync operator app skeleton - #16
Conversation
sadiq1971
changed the title
dea0d60 to
bf74782
Compare
moritzkiefer-da
left a comment
moritzkiefer-da
left a comment
There was a problem hiding this comment.
thanks, this looks mostly reasonable. I left a few comments. I would also suggest adding an integration test directly in this PR that tests that you can at least start and stop the app successfully.
bc2b835 to
36c2ce1
Compare
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
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>
I have addressed the comments. For integration test I have added it on PR #17 to keep this PR clean and less changes. |
| } | ||
| // Only used to partition the store's ingestion offsets; purchases are ingested regardless | ||
| // of it, see SyncOperatorStore.contractFilter. | ||
| domainMigrationId <- appInitStep(s"Resolving domain migration id") { |
There was a problem hiding this comment.
I don't understand why you need this here, there is no migration id for dedicated synchronizers/it's always zero?
There was a problem hiding this comment.
I think it's different migration id. it's mainnet's migration id, not the dedicated one's, and the store re-ingests when it changes.
There was a problem hiding this comment.
mainnet migration id will never change again so I don't think we need this
There was a problem hiding this comment.
It was following what the sibling apps was doing - reading from scan. But if it doesn't change we may not need to fetch it anymore. But is it safe to set 0?
Drop the speculative "yet" from the domain time comment, and call the init step what the variable already calls it. Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
|
@moritzkiefer-da Can you check this CI issue? |
|
is this based on some pretty old branch? that should have been fixed ages ago |
Yeah- It's way behind the main splice repo. The main of the fork is also behind. But the ci issue is happening from tomorrow. |
Is it fine if I place PR for updating the repo main and also the current working branch for the phase 2 implementation? Also It might be better if we just not mainnet a different branch rather than main as it already is a fork. |
yep definitely |
MIGRATION_ID is frozen network wide and logical synchronizer upgrades carry a serial id instead, so the store's partition can never move and resolving it bought nothing. Drops resolveDomainMigrationId and its scan fallback. It also lines the store's stamp up with the payload.migrationId == 0L check in the contract filter, so there is no longer a second migration id a few lines away meaning something different. Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
3 participants
What this does
apps/syncoperatormodule, modelled onapps/splitwell: config, store, automation host, metrics, bootstrap, health.SyncOperatorStoreingestsMemberTrafficwhere this operator is the observer, and exposesgetTotalPurchasedMemberTraffic(member)off an indexed column. The query itself is now shared withDbSvDsoStoreviaMemberTrafficQueries.SyncOperatorAppconnects to Scan and its own sequencer, and takes its synchronizer id from that sequencer rather than from config, so there is nothing to keep in sync at bootstrap.sync_operator_acs_storetable and twoSpliceDbLockCounters.How it's verified
compile,Test/compileandscalafmtCheckAllpass across the tree.SyncOperatorStoreTestcovers the filter: sums across purchases, keeps members apart, and ignores purchases for another synchronizer, another operator, no operator, and a non-zero migration id. 7/7 pass locally against Postgres, and it is now listed intest-full-class-names-non-integration.logso CI runs it.Not covered: that the store survives a decentralized-synchronizer migration without dropping records. That is part of the acceptance for canton-network#35 and needs a fixture that changes the migration id mid-test.
Tracked in
Implements P2-E5.2. Unblocks P2-E5.3, which also needs P2-E5.1 (#15 here). Relies on the migration-id policy in P2-E1.5.