Skip to content
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.

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

tapdb: fix performance of aggregate stats #1310

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tapdb/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const (
// daemon.
//
// NOTE: This MUST be updated when a new migration is added.
LatestMigrationVersion = 26
LatestMigrationVersion = 27
)

// MigrationTarget is a functional option that can be passed to applyMigrations
Expand Down
2 changes: 2 additions & 0 deletions tapdb/sqlc/migrations/000027_better_universe_stats.down.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- This file is empty on purpose. There is nothing to roll back for this
-- migration.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we not want to restore the old universe_view in this rollback?

37 changes: 37 additions & 0 deletions tapdb/sqlc/migrations/000027_better_universe_stats.up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
DROP VIEW universe_stats;

CREATE VIEW universe_stats AS
WITH sync_counts AS (
SELECT universe_root_id, COUNT(*) AS count
FROM universe_events
WHERE event_type = 'SYNC'
GROUP BY universe_root_id
), proof_counts AS (
SELECT universe_root_id, event_type, COUNT(*) AS count
FROM universe_events
WHERE event_type = 'NEW_PROOF'
GROUP BY universe_root_id, event_type
), aggregated AS (
SELECT COALESCE(SUM(count), 0) as total_asset_syncs,
0 AS total_asset_proofs,
universe_root_id
FROM sync_counts
GROUP BY universe_root_id
UNION ALL
SELECT 0 AS total_asset_syncs,
COALESCE(SUM(count), 0) as total_asset_proofs,
universe_root_id
FROM proof_counts
GROUP BY universe_root_id
)
SELECT
SUM(ag.total_asset_syncs) AS total_asset_syncs,
SUM(ag.total_asset_proofs) AS total_asset_proofs,
roots.asset_id,
roots.group_key,
roots.proof_type
FROM aggregated ag
JOIN universe_roots roots
ON ag.universe_root_id = roots.id
GROUP BY roots.asset_id, roots.group_key, roots.proof_type
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we not tweak the index here like in the test?

		CREATE INDEX IF NOT EXISTS idx_universe_leaves_asset
		ON universe_leaves(
			asset_genesis_id, universe_root_id
		);`

Copy link
Contributor

Choose a reason for hiding this comment

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

Same with idx_mssmt_nodes_composite

ORDER BY roots.asset_id, roots.group_key, roots.proof_type;
6 changes: 3 additions & 3 deletions tapdb/universe_perf_long_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package tapdb
const longTestScale = 5

var (
numAssets = 100 * longTestScale
numLeavesPerTree = 300 * longTestScale
numQueries = 100 * longTestScale
numAssets = 100 * longTestScale
numLeavesPerTree = 300 * longTestScale
numQueries = 100 * longTestScale
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the test timeout be a bit longer for the _long_test.go or do you think the 5 minute timeout is ok?

)
Loading
Loading