roachprod: remove -v flag from scp #38890
Merged
Conversation
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
The flag was added in hopes of improving debugging. It only ever added noise.
|
This change is |
ajwerner
added a commit
to ajwerner/cockroach
that referenced
this pull request
Jul 16, 2019
Before the roachtest rewrite in cockroachdb#30977 we used to collect the logs directory from remote hosts unconditionally. We also used to write stats.json histogram files to the logs directory. This combination enabled the collection of histogram files for roachperf. Since the new roachtest only copies logs to the test runner in the case of failure, we have not gotten any perf artifacts since the change. This log policy is sane for logs and other debug artifacts like heap profiles that exist there but has been a bit of a bummer as we've missed out on the dopamine rush due to seeing charts go up and to the right in bespoke D3 glory. The truth is that we weren't all that excited about the previous behavior with regards to perf data either. Sometimes tests would fail and their lousy results would mess with our pretty pictures (https://github.com/cockroachdb/roachperf/issues/37). This PR introduces a new concept, perf artifacts, which live in a different directory, the perfArtifactsDir ("perf") and has the opposite fetching policy from the logs; the "perf" directory is copied from each remove host into the test's artifactsDir only upon successful completion of the test. One downside of this as implemented is that it can spam the test's log (but fortunately not roachtest's logs anymore, <3 @andreimatei). cockroachdb#38890 attempts to address this somewhat. It shouldn't been too big of a problem because it only happens on tests that already passed. This change also necessitates a small change to roachperf to look in this new "perf" directory (and respect the rewrite's test artifact directory structure). Fixes cockroachdb#38871. Release note: None
tbg
approved these changes
Jul 16, 2019
|
bors r=tbg |
craig bot
pushed a commit
that referenced
this pull request
Jul 16, 2019
38890: roachprod: remove -v flag from scp r=tbg a=ajwerner The flag was added in hopes of improving debugging. It only ever added noise. For example, with this flag we'll see: ``` stderr: stdout: 1: Requesting [email protected] debug1: Entering interactive session. debug1: pledge: network debug1: client_input_global_request: rtype [email protected] want_reply 0 debug1: Sending environment. debug1: Sending env LANG = en_US.UTF-8 debug1: Sending command: scp -v -r -f perf debug1: client_input_channel_req: channel 0 rtype exit-status reply 0 debug1: client_input_channel_req: channel 0 rtype [email protected] reply 0 debug1: channel 0: free: client-session, nchannels 1 Sink: \001scp: perf: No such file or directory scp: perf: No such file or directory debug1: fd 0 clearing O_NONBLOCK debug1: fd 2 clearing O_NONBLOCK Transferred: sent 2748, received 2856 bytes, in 0.2 seconds Bytes per second: sent 11097.8, received 11534.0 debug1: Exit status 1 debug1: compress outgoing: raw data 177, compressed 156, factor 0.88 debug1: compress incoming: raw data 1050, compressed 1029, factor 0.98 : exit status 1 ``` Which obfuscates the only really interesting error: ``` scp: perf: No such file or directory ``` Co-authored-by: Andrew Werner <[email protected]>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The flag was added in hopes of improving debugging. It only ever added noise.
For example, with this flag we'll see:
Which obfuscates the only really interesting error: