Write per snapshot table by OriolMunoz-da · Pull Request #6742 · canton-network/splice

OriolMunoz-da · 2026-08-10T17:15:03Z

Part of #6263
Part of #6264 (only done here because otherwise the tests do nothing)

The interface of AcsSnapshotStore stays the same, we just have a new IncrementalAcsSnapshotTable that defines the behavior on insertion

TODOs (later PRs because this is big enough):

Intern strings
Create indexes on snapshot tables: Add ACS snapshot table indexes #6834 (draft)
Better naming?
Test copy tables performance on cilr
forceSnapshot behavior & enable by default in integration tests only
drop table name from migration

Signed-off-by: Oriol Muñoz <[email protected]>

OriolMunoz-da · 2026-08-11T09:43:37Z

@@ -1,3 +1,3 @@
 alter table acs_snapshot
  -- the name 'table_name' is not reserved, but it is a PostgreSQL keyword, so we use a different name to avoid confusion
  add column data_table_name text default null,


observation: this column is pointless:

You need a table name for both _stakeholders and _creates

The code can derive the table name from the snapshot record time anyway

any arguments for keeping this?

Argument for keeping it is that you can change the naming scheme in the future. IMO that's worth the slightly higher complexity.

Unrelated to that: since technically every snapshot table can have a different schema, do we want to include some schema version number column here?

Any time we change acs_snapshot_creates_template, we'd also change the scala constant used for populating the version column.

OriolMunoz-da · 2026-08-11T09:45:59Z

+    snapshot_id             bigint  not null references acs_incremental_snapshot (snapshot_id),
+    contract_id             text    not null,
+
+    -- All the data necessary to reconstruct a created event


we can't join with update_history_creates because of pruning

OriolMunoz-da · 2026-08-11T09:48:12Z

    )
  }
+
+  def assertFromString(s: String): PackageQualifiedName = {


mirrors QualifiedName's impl

OriolMunoz-da · 2026-08-11T09:48:36Z

+    if (storageConfig.perAcsSnapshotTablesEnabled) {
+      AcsSnapshotStore.IncrementalAcsSnapshotTable.NextV2
+    } else {
+      AcsSnapshotStore.IncrementalAcsSnapshotTable.Next
+    }


this will define the storage behavior. note that this is not done for backfilling, as we don't care what happens to those as they'll get pruned

OriolMunoz-da · 2026-08-11T09:52:06Z

+  override val nextTable: IncrementalAcsSnapshotTable = IncrementalAcsSnapshotTable.Next
+}
+
+class TablePerAcsSnapshotStoreTest extends AcsSnapshotStoreTest {


assumption: if the tests pass by replacing the nextTable, then the behavior is correct (insofar as the tests cover it, ofc)

OriolMunoz-da · 2026-08-11T09:55:17Z

          sql"where snapshot.row_id >= $firstRowId and snapshot.row_id <= $lastRowId"
        case Some(_: PerTableAcsSnapshot) =>
-          throw io.grpc.Status.UNIMPLEMENTED.withDescription("TODO #6264").asRuntimeException()
+          throw io.grpc.Status.UNIMPLEMENTED


this is actually called during the sanity check, but it uses the old (pre-incremental snapshot!) behavior, so it was already testing irrelevant code. TODO

OriolMunoz-da · 2026-08-11T09:55:36Z

        )
-      case _: PerTableAcsSnapshot =>
-        throw io.grpc.Status.UNIMPLEMENTED.withDescription("TODO #6263").asRuntimeException()
+      case table: PerTableAcsSnapshot =>


not sure this case can happen in prod, but tbf it's trivial to implement

Will this not happen all the time with pruning?

duh, yeah, I'd expect so. Meant more in the context of the existing code, not realizing this will most likely used by pruning too

OriolMunoz-da · 2026-08-11T09:56:11Z

+    }
+  }
+
+  private def querySnapshotInOwnTable(


clearly I don't know what to name things, so better ideas are more than welcome, for the whole "pertable" stuff in general

OriolMunoz-da · 2026-08-11T09:56:41Z

+        // If we enable PerTableAcsSnapshots, we will necessarily initialize from a LegacyAcsSnapshot,
+        // never from a PerTableAcsSnapshot. Then initializeIncrementalSnapshot will never be called again.


pls verify my hypothesis is correct

Will initializeIncrementalSnapshot ever be called at all? I would expect that by now all SVs have switched to incremental snapshots, so the whole code path for "copy a non-incremental snapshot to the incremental table" is dead. How about leaving the implementation as is, and create an issue for deleting all dead code?

this will execute at least once when we turn on per-table acs snapshots: the table acs_incremental_snapshot_data_next_v2 is empty so we need to initialize it from the last-existing snapshot

OriolMunoz-da · 2026-08-11T09:57:32Z

+      // This doesn't make much sense anymore
+      copiedCreateRows


this is used for metrics, what would we like here? i guess ideally track both tables? (legacy will fail to track one of them but that's fine)

Tracking both sounds good.

OriolMunoz-da · 2026-08-11T09:57:55Z

+        QueryParts.legacyCopyFromUpdateHistorySourceColumns
+    }
+
+    object QueryParts {


moved and added v2, sorry for the noise

OriolMunoz-da · 2026-08-11T09:58:23Z

      })
  }

+  object AcsTableDDL {


as mentioned in the migration: we don't need to keep the table name, lmk if you see an issue with that

Signed-off-by: Oriol Muñoz <[email protected]>

…e-per-snapshot-table-after-token [ci] Signed-off-by: Oriol Muñoz <[email protected]>

OriolMunoz-da · 2026-08-18T11:46:47Z

+      copiedCreateRows <- (sql"""
+        insert into #$createsTableName (contract_id, create_arguments, event_id, record_time, template_id_package_id, contract_key, created_at, signatories, observers, unlocked_amulet_balance, locked_amulet_balance)
+        select s.contract_id, s.create_arguments, s.event_id, s.record_time, s.template_id_package_id, s.contract_key, s.created_at, s.signatories, s.observers, """ ++ IncrementalAcsSnapshotTable.QueryParts
+        .unlockedAmuletBalance() ++ sql", " ++ IncrementalAcsSnapshotTable.QueryParts
+        .lockedAmuletBalance() ++ sql"""
+        from #${table.tableName} s
+        where s.snapshot_id = ${snapshot.snapshotId}
+        -- ensure consistent ordering across SVs
+        order by created_at, contract_id
+      """).toActionBuilder.asUpdate


using cilr's latest snapshot on sv-5:

scan_sv_5=> explain insert into oriol_acs_snapshot_creates_template (contract_id, create_arguments, event_id, record_time, template_id_package_id, contract_key, created_at, signatories, observers, unlocked_amulet_balance, locked_amulet_balance) select s.contract_id, s.create_arguments, s.event_id, s.record_time, s.template_id_package_id, s.contract_key, s.created_at, s.signatories, s.observers, s.unlocked_amulet_balance, s.locked_amulet_balance from oriol_acs_incremental_snapshot_data_next_v2 s where s.snapshot_id = 1 -- ensure consistent ordering across SVs order by created_at, contract_id; QUERY PLAN --------------------------------------------------------------------------------------------------------------------------------------------------------------------- Insert on oriol_acs_snapshot_creates_template (cost=0.43..356025.68 rows=0 width=0) -> Index Scan using acs_incremental_snapshot_data_next_v2_ca_ci on oriol_acs_incremental_snapshot_data_next_v2 s (cost=0.43..356025.68 rows=1484054 width=1300) Index Cond: (snapshot_id = 1) (3 rows) Time: 1.572 ms scan_sv_5=> insert into oriol_acs_snapshot_creates_template (contract_id, create_arguments, event_id, record_time, template_id_package_id, contract_key, created_at, signatories, observers, unlocked_amulet_balance, locked_amulet_balance) select s.contract_id, s.create_arguments, s.event_id, s.record_time, s.template_id_package_id, s.contract_key, s.created_at, s.signatories, s.observers, s.unlocked_amulet_balance, s.locked_amulet_balance from oriol_acs_incremental_snapshot_data_next_v2 s where s.snapshot_id = 1 -- ensure consistent ordering across SVs order by created_at, contract_id; INSERT 0 1483146 Time: 294432.204 ms (04:54.432)

~5m

Query plan looks good, but it's much slower than I hoped 😞

At https://docs.google.com/document/d/11BE-R2kVL3pnC20fDKWhzr-PN1iBf7Su-DtcmXUpUGk/edit?tab=t.0#bookmark=id.qk54mjxeefga, we've seen a dumb copy of 1.5M rows complete in 8sec.

Do you know if the difference is due to having to do an index scan (as opposed to a seq scan), or due to having much wider rows? Or perhaps a cold cache?

Can we run EXPLAIN (ANALYZE, BUFFERS) on the whole thing or just on the select part, to figure out what part is expensive?

Insert on oriol_acs_snapshot_creates_template (cost=0.43..356025.68 rows=0 wid th=0) (actual time=667449.982..667449.984 rows=0.00 loops=1) Buffers: shared hit=8255941 read=1215075 dirtied=325542 written=300217 I/O Timings: shared read=582711.099 write=9218.870 -> Index Scan using acs_incremental_snapshot_data_next_v2_ca_ci on oriol_acs _incremental_snapshot_data_next_v2 s (cost=0.43..356025.68 rows=1484054 width=1 300) (actual time=3.502..592369.286 rows=1483146.00 loops=1) Index Cond: (snapshot_id = 1) Index Searches: 1 Buffers: shared hit=322584 read=1205956 written=25 I/O Timings: shared read=580559.069 write=1.316 Planning: Buffers: shared hit=132 read=19 I/O Timings: shared read=6.269 Planning Time: 7.988 ms Execution Time: 667450.191 ms

OriolMunoz-da · 2026-08-18T11:48:09Z

+        cross join unnest(array_cat(s.observers, s.signatories)) as stakeholder
+        where s.snapshot_id = ${snapshot.snapshotId}
+        order by created_at, contract_id
+      """


scan_sv_5=> explain insert into oriol_acs_snapshot_stakeholders_template (stakeholder, template_id, contract_id) select stakeholder, concat(s.package_name, ':', s.template_id_module_name, ':', s.template_id_entity_name) as template_id, contract_id from oriol_acs_incremental_snapshot_data_next_v2 s cross join unnest(array_cat(s.observers, s.signatories)) as stakeholder where s.snapshot_id = 1 order by created_at, contract_id; QUERY PLAN -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Insert on oriol_acs_snapshot_stakeholders_template (cost=0.43..875444.59 rows=0 width=0) -> Subquery Scan on "*SELECT*" (cost=0.43..875444.59 rows=14840540 width=214) -> Nested Loop (cost=0.43..689937.84 rows=14840540 width=214) -> Index Scan using acs_incremental_snapshot_data_next_v2_ca_ci on oriol_acs_incremental_snapshot_data_next_v2 s (cost=0.43..356025.68 rows=1484054 width=464) Index Cond: (snapshot_id = 1) -> Function Scan on unnest stakeholder (cost=0.01..0.11 rows=10 width=32) (6 rows) Time: 10.872 ms scan_sv_5=> insert into oriol_acs_snapshot_stakeholders_template (stakeholder, template_id, contract_id) select stakeholder, concat(s.package_name, ':', s.template_id_module_name, ':', s.template_id_entity_name) as template_id, contract_id from oriol_acs_incremental_snapshot_data_next_v2 s cross join unnest(array_cat(s.observers, s.signatories)) as stakeholder where s.snapshot_id = 1 order by created_at, contract_id; INSERT 0 3824687 Time: 178101.374 ms (02:58.101)

OriolMunoz-da · 2026-08-18T11:48:56Z

+        where s.snapshot_id = ${snapshot.snapshotId}
+        order by created_at, contract_id
+      """
+      // TODO: we should create the necessary indexes


scan_sv_5=> create index on oriol_acs_snapshot_stakeholders_template (stakeholder, template_id, row_id); CREATE INDEX Time: 42237.803 ms (00:42.238) scan_sv_5=> create index on oriol_acs_snapshot_stakeholders_template (stakeholder, row_id); CREATE INDEX Time: 40198.560 ms (00:40.199)

faster than I expected

OriolMunoz-da · 2026-08-18T11:50:34Z

+      (unlocked_amulet_balance, locked_amulet_balance) <- sql"""
+        select
+            sum(s.unlocked_amulet_balance) AS unlocked_amulet_balance,
+            sum(s.locked_amulet_balance) AS locked_amulet_balance
+        from #${table.tableName} s
+        where snapshot_id = ${snapshot.snapshotId}
+      """.as[(BigDecimal, BigDecimal)].head


scan_sv_5=> explain select sum(s.unlocked_amulet_balance) AS unlocked_amulet_balance, sum(s.locked_amulet_balance) AS locked_amulet_balance from oriol_acs_incremental_snapshot_data_next_v2 s where snapshot_id = 1; QUERY PLAN ---------------------------------------------------------------------------------------------------------------------------------- Finalize Aggregate (cost=279980.47..279980.48 rows=1 width=64) -> Gather (cost=279980.23..279980.44 rows=2 width=64) Workers Planned: 2 -> Partial Aggregate (cost=278980.23..278980.24 rows=1 width=64) -> Parallel Seq Scan on oriol_acs_incremental_snapshot_data_next_v2 s (cost=0.00..275888.45 rows=618356 width=6) Filter: (snapshot_id = 1) (6 rows) scan_sv_5=> select sum(s.unlocked_amulet_balance) AS unlocked_amulet_balance, sum(s.locked_amulet_balance) AS locked_amulet_balance from oriol_acs_incremental_snapshot_data_next_v2 s where snapshot_id = 1; unlocked_amulet_balance | locked_amulet_balance -------------------------+----------------------- 0 | 0 (1 row) Time: 8763.449 ms (00:08.763)

…e-per-snapshot-table-after-token

Signed-off-by: Oriol Muñoz <[email protected]>

rautenrieth-da · 2026-08-19T08:32:56Z

@@ -1,3 +1,3 @@
 alter table acs_snapshot
  -- the name 'table_name' is not reserved, but it is a PostgreSQL keyword, so we use a different name to avoid confusion
  add column data_table_name text default null,


Argument for keeping it is that you can change the naming scheme in the future. IMO that's worth the slightly higher complexity.

rautenrieth-da · 2026-08-19T08:34:20Z

@@ -12,3 +12,80 @@ alter table acs_snapshot
    ((first_row_id is null and last_row_id is null and data_table_name is not null) or
    -- legacy table
    (first_row_id is not null and last_row_id is not null and data_table_name is null));
+
+-- TODO: template ids can be interned already


I assume this will be fixed in some upcoming PR before we merge the feature branch?

rautenrieth-da · 2026-08-19T08:38:23Z

+    -- column will be the same for all rows. However, in tests we can have multiple
+    -- scan instances writing to the same table, so we need to include it.


However, in tests we can have multiple scan instances writing to the same table, so we need to include it.

We could get rid of this column if we created this table on demand, and included the store id in the table name. Not worth the effort.

(disclaimer: the comment was copied from the incremental snapshots migration because the same logic applies)

rautenrieth-da · 2026-08-19T08:45:22Z

@@ -1,3 +1,3 @@
 alter table acs_snapshot
  -- the name 'table_name' is not reserved, but it is a PostgreSQL keyword, so we use a different name to avoid confusion
  add column data_table_name text default null,


Unrelated to that: since technically every snapshot table can have a different schema, do we want to include some schema version number column here?

Any time we change acs_snapshot_creates_template, we'd also change the scala constant used for populating the version column.

rautenrieth-da · 2026-08-19T09:17:48Z

+          templatesFilter(templates) ++
+          afterFilter ++ sql"""
+          order by s.row_id
+          limit ${sqlLimit(limit)}


Don't we need some group by contract_id here? This looks like it will return one row for each combination of stakeholder and contract id.

duh, of course.

note to self: a bunch of the tests use the old insertNewSnapshot which creates legacy snapshots so it's not testing anything

reflection of self: AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA

rautenrieth-da · 2026-08-19T09:27:48Z

+        // If we enable PerTableAcsSnapshots, we will necessarily initialize from a LegacyAcsSnapshot,
+        // never from a PerTableAcsSnapshot. Then initializeIncrementalSnapshot will never be called again.


Will initializeIncrementalSnapshot ever be called at all? I would expect that by now all SVs have switched to incremental snapshots, so the whole code path for "copy a non-incremental snapshot to the incremental table" is dead. How about leaving the implementation as is, and create an issue for deleting all dead code?

rautenrieth-da · 2026-08-19T09:38:20Z

+      copiedCreateRows <- (sql"""
+        insert into #$createsTableName (contract_id, create_arguments, event_id, record_time, template_id_package_id, contract_key, created_at, signatories, observers, unlocked_amulet_balance, locked_amulet_balance)
+        select s.contract_id, s.create_arguments, s.event_id, s.record_time, s.template_id_package_id, s.contract_key, s.created_at, s.signatories, s.observers, """ ++ IncrementalAcsSnapshotTable.QueryParts
+        .unlockedAmuletBalance() ++ sql", " ++ IncrementalAcsSnapshotTable.QueryParts
+        .lockedAmuletBalance() ++ sql"""
+        from #${table.tableName} s
+        where s.snapshot_id = ${snapshot.snapshotId}
+        -- ensure consistent ordering across SVs
+        order by created_at, contract_id
+      """).toActionBuilder.asUpdate


Query plan looks good, but it's much slower than I hoped 😞

At https://docs.google.com/document/d/11BE-R2kVL3pnC20fDKWhzr-PN1iBf7Su-DtcmXUpUGk/edit?tab=t.0#bookmark=id.qk54mjxeefga, we've seen a dumb copy of 1.5M rows complete in 8sec.

Do you know if the difference is due to having to do an index scan (as opposed to a seq scan), or due to having much wider rows? Or perhaps a cold cache?

Can we run EXPLAIN (ANALYZE, BUFFERS) on the whole thing or just on the select part, to figure out what part is expensive?

rautenrieth-da · 2026-08-19T09:49:42Z

+      // This doesn't make much sense anymore
+      copiedCreateRows


Tracking both sounds good.

rautenrieth-da · 2026-08-19T09:52:15Z

+        saveLegacyIncrementalSnapshotStatement(table, snapshot, nextSnapshotTargetRecordTime)
+    }
+    storage.queryAndUpdate(
+      withExclusiveSnapshotDataLock(


Suggested change

withExclusiveSnapshotDataLock(

withDdlStatement(

depending on whether we expect that create index X on A (SqlIndexInitializationTrigger) and create index Y on B (AcsSnapshotStore) can conflict on the catalog update.

Signed-off-by: Oriol Muñoz <[email protected]>

OriolMunoz-da added 5 commits August 6, 2026 08:45

tentative SQL

ecb2d53

Signed-off-by: Oriol Muñoz <[email protected]>

support ACS snapshots per table

e9f0e0b

Signed-off-by: Oriol Muñoz <[email protected]>

fixes

97cf5a2

Signed-off-by: Oriol Muñoz <[email protected]>

add v2 tests and get it to work [ci]

344def0

Signed-off-by: Oriol Muñoz <[email protected]>

[ci] refactor and remove TODOs

421f68c

Signed-off-by: Oriol Muñoz <[email protected]>

OriolMunoz-da commented Aug 11, 2026

View reviewed changes

OriolMunoz-da changed the title ~~[wip] e1.2 Write per snapshot table~~ Write per snapshot table Aug 11, 2026

OriolMunoz-da added 3 commits August 11, 2026 09:58

fix/add todos [ci]

cc61bf0

Signed-off-by: Oriol Muñoz <[email protected]>

[ci] scalafmt

d59c082

Signed-off-by: Oriol Muñoz <[email protected]>

[ci] prevent tablename collisions

5f4e43f

Signed-off-by: Oriol Muñoz <[email protected]>

OriolMunoz-da marked this pull request as ready for review August 11, 2026 10:03

OriolMunoz-da requested review from rautenrieth-da and ray-roestenburg-da August 11, 2026 10:03

Merge branch 'oriol/e1.1-acs-snapshot-migration' into oriol/e1.2-writ…

aebc071

…e-per-snapshot-table-after-token [ci] Signed-off-by: Oriol Muñoz <[email protected]>

martinflorian-da mentioned this pull request Aug 17, 2026

New store for unavailable parties #6725

Merged

8 tasks

OriolMunoz-da commented Aug 18, 2026

View reviewed changes

OriolMunoz-da added 2 commits August 18, 2026 14:14

Merge branch 'oriol/e1.1-acs-snapshot-migration' into oriol/e1.2-writ…

5f2e616

…e-per-snapshot-table-after-token

fix imports

d1a433d

Signed-off-by: Oriol Muñoz <[email protected]>

OriolMunoz-da mentioned this pull request Aug 18, 2026

Add ACS snapshot table indexes #6834

Draft

rautenrieth-da reviewed Aug 19, 2026

View reviewed changes

OriolMunoz-da added 5 commits August 19, 2026 11:08

ddl lock on drop

01572e7

Signed-off-by: Oriol Muñoz <[email protected]>

distinct on

0d6406f

Signed-off-by: Oriol Muñoz <[email protected]>

Move migration

73301af

Signed-off-by: Oriol Muñoz <[email protected]>

fixes

fa958f9

Signed-off-by: Oriol Muñoz <[email protected]>

properly fix the query which probably breaks indexes

8f9fb17

Signed-off-by: Oriol Muñoz <[email protected]>

OriolMunoz-da removed the request for review from ray-roestenburg-da August 19, 2026 14:39

whatever

d518185

Signed-off-by: Oriol Muñoz <[email protected]>

OriolMunoz-da marked this pull request as draft August 19, 2026 14:55

		// If we enable PerTableAcsSnapshots, we will necessarily initialize from a LegacyAcsSnapshot,
		// never from a PerTableAcsSnapshot. Then initializeIncrementalSnapshot will never be called again.

		-- column will be the same for all rows. However, in tests we can have multiple
		-- scan instances writing to the same table, so we need to include it.

Uh oh!

Conversation

OriolMunoz-da commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

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!

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!

OriolMunoz-da commented Aug 10, 2026 •

edited

Loading