-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Snapshot restore progress #490
Merged
Merged
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
68f5bca
extract logger creation block of code
rboyer 193eb18
extract single snapshot restore method
rboyer 9ddd4e5
add progress restoration logger tooling
rboyer 4577041
thread a logger down into fsmRestoreAndMeasure
rboyer 9901a92
integrate progress reporting for the snapshot restore function
rboyer 03d8115
also check on times for snapshot transfer
rboyer 582f2a6
remove dead code
rboyer 3bd3071
add a simple test for the snapshot restoration progress
rboyer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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 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 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 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| package raft | ||
|
|
||
| import ( | ||
| "context" | ||
| "io" | ||
| "sync" | ||
| "time" | ||
|
|
||
| hclog "github.com/hashicorp/go-hclog" | ||
| ) | ||
|
|
||
| const ( | ||
| snapshotRestoreMonitorInterval = 10 * time.Second | ||
| ) | ||
|
|
||
| type snapshotRestoreMonitor struct { | ||
| logger hclog.Logger | ||
| cr CountingReader | ||
| size int64 | ||
| networkTransfer bool | ||
|
|
||
| once sync.Once | ||
| cancel func() | ||
| doneCh chan struct{} | ||
| } | ||
|
|
||
| func startSnapshotRestoreMonitor( | ||
| logger hclog.Logger, | ||
| cr CountingReader, | ||
| size int64, | ||
| networkTransfer bool, | ||
| ) *snapshotRestoreMonitor { | ||
| ctx, cancel := context.WithCancel(context.Background()) | ||
|
|
||
| m := &snapshotRestoreMonitor{ | ||
| logger: logger, | ||
| cr: cr, | ||
| size: size, | ||
| networkTransfer: networkTransfer, | ||
| cancel: cancel, | ||
| doneCh: make(chan struct{}), | ||
| } | ||
| go m.run(ctx) | ||
| return m | ||
| } | ||
|
|
||
| func (m *snapshotRestoreMonitor) run(ctx context.Context) { | ||
| defer close(m.doneCh) | ||
|
|
||
| ticker := time.NewTicker(snapshotRestoreMonitorInterval) | ||
| defer ticker.Stop() | ||
|
|
||
| ranOnce := false | ||
| for { | ||
| select { | ||
| case <-ctx.Done(): | ||
| if !ranOnce { | ||
| m.runOnce() | ||
| } | ||
| return | ||
| case <-ticker.C: | ||
| m.runOnce() | ||
| ranOnce = true | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func (m *snapshotRestoreMonitor) runOnce() { | ||
| readBytes := m.cr.Count() | ||
| pct := float64(100*readBytes) / float64(m.size) | ||
|
|
||
| message := "snapshot restore progress" | ||
| if m.networkTransfer { | ||
| message = "snapshot network transfer progress" | ||
| } | ||
|
|
||
| m.logger.Info(message, | ||
| "read-bytes", readBytes, | ||
| "percent-complete", hclog.Fmt("%0.2f%%", pct), | ||
| ) | ||
| } | ||
|
|
||
| func (m *snapshotRestoreMonitor) StopAndWait() { | ||
| m.once.Do(func() { | ||
| m.cancel() | ||
| <-m.doneCh | ||
| }) | ||
| } | ||
|
|
||
| func reportProgress( | ||
rboyer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ctx context.Context, | ||
| interval time.Duration, | ||
| reportOnceFn func(last bool), | ||
| ) <-chan struct{} { | ||
| done := make(chan struct{}) | ||
|
|
||
| go func() { | ||
| defer func() { | ||
| reportOnceFn(true) | ||
| close(done) | ||
| }() | ||
|
|
||
| ticker := time.NewTicker(interval) | ||
| defer ticker.Stop() | ||
|
|
||
| for { | ||
| select { | ||
| case <-ctx.Done(): | ||
| return | ||
| case <-ticker.C: | ||
| } | ||
|
|
||
| reportOnceFn(false) | ||
| } | ||
| }() | ||
|
|
||
| return done | ||
| } | ||
|
|
||
| type CountingReader interface { | ||
| io.Reader | ||
| Count() int64 | ||
| } | ||
|
|
||
| type countingReader struct { | ||
| reader io.Reader | ||
|
|
||
| mu sync.Mutex | ||
| bytes int64 | ||
| } | ||
|
|
||
| func (r *countingReader) Read(p []byte) (n int, err error) { | ||
| n, err = r.reader.Read(p) | ||
| r.mu.Lock() | ||
| r.bytes += int64(n) | ||
| r.mu.Unlock() | ||
| return n, err | ||
| } | ||
|
|
||
| func (r *countingReader) Count() int64 { | ||
| r.mu.Lock() | ||
| defer r.mu.Unlock() | ||
| return r.bytes | ||
| } | ||
|
|
||
| func newCountingReader(r io.Reader) *countingReader { | ||
| return &countingReader{reader: r} | ||
| } | ||
|
|
||
| type countingReadCloser struct { | ||
| *countingReader | ||
| io.Closer | ||
| } | ||
|
|
||
| func newCountingReadCloser(rc io.ReadCloser) *countingReadCloser { | ||
| return &countingReadCloser{ | ||
| countingReader: newCountingReader(rc), | ||
| Closer: rc, | ||
| } | ||
| } | ||
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldn't be better to make
tryRestoreSingleSnapshotreturn an error and create the logger outside of it and log the error when it's returned?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe? I wanted to have a derived logger here (
Logger.With()) so that all of the progress logs and the errors got the same set of bonus KV data logged.Given that, I'd end up creating the logger outside of this method to pass in for progress in
fsmRestoreAndMeasure, but returning an error that is immediately logged by the caller, which seemed differently strange.Also the two current logs are slightly different today:
and are carried over from the existing code. If these were changed to
fmt.Errorfreturn values there'd be a slight change of output as we'd end up having to dosnapLogger.Error(err.Error())instead, and we'd lose the "error" hclog attribute.I don't mind inverting that logic if you think it is warranted.