-
Notifications
You must be signed in to change notification settings - Fork 52
check_last_maintenance : filter out small tables #426
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
d634622
981678e
6bad3f3
0ad5044
8d9141b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -772,6 +772,11 @@ COMPATIBILITY | |
|
|
||
| You can use multiple "--exclude REGEX" parameters. | ||
|
|
||
| This service supports a "--analyze_table_min_size" parameter to | ||
| exclude small relations. The value must be a positive integer | ||
| representing the threshold in kilobytes below which the relation | ||
| is excluded. This defaults to 1024 (1MB). | ||
|
|
||
| Required privileges: unprivileged role able to log in all databases. | ||
|
|
||
| last_vacuum (8.2+) | ||
|
|
@@ -810,6 +815,11 @@ COMPATIBILITY | |
|
|
||
| You can use multiple "--exclude REGEX" parameters. | ||
|
|
||
| This service supports a "--vacuum_table_min_size" parameter to | ||
| exclude small relations. The value must be a positive integer | ||
| representing the threshold in kilobytes below which the relation | ||
| is excluded. This defaults to 10240 (10MB). | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as |
||
|
|
||
| Required privileges: unprivileged role able to log in all databases. | ||
|
|
||
| locks (all) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -503,6 +503,8 @@ my %args = ( | |
| 'wal_buffers' => undef, | ||
| 'checkpoint_segments' => undef, | ||
| 'effective_cache_size' => undef, | ||
| 'vacuum_table_min_size' => 10 * 1024, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 10 MB is already too much I think. Most small tables that gives false alarms are only a few blocks wide. We can discuss at length the right limit, but since we had service that was much too sensitive, perhaps shall we raise the bar not so high.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about 2 MB ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think 1000 tuples would be a better threshold ( A 2MB table should contains at least that number of tuples. |
||
| 'analyze_table_min_size' => 1024, | ||
| 'no_check_autovacuum' => 0, | ||
| 'no_check_fsync' => 0, | ||
| 'no_check_enable' => 0, | ||
|
|
@@ -5270,6 +5272,7 @@ sub check_last_maintenance { | |
| my @rs; | ||
| my $c_limit; | ||
| my $w_limit; | ||
| my $table_min_size; | ||
| my @perfdata; | ||
| my @msg_crit; | ||
| my @msg_warn; | ||
|
|
@@ -5285,6 +5288,8 @@ sub check_last_maintenance { | |
| my @dbexclude = @{ $args{'dbexclude'} }; | ||
| my $me = 'POSTGRES_LAST_' . uc($type); | ||
|
|
||
| $table_min_size = $args{$type . "_table_min_size"} * 1024; | ||
|
|
||
| # warning and critical are mandatory. | ||
| pod2usage( | ||
| -message => "FATAL: you must specify critical and warning thresholds.", | ||
|
|
@@ -5334,6 +5339,7 @@ sub check_last_maintenance { | |
| ||'$c_limit $w_limit')) | ||
| FROM pg_stat_all_tables a | ||
| WHERE schemaname NOT LIKE 'pg_temp_%' | ||
| AND pg_relation_size(relid) >= $table_min_size | ||
| AND schemaname NOT LIKE 'information_schema' | ||
| AND (('${type}' = 'analyze' AND schemaname NOT LIKE 'pg_toast') -- TOAST tables aren't ANALYZEd | ||
| OR ('${type}' = 'vacuum')) | ||
|
|
@@ -5364,6 +5370,7 @@ sub check_last_maintenance { | |
| FROM pg_stat_all_tables a | ||
| WHERE schemaname NOT LIKE 'pg_temp_%' | ||
| AND schemaname NOT LIKE 'pg_toast_temp_%' | ||
| AND pg_relation_size(relid) >= $table_min_size | ||
| AND schemaname NOT LIKE 'information_schema' | ||
| AND (('${type}' = 'analyze' AND schemaname NOT LIKE 'pg_toast') -- TOAST tables aren't ANALYZEd | ||
| OR ('${type}' = 'vacuum')) | ||
|
|
@@ -5396,6 +5403,7 @@ sub check_last_maintenance { | |
| FROM pg_stat_all_tables a | ||
| WHERE schemaname NOT LIKE 'pg_temp_%' | ||
| AND schemaname NOT LIKE 'pg_toast_temp_%' | ||
| AND pg_relation_size(relid) >= $table_min_size | ||
| AND schemaname NOT LIKE 'information_schema' | ||
| AND (('${type}' = 'analyze' AND schemaname NOT LIKE 'pg_toast') -- TOAST tables aren't ANALYZEd | ||
| OR ('${type}' = 'vacuum')) | ||
|
|
@@ -5438,6 +5446,7 @@ sub check_last_maintenance { | |
| JOIN pg_class b on a.relid = b.oid | ||
| WHERE schemaname NOT LIKE 'pg_temp_%' | ||
| AND schemaname NOT LIKE 'pg_toast_temp_%' | ||
| AND pg_relation_size(relid) >= $table_min_size | ||
| AND schemaname NOT LIKE 'information_schema' | ||
| AND (('${type}' = 'analyze' AND schemaname NOT LIKE 'pg_toast') -- TOAST tables aren't ANALYZEd | ||
| OR ('${type}' = 'vacuum')) | ||
|
|
@@ -10035,6 +10044,8 @@ GetOptions( | |
| 'dump-status-file!', | ||
| 'dump-bin-file:s', | ||
| 'effective_cache_size=i', | ||
| 'vacuum_table_min_size=i', | ||
| 'analyze_table_min_size=i', | ||
| 'exclude=s', | ||
| 'format|F=s', | ||
| 'global-pattern=s', | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,6 +50,7 @@ sleep(1); | |
|
|
||
| $node->command_checks_all( [ | ||
| './check_pgactivity', '--service' => 'last_analyze', | ||
| '--analyze_table_min_size' => 0, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should only use dash in a CLI option, so no underline (meaning --analyze-table-min-size). |
||
| '--username' => $ENV{'USER'} || 'postgres', | ||
| '--format' => 'human', | ||
| '--dbname' => 'testdb', | ||
|
|
@@ -106,6 +107,7 @@ SKIP: { | |
|
|
||
| $node->command_checks_all( [ | ||
| './check_pgactivity', '--service' => 'last_analyze', | ||
| '--analyze_table_min_size' => 0, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. |
||
| '--username' => $ENV{'USER'} || 'postgres', | ||
| '--format' => 'human', | ||
| '--dbname' => 'testdb', | ||
|
|
@@ -145,6 +147,7 @@ push @stdout, ( | |
|
|
||
| $node->command_checks_all( [ | ||
| './check_pgactivity', '--service' => 'last_analyze', | ||
| '--analyze_table_min_size' => 0, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, same everywhere :) |
||
| '--username' => $ENV{'USER'} || 'postgres', | ||
| '--format' => 'human', | ||
| '--dbname' => 'testdb', | ||
|
|
@@ -182,6 +185,7 @@ push @stdout, ( | |
|
|
||
| $node->command_checks_all( [ | ||
| './check_pgactivity', '--service' => 'last_analyze', | ||
| '--analyze_table_min_size' => 0, | ||
| '--username' => $ENV{'USER'} || 'postgres', | ||
| '--format' => 'human', | ||
| '--dbname' => 'testdb', | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should specify that we are talking about the "main fork" and maybe reflect
this in the parameter name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect units to work here.
I don't know if that's necessary, we will likely filter small table so the value should be easy to
read whthout it. But it would be consistent with other sizes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
main fork only? toast not included?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes because the toast table is processed separately by autovacuum. The main fork could be only 8kB, 100 tuples, never vacuumed, while the toast table can be 100MB, 50000 tuples, already vacuumed. If you use
pg_table_sizeinstead ofpg_relation_size, you will get an alert for the main fork, and we don't want that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 6bad3f3