Vacuum compressed chunks missing stats #1804
Conversation
jgpruitt
force-pushed
the
john/chunks_to_vacuum
branch
from
09efb41
to
178a721
Compare
jgpruitt
force-pushed
the
john/chunks_to_vacuum
branch
2 times, most recently
from
1cb22b4
to
c05ec6e
Compare
pkg/vacuum/vacuum.go
Outdated
| // next, we look for compressed chunks that have never been vacuum and seem to be missing | ||
| // statistics. autovacuum will ignore these until they hit vacuum_freeze_min_age, so let's | ||
| // catch these early | ||
| for { |
There was a problem hiding this comment.
This for, I believe can starve out the loop above and vice-versa,.This should probably be one for loop that alternates between the two queries. Or maybe the loop above is more important and should starve-out this loop?
There was a problem hiding this comment.
agreed. I think the first loop is more important. I will limit the second for loop to 3 loops. If it finishes its work, the run ends. If it doesn't finish in 3 iterations, it will give the first loop another turn before continuing.
There was a problem hiding this comment.
take another look? @cevian
jgpruitt
force-pushed
the
john/chunks_to_vacuum
branch
4 times, most recently
from
9d6e6fe
to
5201ed5
Compare
jgpruitt
force-pushed
the
john/chunks_to_vacuum
branch
2 times, most recently
from
94a0812
to
2dcc607
Compare
jgpruitt
force-pushed
the
john/chunks_to_vacuum
branch
from
2dcc607
to
8c9ee22
Compare
jgpruitt
force-pushed
the
john/chunks_to_vacuum
branch
2 times, most recently
from
d13dfa5
to
d8ed2ca
Compare
pkg/vacuum/vacuum.go
Outdated
|
|
||
| // if this workload already finished, don't continue working it, work the other | ||
| if w.finished { | ||
| continue |
There was a problem hiding this comment.
doesn't this mean chunksMissingStats can indefinitely starve out chunksToFreeze after chunksToFreeze.finish becomes true?
We have seen instances in which compresses chunks are missing statistics. Autovacuum ignores tables that are missing statistics. Analyzing these tables does not help since we rarely modify chunks after they are compressed. Therefore, these chunks are ignored until they pass the vacuum_freeze_max_age. This can be bad for performance. So, the vacuum engine looks for these chunks and vacuums them. We only use one worker for these.
jgpruitt
force-pushed
the
john/chunks_to_vacuum
branch
from
d8ed2ca
to
644dc5c
Compare
| return | ||
| log.Error("msg", fmt.Sprintf("failed to list %s", w.name), "error", err) | ||
| w.stop = true | ||
| continue | ||
| } | ||
| tablesNeedingVacuum.Set(float64(len(chunks))) |
There was a problem hiding this comment.
shouldn't we add separate metrics as well to count the work for each type?
There was a problem hiding this comment.
yes. I'll do a follow-up PR
Description
We have seen instances in which compresses chunks are missing statistics. Autovacuum ignores tables that are missing statistics. Analyzing these tables does not help since we rarely modify chunks after they are compressed. Therefore, these chunks are ignored until they pass the vacuum_freeze_max_age. This can be bad for performance. So, the vacuum engine makes a second pass looking for these chunks and vacuums them. We only use one worker for these.
See also: timescale/promscale_extension#595
Merge requirements
Please take into account the following non-code changes that you may need to make with your PR: