Add trigger for periodic topology snapshots #3188

julientinguely-da · 2025-11-12T16:22:14Z

test bucket: https://console.cloud.google.com/storage/browser/da-splice-topology-snapshots;tab=objects?forceOnBucketsSortingFiltering=true&project=da-cn-splice&prefix=&forceOnObjectsSortingFiltering=false

The plan would be to activate it on sv1 in our production clusters (will be available from 0.5.2).

Remaining:

refactor code
add test
pulumi changes

Pull Request Checklist

Cluster Testing

If a cluster test is required, comment /cluster_test on this PR to request it, and ping someone with access to the DA-internal system to approve it.
If a hard-migration test is required (from the latest release), comment /hdm_test on this PR to request it, and ping someone with access to the DA-internal system to approve it.

PR Guidelines

Include any change that might be observable by our partners or affect their deployment in the release notes.
Specify fixed issues with Fixes #n, and mention issues worked on using #n
Include a screenshot for frontend-related PRs - see README or use your favorite screenshot tool

Merge Guidelines

Make the git commit message look sensible when squash-merging on GitHub (most likely: just copy your PR description).

Signed-off-by: Julien Tinguely <julien.tinguely@digitalasset.com>

Signed-off-by: Julien Tinguely <julien.tinguely@digitalasset.com> [ci] refactor Signed-off-by: Julien Tinguely <julien.tinguely@digitalasset.com>

Signed-off-by: Julien Tinguely <julien.tinguely@digitalasset.com>

martinflorian-da

Thanks for your patience! It's very nice work, thank you!

I did collect a fair number of comments though... Let's do another iteration once you had a chance to look at them.

martinflorian-da · 2025-11-18T10:38:55Z

cluster/pulumi/common/src/topology-snapshot.ts

+export async function topologySnapshotConfig(prefix: string): Promise<BackupConfig> {
+  const bucketSpec = await bootstrapDataBucketSpec(
+    config.optionalEnv('TOPOLOGY_SNAPSHOT_PROJECT') || 'da-cn-devnet',
+    config.optionalEnv('TOPOLOGY_SNAPSHOT_BUCKET') || 'da-cn-topology-snapshots'


Let's add these to the config schema! We want to avoid adding new env vars whenever it's reasonable...

martinflorian-da · 2025-11-18T10:39:40Z

cluster/pulumi/common/src/topology-snapshot.ts

+    config.optionalEnv('TOPOLOGY_SNAPSHOT_BUCKET') || 'da-cn-topology-snapshots'
+  );
+
+  return { backupInterval: '2h', location: { bucket: bucketSpec, prefix: prefix } };


Let's also make the backupInterval configurable in the config.yaml schema. Seems cheap enough to do, and if we need it we'll already have it.

Maybe also add a comment to that config option that it will produce a dump at most once per day, even if you set the interval to <24h

martinflorian-da · 2025-11-18T10:40:32Z

cluster/pulumi/common-sv/src/config.ts

  onboardingPollingInterval?: string;
  cometBftGovernanceKey?: CnInput<SvCometBftGovernanceKey>;
  initialRound?: string;
+  periodicTopologySnapshot?: boolean;


Doesn't the existence of a periodicTopologySnapshotConfig already imply true/false here? Why the extra variable?

martinflorian-da · 2025-11-18T10:44:26Z

cluster/helm/splice-sv-node/templates/sv.yaml

It looks right to me but I would probably have added a small Helm test for this. (Don't mind it if you think it's not necessary.)

martinflorian-da · 2025-11-18T10:47:48Z

cluster/deployment/config.yaml

        limits:
          memory: '18Gi'
+    svApp:
+      periodicTopologySnapshot: false


Suggestion: How about using this here for all defaults that you currently have in pulumi?

And making something like enabled one of the parameters of (what would have to become) something like periodicTopologySnapshotConfig.

Actually I think we can make this a bit more future-proof by having a config path like svApp.periodicSnapshots.topology. This way it's clear how to add ACS later on, for example.

martinflorian-da · 2025-11-18T10:58:27Z

...t/scala/org/lfdecentralizedtrust/splice/integration/tests/PeriodicTopologySnapshotTest.scala

+              logEntries,
+            )(logEntry =>
+              logEntry.message should (include(s"Took a new topology snapshot for $utcDate") or
+                include("Today's topology snapshot already exists."))


Hm so for identity dumps we seem to be fine with taking multiple per day - why limit ourselves on topology snapshots?

OK I found the explanation for this now on the issue...

Let's add a comment about our reasoning here though (to the trigger code).

martinflorian-da · 2025-11-18T10:59:49Z

...in/scala/org/lfdecentralizedtrust/splice/sv/automation/PeriodicTopologySnapshotTrigger.scala

+            BackupDump.write(
+              config.location,
+              Paths.get(s"$folderName/transactions-summary"),
+              summary.toString,


I wonder: Shall we also add the sequencer ID here? Might be informative, and that file is only for human consumption anyway.

Maybe also the specific timestamp of the dump? The bucket has a granularity of "days", might be nice to quickly see what was the time more specifically.

martinflorian-da · 2025-11-18T11:03:09Z

...in/scala/org/lfdecentralizedtrust/splice/sv/automation/PeriodicTopologySnapshotTrigger.scala

+      onboardingState <- sequencerAdminConnection.getOnboardingState(sequencerId)
+      authorizedStore <- sequencerAdminConnection.exportAuthorizedStoreSnapshot(sequencerId.uid)
+      // list a summary of the transactions state at the time of the snapshot to validate further imports
+      summary <- sequencerAdminConnection.getSummaryOfTransactions(


Don't you have a race here? You want the summary of transactions at exactly the time where the snapshots were taken.

Probably also want to add an appropriate TimeQuery the authorized store snapshot if possible.

Copy pasting what we did last time on the console:

import com.digitalasset.canton.data.CantonTimestamp val now = CantonTimestamp.now() val genesisState = sequencer.topology.transactions.genesis_stateV2(None, Some(now)) sequencer.topology.transactions.list(store = sequencer.synchronizer_id, proposals = false, timeQuery = TimeQuery.Snapshot(now)).result.groupMapReduce(_.mapping.code)(_ => 1)(_ + _) genesisState.writeTo(new java.io.FileOutputStream("/tmp/state-export/genesis-state"))

martinflorian-da · 2025-11-18T11:04:59Z

...in/scala/org/lfdecentralizedtrust/splice/sv/automation/PeriodicTopologySnapshotTrigger.scala

+  )(implicit traceContext: TraceContext): Future[TaskSuccess] =
+    for {
+      sequencerId <- sequencerAdminConnection.getSequencerId
+      onboardingState <- sequencerAdminConnection.getOnboardingState(sequencerId)


Huh don't we want getGenesisState here?

This mistake makes me think that you might want to test a topology import from the export that you're doing here; one time manually should be more than enough.

OK I read about it now on the issue...

So do you understand how to test an import from this? Would we also need keys from the sequencer (probably not?)?

And why do we actually need the authorized store? (I'd avoid dumping something we don't know anyone will use.)

Oh and aren't you missing this part of Moritz's design?

stream the snapshot so we never hold the full one in memory.

martinflorian-da · 2025-11-18T11:06:01Z

...in/scala/org/lfdecentralizedtrust/splice/sv/automation/PeriodicTopologySnapshotTrigger.scala

+            BackupDump.write(
+              config.location,
+              Paths.get(s"$folderName/genesis-state"),
+              onboardingState.toStringUtf8,


We can't just dump bytes to make it smaller and more easier to use/import?

julientinguely-da added 2 commits November 12, 2025 16:19

[static] init

33ad0f6

Signed-off-by: Julien Tinguely <julien.tinguely@digitalasset.com>

[ci] added integration test and summary for transactions

7a1cb7e

Signed-off-by: Julien Tinguely <julien.tinguely@digitalasset.com> [ci] refactor Signed-off-by: Julien Tinguely <julien.tinguely@digitalasset.com>

julientinguely-da force-pushed the julien/1133-periodic-snapshots-prod-cluster branch from e7fee87 to 7a1cb7e Compare November 14, 2025 14:31

[ci] added pulumi code and periodicTopologySnapshot boolean flag

f9d0edc

Signed-off-by: Julien Tinguely <julien.tinguely@digitalasset.com>

julientinguely-da marked this pull request as ready for review November 14, 2025 17:06

julientinguely-da requested review from martinflorian-da and nicu-da November 14, 2025 17:07

martinflorian-da approved these changes Nov 18, 2025

View reviewed changes

Add trigger for periodic topology snapshots #3188

Are you sure you want to change the base?

Add trigger for periodic topology snapshots #3188

Conversation

julientinguely-da commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Checklist

Cluster Testing

PR Guidelines

Merge Guidelines

Uh oh!

martinflorian-da left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martinflorian-da Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

julientinguely-da commented Nov 12, 2025 •

edited

Loading

martinflorian-da Nov 18, 2025 •

edited

Loading