storage: correct terminology in log line for disk stalls#114746
storage: correct terminology in log line for disk stalls#114746craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
Previously, we'd use the term "file write stall" in the log
line for a disk stall, and not use the term "disk stall" which is
what metrics and documentation refers to this event as. Furthermore,
"write stalls" are a different, unrelated kind of stall in Pebble.
This change addresses this by changing the wording to refer
to this event as a "disk stall" in the log.
Epic: none
Release note (ops change): Updates the error message in case of stalled
disks to use the appropriate term for that event ("disk stall"), which
is also what metrics/dashboards refer to that event as.
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
jbowens
left a comment
There was a problem hiding this comment.
I think this language was deliberately changed to "file write stall" after we once saw a single large (megabytes) write take long enough to trigger the disk stall detector. I do think that that change confused more than it clarified, so this
With hindsight, I wish we named "write stalls" as "engine stalls," and "disk stalls" as "IO stalls."
Reviewable status:
complete! 1 of 0 LGTMs obtained
|
@jbowens ah okay, that's good to know. I didn't realize we've litigated this already, I just guessed it was a mistake from the last time we touched this code. The other option is "file write disk stall". I just want a more direct connection between this event and the term "disk stall" but it's not the most pressing thing either. I don't mind closing this PR |
|
@itsbilal Idk, I say merge it. It's probably most important that the end user understand which of the two concepts they're observing: a disk stall or a write stall. |
|
Sounds good. Thanks! bors r=jbowens |
|
Build succeeded: |
Previously, we'd use the term "file write stall" in the log line for a disk stall, and not use the term "disk stall" which is what metrics and documentation refers to this event as. Furthermore, "write stalls" are a different, unrelated kind of stall in Pebble.
This change addresses this by changing the wording to refer to this event as a "disk stall" in the log.
Epic: none
Release note (ops change): Updates the error message in case of stalled disks to use the appropriate term for that event ("disk stall"), which is also what metrics/dashboards refer to that event as.