Add stats for active compactions #7281
Conversation
| statTSMOptimizeCompactions = "tsmOptimizeCompactions" | ||
| statTSMOptimizeCompactionsActive = "tsmOptimizeCompactionsActive" |
There was a problem hiding this comment.
Should we do something similar to the query executor for this? I remember for the query executor, we decided to add tracking how many compactions were run because it would end up being zero a lot. I know TSM compactions generally run faster so this might not be an issue for them, but this might run into the same issue where a compaction runs between monitor intervals and it never gets recorded.
Although we kept both for the query executor since one was more useful for SHOW STATS. Maybe we should have both?
There was a problem hiding this comment.
Some (many? most?) compactions will run in less than the 10-second period between writes to _internal, but we've seen systems under heavy load that have had compactions running for several minutes. The difficulty in determining the cause of the change in system load without an indicator of active compactions, was the driving motivation in adding a stat for active compactions.
What do you mean by "something similar to the query executor"? Separate counters for started vs finished?
There was a problem hiding this comment.
Yea, that's what I was thinking. Separate counters for started vs finished. This could be fine by itself though.
mark-rushakoff
force-pushed
the
mr-compaction-stats
branch
from
be5a5de
to
930daeb
Compare
|
@jsternberg I moved all the logic around executing a compaction into a |
mark-rushakoff
force-pushed
the
mr-compaction-stats
branch
from
930daeb
to
4ea4485
Compare
|
|
||
| if err == errCompactionInProgress { | ||
| time.Sleep(time.Second) |
There was a problem hiding this comment.
@jwilder it seems like every error case has a time.Sleep - but errCompactionsDisabled seems like the only error case where we don't sleep.
Do you think it would be okay to remove the sleep calls from compactGroup and unconditionally sleep inside compactTSMLevel and compactTSMFull?
| TSMFullCompactions int64 | ||
| TSMFullCompactionErrors int64 | ||
| TSMFullCompactionDuration int64 | ||
| CacheCompactions int64 // Counter of cache compactions that have ever run. |
mark-rushakoff
force-pushed
the
mr-compaction-stats
branch
2 times, most recently
from
2f7b08b
to
3573c83
Compare
mark-rushakoff
force-pushed
the
mr-compaction-stats
branch
2 times, most recently
from
c704140
to
882536f
Compare
|
@mark-rushakoff Looks good. Needs a rebase though. |
Unify logic around compaction execution to a single place. Also report on the error stats that we track. Previously they were not emitted in the stats output.
mark-rushakoff
force-pushed
the
mr-compaction-stats
branch
from
882536f
to
377c40f
Compare
Required for all non-trivial PRs
Adds stats to track the current number of active compactions. The existing stats for compactions were only updated after a compaction completed.
Also report on the error stats that we track. Previously they were not
emitted in the stats output.