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

roachtest: print global seed to test.log #113950

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions pkg/cmd/roachtest/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -1936,6 +1936,7 @@ func (c *clusterImpl) StartE(
// Set the same seed on every node, to be used by builds with
// runtime assertions enabled.
setUnlessExists("COCKROACH_RANDOM_SEED", c.cockroachRandomSeed())
l.Printf("ROACHTEST COCKROACH_RANDOM_SEED: %d", c.cockroachRandomSeed())
Copy link
Contributor

Choose a reason for hiding this comment

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

Two things to consider about this approach:

  • It will be logged even if we are not using a metamorphic build.
  • It will be logged every time we start cockroach, which could be several times in a test (especially in upgrade tests).

Maybe the best place to put this type of message is during cluster setup (only once); it could even be done as part of your work in #113301, where there's a pretty well-defined moment where we decide if we are running with metamorphic builds or not (and that only happens once during a test's lifetime).

Thoughts?


Nit: I would also slightly change the messaging, something like to reproduce failures using the same metamorphic constants, run this test with %s=%d (roachtest env var, seed). Doesn't have to be this exactly, but it would be nice to convey that information in the log message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me


// Needed for backward-compat on crdb_internal.ranges{_no_leases}.
// Remove in v23.2.
Expand Down