Flush RRD only when TXGs contain data #18138
Conversation
oshogbo
force-pushed
the
oshogbo/flush_bad
branch
from
7382076 to
dca9998
Compare
amotin
left a comment
amotin
left a comment
There was a problem hiding this comment.
I am not 100% sure dp_dirty_pertxg at this point reliably means there is nothing to be written in this TXG. It may need a deeper look. But yea, this might be the direction.
| if (force || | ||
| (txg > spa->spa_last_noted_txg && | ||
| curtime >= spa->spa_last_noted_txg_time + spa_note_txg_time)) { |
There was a problem hiding this comment.
This condition does not look right. We should not care about force if we already logged the txg.
There was a problem hiding this comment.
I figured it’s better to log during export, but we can omit it if you prefer so.
There was a problem hiding this comment.
Again, this condition is wrong. I haven't looked whether on export txg can happen to be equal to spa->spa_last_noted_txg, but if it can, force will make it to insert a duplicate value. I suppose it should be:
txg > spa->spa_last_noted_txg && (force || curtime >= spa->spa_last_noted_txg_time + spa_note_txg_time).
This change modifies the behavior of spa_sync_time_logger when flushing the RRD database. Previously, once the sync interval elapsed, a flush would always be generated. On solid-state devices, especially when the pool was otherwise idle, this caused disks to wake up solely to write RRD data. Since RRD is best-effort telemetry, this behavior is unnecessary and wasteful. With this change, spa_sync_time_logger delays flushing until a TXG that already contains data is being synced. The RRD update is appended to that TXG instead of forcing the creation of a new write-only TXG. During pool export, flushing is forced regardless of whether the TXG contains user data. At that stage, data durability takes precedence and a write must be issued. Sponsored by: [Wasabi Technology, Inc.; Klara, Inc.] Signed-off-by: Mariusz Zaborski <mariusz.zaborski@klarasystems.com>
oshogbo
force-pushed
the
oshogbo/flush_bad
branch
from
dca9998 to
eda07a7
Compare
2 participants
Description
This change modifies the behavior of spa_sync_time_logger when flushing the RRD database.
Previously, once the sync interval elapsed, a flush would always be generated. On solid-state devices, especially when the pool was otherwise idle, this caused disks to wake up solely to write RRD data. Since RRD is best-effort telemetry, this behavior is unnecessary and wasteful.
With this change, spa_sync_time_logger delays flushing until a TXG that already contains data is being synced. The RRD update is appended to that TXG instead of forcing the creation of a new write-only TXG.
During pool export, flushing is forced regardless of whether the TXG contains user data. At that stage, data durability takes precedence and a write must be issued.
This fixes #18082
This change was inspired from @amotin in comments #18120.
Sponsored by: [Wasabi Technology, Inc.; Klara, Inc.]
How Has This Been Tested?
I have added logs to check when the database is flushed and what is the size of database.
Types of changes
Checklist:
Signed-off-by.