-
Notifications
You must be signed in to change notification settings - Fork 442
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
fix(test): storage scrubber should only log to stdout with info #9067
Conversation
5038 tests run: 4874 passed, 0 failed, 164 skipped (full report)Flaky tests (6)Postgres 17
Postgres 15
Postgres 14
Code coverage* (full report)
* collected from Rust tests only The comment gets automatically updated with the latest test results
1e0f817 at 2024-09-24T22:45:26.199Z :recycle: |
Example test report. It still has the same captured (note captured stderr and stdout files) AND forwarded logging (see test log). Overall it's more healthy to output two (stderr, "pipe" to test log) rather than three times (stderr, "pipe" to test log, write to special file which must've been ignored by allure report generation?). I do not understand why we need to pipe it to the test log in any case. We do not do this for pageserver, for example, or safekeepers, or postgres, or storage controller. |
Well, probably it's because scrubber is a foreground process that gets explicitly called in the code path. I'll see how to fully remove it... |
added a commit 0d1e1d3 |
0d1e1d3
to
002df5b
Compare
should finally work now (tried different combinations of these parameters 🤣) |
Signed-off-by: Alex Chi Z <[email protected]>
Signed-off-by: Alex Chi Z <[email protected]>
Signed-off-by: Alex Chi Z <[email protected]>
Signed-off-by: Alex Chi Z <[email protected]>
4d6db72
to
1e0f817
Compare
Problem
As @koivunej mentioned in the storage channel, for regress test, we don't need to create a log file for the scrubber, and we should reduce noisy logs.
Summary of changes
Checklist before requesting a review
Checklist before merging