Skip to content

PMM-14566 Slower QAN v3 compared to v2. #4837

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

Open
JiriCtvrtka wants to merge 13 commits into
base: v3
Choose a base branch
from
Open

PMM-14566 Slower QAN v3 compared to v2. #4837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
d97115e
PMM-14566 Add indexes.
JiriCtvrtka Dec 12, 2025
e334f5f
PMM-14566 Apply index on history data.
JiriCtvrtka Dec 15, 2025
ddb0273
Merge branch 'v3' into PMM-14566-slow-QAN
JiriCtvrtka Dec 15, 2025
b7eee32
PMM-14566 Empty line.
JiriCtvrtka Dec 15, 2025
6d8c9c2
Merge branch 'v3' into PMM-14566-slow-QAN
JiriCtvrtka Dec 17, 2025
810daee
PMM-14566 Rename because of new migration merged.
JiriCtvrtka Dec 17, 2025
aacd067
Merge branch 'v3' into PMM-14566-slow-QAN
JiriCtvrtka Dec 17, 2025
2499029
Merge branch 'v3' into PMM-14566-slow-QAN
JiriCtvrtka Dec 19, 2025
2d4dd2e
Merge branch 'v3' into PMM-14566-slow-QAN
JiriCtvrtka Jan 5, 2026
07ff5eb
PMM-14566 Remove OPTIMIZE.
JiriCtvrtka Jan 5, 2026
4ccbb18
Merge branch 'v3' into PMM-14566-slow-QAN
JiriCtvrtka Jan 6, 2026
9319f79
Merge branch 'v3' into PMM-14566-slow-QAN
JiriCtvrtka Jan 6, 2026
2ddd390
Merge branch 'v3' into PMM-14566-slow-QAN
JiriCtvrtka Jan 9, 2026
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
1 change: 1 addition & 0 deletions qan-api2/migrations/sql/23_add_index_queryid.down.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE metrics DROP INDEX idx_metrics_queryid;
Copy link
Contributor

@maxkondr maxkondr Jan 6, 2026

Choose a reason for hiding this comment

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

Suggested change
ALTER TABLE metrics DROP INDEX idx_metrics_queryid;
ALTER TABLE metrics DROP INDEX IF EXISTS idx_metrics_queryid;

1 change: 1 addition & 0 deletions qan-api2/migrations/sql/23_add_index_queryid.up.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE metrics ADD INDEX idx_metrics_queryid queryid TYPE set(100);
Copy link
Member

@ademidoff ademidoff Dec 17, 2025

Choose a reason for hiding this comment

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

Any reason why we chose the specific number of granules for the index?

Copy link
Contributor Author

@JiriCtvrtka JiriCtvrtka Dec 17, 2025

Choose a reason for hiding this comment

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

If I understand correctly, higher granularity improves speed but increases memory consumption. In this case, there should be a balance between performance and memory consumption. Feel free to suggest a different value.

Copy link
Contributor

@maxkondr maxkondr Jan 6, 2026

Choose a reason for hiding this comment

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

Suggested change
ALTER TABLE metrics ADD INDEX idx_metrics_queryid queryid TYPE set(100);
ALTER TABLE metrics ADD INDEX IF NOT EXISTS idx_metrics_queryid queryid TYPE set(100);

Copy link
Contributor

@maxkondr maxkondr Jan 6, 2026

Choose a reason for hiding this comment

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

what about materialising the index?

Copy link
Contributor Author

@JiriCtvrtka JiriCtvrtka Jan 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Alex suggested to don’t apply the new indexes to existing data, since we currently can’t properly test the impact/safety of materializing them. See the discussion here: #4837 (comment)

Our data TTL is 30 days, but it looks like our query that drops old partitions doesn’t work reliably. I created a ticket for this: https://perconadev.atlassian.net/browse/PMM-14670

This definitely happens in HA, and it’s likely caused by our approach of taking the partition name, converting it to UInt32, and comparing it to a timestamp. I suspect this logic might be broken even without HA as well.

Because of that, my original idea was to run an OPTIMIZE to force merges (and indirectly rebuild indexes / clean up parts), see the suggested query here: 07ff5eb

but again, we cannot test it.

1 change: 1 addition & 0 deletions qan-api2/migrations/sql/24_add_index_period_start.down.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE metrics DROP INDEX idx_metrics_period_start;
Copy link
Contributor

@maxkondr maxkondr Jan 6, 2026

Choose a reason for hiding this comment

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

Suggested change
ALTER TABLE metrics DROP INDEX idx_metrics_period_start;
ALTER TABLE metrics DROP INDEX IF EXISTS idx_metrics_period_start;

1 change: 1 addition & 0 deletions qan-api2/migrations/sql/24_add_index_period_start.up.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE metrics ADD INDEX idx_metrics_period_start period_start TYPE minmax;
Copy link
Contributor

@maxkondr maxkondr Jan 6, 2026

Choose a reason for hiding this comment

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

Suggested change
ALTER TABLE metrics ADD INDEX idx_metrics_period_start period_start TYPE minmax;
ALTER TABLE metrics ADD INDEX IF NOT EXISTS idx_metrics_period_start period_start TYPE minmax;

Copy link
Contributor Author

@JiriCtvrtka JiriCtvrtka Jan 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

All migrations are applied from scratch, so it should not exists, but lets discuss this. Same for other migrations.

Copy link
Contributor

@maxkondr maxkondr Jan 7, 2026

Choose a reason for hiding this comment

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

this is always a good approach to make changes idempotent. It really costs nothing but prevents from possible errors

1 change: 1 addition & 0 deletions qan-api2/migrations/sql/25_add_index_service_id.down.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE metrics DROP INDEX idx_metrics_service_id;
Copy link
Contributor

@maxkondr maxkondr Jan 6, 2026

Choose a reason for hiding this comment

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

Suggested change
ALTER TABLE metrics DROP INDEX idx_metrics_service_id;
ALTER TABLE metrics DROP INDEX IF EXISTS idx_metrics_service_id;

1 change: 1 addition & 0 deletions qan-api2/migrations/sql/25_add_index_service_id.up.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE metrics ADD INDEX idx_metrics_service_id service_id TYPE set(100);
Copy link
Contributor

@maxkondr maxkondr Jan 6, 2026

Choose a reason for hiding this comment

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

Suggested change
ALTER TABLE metrics ADD INDEX idx_metrics_service_id service_id TYPE set(100);
ALTER TABLE metrics ADD INDEX IF NOT EXISTS idx_metrics_service_id service_id TYPE set(100);

Loading