Skip to content

Add balances to ACS snapshots - #2318

Merged
OriolMunoz-da merged 4 commits into
mainfrom
oriol/scan-snapshot-add-balances
Sep 19, 2025
Merged

Add balances to ACS snapshots#2318
OriolMunoz-da merged 4 commits into
mainfrom
oriol/scan-snapshot-add-balances

Conversation

@OriolMunoz-da

Copy link
Copy Markdown
Contributor

Part of #2254

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>

@meiersi-da meiersi-da left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work! Computing it jointly with the snapshot sounds great. Best chance to share the work.

@moritzkiefer-da moritzkiefer-da left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nice! Query plans and some timings on CILR would be great

@OriolMunoz-da

Copy link
Copy Markdown
Contributor Author

Here's a regular explain while I fail to run the analyze on cloudsql: https://www.pgexplain.dev/plan/f4527fda-e660-4941-840a-9f1f167cac21

@OriolMunoz-da

Copy link
Copy Markdown
Contributor Author

And here with explain analyze buffers:
https://www.pgexplain.dev/plan/1c247120-d537-436b-ab4b-3b60e09764c9#plan

image

^This part was pre-existing.

image

^For this:

  • The CTE Scan of inserted_rows includes the first screenshot, (i.e., 4m are included in 5m).
  • The CTE scan on creates_to_insert and the hash join are the new parts. It's... not great, but not the worst part of the query I guess

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>
Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>
@OriolMunoz-da
OriolMunoz-da marked this pull request as ready for review September 18, 2025 17:12

@moritzkiefer-da moritzkiefer-da left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nice lgtm! Should we extend the update plugin at the end of the test to check that the balance updates are identical? Thinking about this again, I guess you're not actually exposing it over endpoints yet so this would be a follow up.

@OriolMunoz-da

Copy link
Copy Markdown
Contributor Author

@moritzkiefer-da implemented in the last commit in #2334

@meiersi-da meiersi-da left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! Thanks a lot!

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>
@OriolMunoz-da
OriolMunoz-da enabled auto-merge (squash) September 19, 2025 07:26
@OriolMunoz-da
OriolMunoz-da merged commit 65eacc0 into main Sep 19, 2025
106 of 108 checks passed
@OriolMunoz-da
OriolMunoz-da deleted the oriol/scan-snapshot-add-balances branch September 19, 2025 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants