vfs: capture operation type affected by disk slowness #2255
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
|
Since this change was mostly already reviewed by @sumeerbhola over at #1672, I have assigned Sumeer directly. Storage folks, let me know if this is still a better time to just assign storage team as per the recent discussion. |
joshimhoff
force-pushed
the
improve_write_stall_info_safe
branch
2 times, most recently
from
5164e72 to
5ace9bf
Compare
nicktrav
approved these changes
Jan 24, 2023
There was a problem hiding this comment.
Basically reviewing my own code ... it's been a while!
Reviewed 4 of 6 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @sumeerbhola)
joshimhoff
force-pushed
the
improve_write_stall_info_safe
branch
from
5ace9bf to
1de7da9
Compare
|
TTFR! |
Currently, if a Pebble DB is backed by `vfs.FS` that is wrapped with a `vfs.diskHealthCheckingFS`, the DB can be made aware of operations that are taking longer than some threshold. The current implementation does not make any distinction between the operation type (write, sync, etc.) that was observed as slow. Capture the type of operation being performed when emitting a disk slowness event.
joshimhoff
force-pushed
the
improve_write_stall_info_safe
branch
from
1de7da9 to
d9b570e
Compare
nicktrav
reviewed
Jan 24, 2023
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status:
This was referenced Jan 25, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of cockroachdb/cockroach#67856.
This is #1672 from @nicktrav, rebased on top of #1677 and extended so that the op type of filesystem metadata operations is also captured. I also addressed @sumeerbhola's last comments over at #1672.
With this one merged, I still have more thing to do. I will extend
DiskSlowInfoto track write size in case a sized write (e.g. a call toWrite).As an aside, some discussion was had at https://cockroachlabs.slack.com/archives/CAC6K3SLU/p1674138778490679 about moving from a single long-lived monitor goroutine per file to a shorter-lived monitor goroutine per file op. Would mean no shared mutable state and no packed int64. We prefer the existing long-lived monitor goroutine per file, as the perf impacts of the suggested change may be problematic (it may also be hard to observe these bad impacts via
pebble workload, etc.).vfs: capture operation type affected by disk slowness
Currently, if a Pebble DB is backed by
vfs.FSthat is wrapped with avfs.diskHealthCheckingFS, the DB can be made aware of operations that are taking longer than some threshold. The current implementation does not make any distinction between the operation type (write, sync, etc.) that was observed as slow.Capture the type of operation being performed when emitting a disk slowness event.