Skip to content

[wait until 0.5.1] Disable UpdateHistory for SV and Splitwell apps #2675

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

[wait until 0.5.1] Disable UpdateHistory for SV and Splitwell apps #2675

wants to merge 13 commits into from

Conversation

@OriolMunoz-da
Copy link
Contributor

@OriolMunoz-da OriolMunoz-da commented Oct 13, 2025 •
edited
Loading

Fixes #1572

This PR removes the UpdateHistory from:

  • sv-app SvSvStore
  • sv-app SvDsoStore
  • SplitwellStore

Includes also:

  • Separate UpdateHistory from stores. The alternative seemed to involve cake pattern / visitor pattern / instance matching when registering the ingestion service. This seems way cleaner.
  • No more passing around Storage and throwing on every spot that can only work with DbStorage
  • No more Option[UpdateHistory] metrics, now that UpdateHistory is separate from stores
  • See also Make scanConfig mandatory in SV app #2686 for more annoyance

OriolMunoz-da
OriolMunoz-da commented Oct 14, 2025
View reviewed changes
...n/scala/org/lfdecentralizedtrust/splice/sv/automation/singlesv/SequencerPruningTrigger.scala
Comment on lines +93 to +95
scanConnection <- createScanConnection()
recordTimeRangeO <- scanConnection
.getMigrationInfo(migrationId)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pay attention to this change

@OriolMunoz-da
[ci] remove register duplicates
3aefc00 
Signed-off-by: Oriol Muñoz <[email protected]>
@OriolMunoz-da
[ci] DbStorage errywhere
695a66e 
Signed-off-by: Oriol Muñoz <[email protected]>
@OriolMunoz-da
[ci] Docs
c1a2fcf 
Signed-off-by: Oriol Muñoz <[email protected]>
@OriolMunoz-da
Merge branch 'main' into oriol/non-optional-metrics
2827e8b 
Signed-off-by: Oriol Muñoz <[email protected]>
@OriolMunoz-da
[ci] run pls gh
2ff977b 
Signed-off-by: Oriol Muñoz <[email protected]>
@OriolMunoz-da OriolMunoz-da marked this pull request as ready for review October 14, 2025 13:14
@OriolMunoz-da OriolMunoz-da changed the title Disable UpdateHistory for SV and Splitwell apps [wait until 0.4.21] Disable UpdateHistory for SV and Splitwell apps Oct 15, 2025
OriolMunoz-da
OriolMunoz-da commented Oct 15, 2025
View reviewed changes
...main/resources/db/migration/canton-network/postgres/stable/V047__truncate_sv_app_history.sql Outdated
from update_history_descriptors;

IF descriptors = '{"DbSvDsoStore", "DbSvSvStore"}' THEN
RAISE NOTICE 'Truncating update history tables as only SV app descriptors are present. Descriptors: %', descriptors::text;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fun fact, flyway logs these, very useful!

Image

OriolMunoz-da
OriolMunoz-da commented Oct 15, 2025
View reviewed changes
...main/resources/db/migration/canton-network/postgres/stable/V047__truncate_sv_app_history.sql Outdated
select array_agg(store_name order by store_name) into descriptors
from update_history_descriptors;

IF descriptors = '{"DbSvDsoStore", "DbSvSvStore"}' THEN
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more strict than it needs to be - some of the descriptors in the update_history_descriptors might not be used by the tables that we want to truncate.

Also, in case someone has a very stupid production setup where multiple apps write into the same database, this will fail to delete the unused data.

I still think it's fine as is, in real setups it should work and worst case it just leaves around unused data.

rautenrieth-da
rautenrieth-da approved these changes Oct 15, 2025
View reviewed changes
Copy link
Contributor

@rautenrieth-da rautenrieth-da left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Nice cleanup of the useless case dbStorage: DbStorage 🎉

...main/resources/db/migration/canton-network/postgres/stable/V047__truncate_sv_app_history.sql Outdated
select array_agg(store_name order by store_name) into descriptors
from update_history_descriptors;

IF descriptors = '{"DbSvDsoStore", "DbSvSvStore"}' THEN
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more strict than it needs to be - some of the descriptors in the update_history_descriptors might not be used by the tables that we want to truncate.

Also, in case someone has a very stupid production setup where multiple apps write into the same database, this will fail to delete the unused data.

I still think it's fine as is, in real setups it should work and worst case it just leaves around unused data.

...esources/db/migration/canton-network/postgres/stable/V048__truncate_sv_splitwell_history.sql
EXECUTE 'TRUNCATE TABLE update_history_assignments CASCADE';
EXECUTE 'TRUNCATE TABLE update_history_unassignments CASCADE';
EXECUTE 'TRUNCATE TABLE update_history_backfilling CASCADE';
EXECUTE 'TRUNCATE TABLE update_history_creates CASCADE';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: The acs_snapshot_data table has a foreign key constraint to update_history_creates. We don't need to truncate acs_snapshot_data because the SV app doesn't write any data into it (and we'd rather fail if there is any unexpected data in there after all).

@OriolMunoz-da OriolMunoz-da self-assigned this Oct 15, 2025
@martinflorian-da martinflorian-da mentioned this pull request Oct 16, 2025
Open
@OriolMunoz-da OriolMunoz-da changed the title [wait until 0.4.21] Disable UpdateHistory for SV and Splitwell apps [wait until 0.5.1] Disable UpdateHistory for SV and Splitwell apps Oct 20, 2025
@OriolMunoz-da
[ci] chekc update history of externals
3f938d6 
Signed-off-by: Oriol Muñoz <[email protected]>
@OriolMunoz-da
Make scanConfig mandatory in SV app (#2686)
6163ffd 
* Make scanConfig mandatory in SV app

[ci] let's see what breaks

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

* [ci] docs

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

* [ci] run pls gh

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

* update helm json schema

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

* [ci] release notes fix

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

---------

Signed-off-by: Oriol Muñoz <[email protected]>
ray-roestenburg-da
ray-roestenburg-da approved these changes Oct 20, 2025
View reviewed changes
Copy link
Contributor

@ray-roestenburg-da ray-roestenburg-da left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rautenrieth-da rautenrieth-da rautenrieth-da approved these changes

@ray-roestenburg-da ray-roestenburg-da ray-roestenburg-da approved these changes

Assignees

@OriolMunoz-da OriolMunoz-da

Labels

sprint2541

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove UpdateHistory from DbDsoStore

3 participants

@OriolMunoz-da @rautenrieth-da @ray-roestenburg-da