Skip to content
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

roachperf: tpc-c data not present for July #38871

Closed
awoods187 opened this issue Jul 15, 2019 · 3 comments · Fixed by #38889
Closed

roachperf: tpc-c data not present for July #38871

awoods187 opened this issue Jul 15, 2019 · 3 comments · Fixed by #38889
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@awoods187
Copy link
Contributor

image

https://roachperf.crdb.dev/?filter=&view=tpccbench%2Fnodes%3D3%2Fcpu%3D16&tab=aws

@awoods187
Copy link
Contributor Author

cc @kenliu @nvanbenschoten

@awoods187 awoods187 added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jul 15, 2019
@awoods187
Copy link
Contributor Author

@awoods187
Copy link
Contributor Author

ajwerner added a commit to ajwerner/cockroach that referenced this issue Jul 15, 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 does not unconditionally
copy logs to the test runner, we have not gotten any perf artifacts since the
change.

This PR introduces a new concept, perf artifacts, which live in a different
directory, the perfArtifactsDir. If a testSpec indicates that it
HasPerfArtifacts then that directory is copied to the test runner's
artifactsDir for that test from each node in the cluster upon success.

This change also necessitates a small change to roachperf to look in this new
"perf" directory.

Fixes cockroachdb#38871.

Release note: None
ajwerner added a commit to ajwerner/cockroach that referenced this issue 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
craig bot pushed a commit that referenced this issue Jul 16, 2019
38848: roachpb: further clarify correctness of observed timestamps r=nvanbenschoten a=nvanbenschoten

This commit expands on the explanation added in b7cc205. Specifically,
it focuses on the inability to bound a transaction's uncertainty interval
below the starting timestamp of the lease that it is executing a request
on. This limitation was added in 85d4627. I tend to forget about it once
every few months and get very confused how this all works.

Release note: None

38889: roachtest: create perf artifacts concept and collect them r=nvanbenschoten a=ajwerner

Before the roachtest rewrite in #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 does not unconditionally
copy logs to the test runner, we have not gotten any perf artifacts since the
change.

This PR introduces a new concept, perf artifacts, which live in a different
directory, the perfArtifactsDir. If a testSpec indicates that it
HasPerfArtifacts then that directory is copied to the test runner's
artifactsDir for that test from each node in the cluster upon success.

This change also necessitates a small change to roachperf to look in this new
"perf" directory.

Fixes #38871.

Release note: None

38895: roachprod: disable ssh logging r=ajwerner a=tbg

This was never fully introduced and there is currently no impetus to
push it over the finish line. It would be useful to diagnose issues
that are readily reproducible, but we haven't seen any of those.

Release note: None

Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Tobias Schottdorf <[email protected]>
@craig craig bot closed this as completed in #38889 Jul 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant