-
Notifications
You must be signed in to change notification settings - Fork 476
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
catchup: skip logging err for catchup cancellation #6053
Conversation
I'm not sure if we want to just drop these messages completely. The two contexts I see this firing for recently are:
|
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.
fetchAndWrite(%v) vs fetchAndWrite(%d) is inconsistent in this file, all need to be %d (or %v) eventually.
Please rebase/merge master - it as some flaky test fixes. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6053 +/- ##
==========================================
- Coverage 56.13% 56.11% -0.02%
==========================================
Files 488 488
Lines 69439 69439
==========================================
- Hits 38978 38965 -13
- Misses 27813 27821 +8
- Partials 2648 2653 +5 ☔ View full report in Codecov by Sentry. |
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.
LGTM, if you really want to see every error you can turn on debug-level logging. the ctx.Done() will occur when pipelinedFetch() finishes, so the reasons for it finishing would be logged inside pipelinedFetch.
Should logging err value here as we have no access to the original cause for abort.