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

build: plumb COCKROACH_PROPOSER_EVALUATED_KV env var #10455

Merged
merged 1 commit into from
Nov 4, 2016

Conversation

tbg
Copy link
Member

@tbg tbg commented Nov 4, 2016

This should make it easy to run a nightly build for which tests are run using
proposer-evaluated KV.

Updates #10454.


This change is Reviewable

@tbg
Copy link
Member Author

tbg commented Nov 4, 2016

My current impression is that the best way to set up the nightly is to trigger the GitHub CI job with env.COCKROACH_PROPOSER_EVALUATED_KV=true. Please let me know if that's a bad idea.

@tamird
Copy link
Contributor

tamird commented Nov 4, 2016

What are you trying to achieve? Do you want a single nightly run of the tests with prop-evaluated kv, or do you want stress with that configuration?


Review status: 0 of 5 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@tbg
Copy link
Member Author

tbg commented Nov 4, 2016

The former, with an option of the latter (but that would presumably use your stress-meta).

@jordanlewis
Copy link
Member

I think this strategy makes sense. The main pitfall you'll run into is that there's no UI option to trigger with a set of environment variables - you need to use an API call. It shouldn't be too hard, though - make a nightly job that hits that API endpoint. http://stackoverflow.com/questions/10007874/trigger-option-to-set-specific-build-parameters


Review status: 0 of 5 files reviewed at latest revision, 1 unresolved discussion.


build/teamcity-acceptance.sh, line 7 at r1 (raw file):

build/builder.sh go test -v -c -tags acceptance ./pkg/acceptance
cd pkg/acceptance
COCKROACH_PROPOSER_EVALUATED_KV="${COCKROACH_PROPOSER_EVALUATED_KV-}" ../../acceptance.test -nodes 3 -l ../../artifacts/acceptance -test.v -test.timeout 10m 2>&1 | go-test-teamcity

I don't think you actually need to do this, since we're not running this command from within the builder. I would suggest echoing the value of this variable on the line above or something to make it clear in the build log what's going on.


Comments from Reviewable

@tamird
Copy link
Contributor

tamird commented Nov 4, 2016

There is a UI option, it's just not very ergonomic. :lgtm:


Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


build/teamcity-acceptance.sh, line 7 at r1 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

I don't think you actually need to do this, since we're not running this command from within the builder. I would suggest echoing the value of this variable on the line above or something to make it clear in the build log what's going on.

+1

don't echo this variable unless you plan to also echo the entire env


pkg/cmd/github-pull-request-make/main.go, line 169 at r1 (raw file):

              fmt.Sprintf("PKG=./%s", name),
              fmt.Sprintf("TESTS=%s", tests),
              fmt.Sprintf("COCKROACH_PROPOSER_EVALUATED_KV=%t", envutil.EnvOrDefaultBool("COCKROACH_PROPOSER_EVALUATED_KV", false)),

remove this, since it will literally not do anything (PR builds don't have this env var set, unless you plan to set it?)


Comments from Reviewable

@tamird
Copy link
Contributor

tamird commented Nov 4, 2016

Oh, there might not be a UI option for a scheduled trigger with specific parameters?


Review status: all files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

@jordanlewis
Copy link
Member

Yes, that's what I'm saying.


Review status: all files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

@tbg tbg force-pushed the nightly-prop-eval-kv branch from 1b407ae to 0052281 Compare November 4, 2016 19:01
This should make it easy to run a nightly build for which tests are run using
proposer-evaluated KV.

Updates cockroachdb#10454.
@tbg tbg force-pushed the nightly-prop-eval-kv branch from 0052281 to ca5a956 Compare November 4, 2016 19:06
@tbg
Copy link
Member Author

tbg commented Nov 4, 2016

OK, plumbing this only for the main test and testrace invocation (and implicitly for acceptance). I also let cockroach print a message when it's running with proposer-evaluated KV; that way test failures should be easy to recognize. PTAL and then I'll try to set up the CI.


Review status: 2 of 4 files reviewed at latest revision, 2 unresolved discussions.


build/teamcity-acceptance.sh, line 7 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

+1

don't echo this variable unless you plan to also echo the entire env

Removed.

pkg/cmd/github-pull-request-make/main.go, line 169 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

remove this, since it will literally not do anything (PR builds don't have this env var set, unless you plan to set it?)

Done. Not planning to set this, just plumbed a little bit too much.

Comments from Reviewable

@tbg tbg merged commit d5977ba into cockroachdb:master Nov 4, 2016
@tbg tbg deleted the nightly-prop-eval-kv branch November 4, 2016 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants