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.

Sign up for GitHub

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
Merged

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions test-utils/src/dev/db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,21 @@ impl CockroachStarterBuilder {
CockroachStarterBuilder::temp_path(&temp_dir, "data")
.into_os_string()
});

// Disable the CockroachDB automatic emergency ballast file. By default
// CockroachDB creates a 1 GiB ballast file on startup; because we start
// many instances while running tests in parallel, this can quickly eat
// a large amount of disk space. Disable it by setting the size to 0.
//
// 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

@davepacheco davepacheco May 3, 2022

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

@jgallagher jgallagher May 3, 2022

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

@davepacheco davepacheco May 3, 2022

Choose a reason for hiding this comment

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

Thanks for testing it!

store_arg.push(",ballast-size=0");

let listen_url_file =
CockroachStarterBuilder::temp_path(&temp_dir, "listen-url");
let listen_arg = format!("127.0.0.1:{}", self.listen_port);
self.arg("--store")
.arg(&store_dir)
self.arg(&store_arg)
.arg("--listen-addr")
.arg(&listen_arg)
.arg("--listening-url-file")
Expand Down