roachtest: exclude stats.json from artifacts.zip #125022
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
Upon test completion, the test runner invokes `zipArtifacts()` in order to reduce the footprint of all test-specific logs retained by TC. In case of a successful benchmark, performance artifacts (i.e., stats.json) are fetched. Historically, those do not get zipped. Instead, they are uploaded (as is) into GCS. In [1], a new pattern was introduced, wherein `stats.json` is written directly on the test runner node, as opposed to, on some remote (cluster) node. Since `zipArtifacts` runs _before_ fetching performance artifacts, `stats.json` is effectively evaporated; i.e., the CI script no longer finds it. This PR adds an exclusion filter to `zipArtifacts`, to ignore all paths ending in `stats.json`. Unrelated, we also disable _verbose_ ssh logging for `roachprod.SetupSSH`, which can result in _thousands_ of useless logs; this was observed in the same CI run, while debugging the "disappering" `stats.json` issue. [1] cockroachdb#124664 Epic: none Informs: cockroachdb#124827 Release note: None
srosenberg
requested review from
herkolategan and
DarrylWong
and removed request for
a team
|
This change is |
herkolategan
approved these changes
Jun 4, 2024
|
TFTR! bors r=herkolategan |
DarrylWong
added a commit
to DarrylWong/fork
that referenced
this pull request
Jun 10, 2024
In cockroachdb#125022, we excluded stats.json files from being zipped. However, because the stats.json file lives in a .perf dir, this dir is found first by filterDirEntries and zipped. This change instead excludes the .perf directory from being zipped. Release note: none Epic: none Fixes: none
srosenberg
added a commit
to srosenberg/cockroach
that referenced
this pull request
Jun 10, 2024
In cockroachdb#125022, we excluded stats.json files from being zipped. However, because the stats.json file lives in a .perf dir this dir is found first by filterDirEntries and zipped. This change instead excludes the .perf directory from being zipped. Epic: none Fixes: none Release note: None Co-authored-by: Darryl Wong <darryl@cockroachlabs.com>
srosenberg
added a commit
to srosenberg/cockroach
that referenced
this pull request
Jun 10, 2024
In cockroachdb#125022, we excluded stats.json files from being zipped. However, because the stats.json file lives in a .perf dir this dir is found first by filterDirEntries and zipped. This change instead excludes the .perf directory from being zipped. Epic: none Fixes: none Release note: None Co-authored-by: Darryl Wong <darryl@cockroachlabs.com>
craig bot
pushed a commit
that referenced
this pull request
Jun 10, 2024
125152: sql: extend EXPLAIN ANALYZE for follower reads and AOST r=yuzefovich a=yuzefovich **sql: extend EXPLAIN ANALYZE to indicate that follower reads were used** This commit extends EXPLAIN ANALYZE output for KV-reading operators to indicate whether the follower reads were used. This is achieved by reusing existing `ScanStats` infrastructure by adding a "marker" protobuf message. Note that we couldn't directly reuse the same `kvpb.ScanStats` object as the one we create in `evaluateBatch` since the determination whether the follower reads are eligible is done at a different point in time. We could have extended that protobuf and created a fresh object, but that seems a bit wrong. Co-authored with `@Uzair5162.` Release note (sql change): New field "used follower read" is added to EXPLAIN ANALYZE output to SQL operators whenever their reads were served by the follower replicas (previously, this information was only available in the trace). **sql: extend EXPLAIN ANALYZE for "historical" reads** This commit adds `historical: AS OF SYSTEM TIME ...` top-level attribute to EXPLAIN ANALYZE whenever the txn is historical (meaning AS OF SYSTEM TIME was specified either in the stmt or at the txn level). Release note (sql change): New attribute `historical: AS OF SYSTEM TIME ...` is now included into EXPLAIN ANALYZE output whenever the query performs historical reads. Fixes: #99984. 125431: roachtest: exclude .perf directories from artifacts.zip r=DarrylWong,renatolabs a=srosenberg In #125022, we excluded stats.json files from being zipped. However, because the stats.json file lives in a .perf dir this dir is found first by filterDirEntries and zipped. This change instead excludes the .perf directory from being zipped. Epic: none Fixes: none Release note: None Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com> Co-authored-by: Stan Rosenberg <stan.rosenberg@gmail.com>
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.
Upon test completion, the test runner invokes
zipArtifacts()in order to reduce the footprint of all test-specific logs retained by TC. In case of a successful benchmark, performance artifacts (i.e., stats.json) are fetched. Historically, those do not get zipped. Instead, they are uploaded (as is) into GCS.In [1], a new pattern was introduced, wherein
stats.jsonis written directly on the test runner node, as opposed to, on some remote (cluster) node. SincezipArtifactsruns before fetching performance artifacts,stats.jsonis effectively evaporated; i.e., the CI script no longer finds it.This PR adds an exclusion filter to
zipArtifacts, to ignore all paths ending instats.json.Unrelated, we also disable verbose ssh
logging for
roachprod.SetupSSH, which can result in thousands of useless logs; this was observed in the same CI run, while debugging the "disappering"stats.jsonissue.[1] #124664
Epic: none
Informs: #124827
Release note: None