feat: Write makeSnapshot telemetry to slog #6188
Conversation
gibson042
force-pushed
the
gibson-6164-snapshot-slog-telemetry
branch
2 times, most recently
from
d514ce5 to
2718bdb
Compare
|
@mhofman I've taken all your suggestions. PTAL. |
| @@ -231,12 +237,13 @@ export function makeSnapStore( | |||
| if (fileStat) { | |||
| return freeze(info); | |||
There was a problem hiding this comment.
Since we stated the file, should we include the compressedByteCount with a 0 or undefined compressSeconds ?
There was a problem hiding this comment.
Good point... I think we do want to emit fields consistently, including data types. I've opted to read compressedByteCount from stat info, use 0 for compressSeconds, and introduce a boolean newFile to support exclusion of those values for accurate statistics.
| async function atomicWrite(destFilePath, writeContents) { | ||
| // Atomicity requires remaining on the same filesystem, | ||
| // so we perform all operations in the destination directory. | ||
| const dir = resolve(destFilePath, '..'); |
There was a problem hiding this comment.
Why not use dirname?
| const dir = resolve(destFilePath, '..'); | |
| const dir = path.dirname(destFilePath); |
There was a problem hiding this comment.
resolve is an endowment to makeSnapStore, and I didn't want to require another one or risk conflict between it and the built-in path module (however unlikely that may be). But I've restored root-relative behavior to avoid that, and this should be refactored in the performance-improving followup anyway.
gibson042
force-pushed
the
gibson-6164-snapshot-slog-telemetry
branch
from
48e7fd5 to
fd1b27d
Compare
|
@mhofman Back to you. And should I start work on the performance improvements, or did you plan to take that on? |
…e vs. existing-file
It was missing a valid export in a dependency package. Ref 000f738
turadg
force-pushed
the
gibson-6164-snapshot-slog-telemetry
branch
from
3188a60 to
45bc40d
Compare
closes: #6164
Description
Endow SnapStore instances with a now function so
savecan return duration and size telemetry all the way up to VatKeepersaveSnapshotwhere it is written into the slog.Security Considerations
Filesystem access is generally more powerful than a timer, so this should be acceptable.
Documentation Considerations
n/a
Testing Considerations
I'm not actually testing slog contents, is that something we do?