Skip to content

test-utils: Disable CockroachDB's automatic emergency ballast #1005

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

Merged
merged 1 commit into from
May 3, 2022

Conversation

jgallagher
Copy link
Contributor

Should fix #1004

@jgallagher jgallagher requested a review from davepacheco May 3, 2022 20:30
@jgallagher jgallagher enabled auto-merge (squash) May 3, 2022 20:38
//
// https://www.cockroachlabs.com/docs/v21.2/cluster-setup-troubleshooting#automatic-ballast-files
let mut store_arg = OsString::from("--store=path=");
store_arg.push(&store_dir);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this still work if the store_dir has a space in it? I think it would have before because we were passing it as a separate exec argument. Now...I think it still will but I'm less clear on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that it does. I changed the code above this that creates the temp dir to

let temp_dir = tempfile::tempdir_in("/tmp/my dir").with_context(|| "creating temporary directory")?;

and confirmed the tests pass (and that I was seeing cockroachdb files under /tmp/my dir/.tmpXXXXXX).

One thing that surprised me was that originally I had written this as

.arg("--store")
.arg(&store_arg) // "path={store_dir),ballast-size=0"

and the tests failed in a way that looked like the path was not being set at all (specifically, the cleanup was failing claiming that the expected /tmp/.tmpXXXXXX directory didn't exist); possibly Cockroach was treating the full string as a path? I did not look into this but assume we're at the mercy of their command line parsing.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for testing it!

@jgallagher jgallagher disabled auto-merge May 3, 2022 20:49
@jgallagher jgallagher enabled auto-merge (squash) May 3, 2022 21:09
@jgallagher jgallagher merged commit f320eea into main May 3, 2022
@jgallagher jgallagher deleted the disable-cockroach-ballast-in-tests branch May 3, 2022 21:30
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.

tests use lots of disk space after recent CockroachDB upgrade
3 participants