Skip to content

db: improve compaction test timeout observability - #5279

Merged
RaduBerinde merged 2 commits into
cockroachdb:masterfrom
RaduBerinde:debug-compaction-timeout
Sep 16, 2025
Merged

RaduBerinde merged 2 commits into
cockroachdb:masterfrom
RaduBerinde:debug-compaction-timeout

Conversation

@RaduBerinde

Copy link
Copy Markdown
Member
  • set up a 1 minute timeout for any datadriven compactions;
  • print db logs on test failures;
  • upgrade datadriven to include a recent change to periodically log
    when a single command takes a long time.

Informs #5277

@RaduBerinde
RaduBerinde requested a review from jbowens September 5, 2025 18:14
@RaduBerinde
RaduBerinde requested a review from a team as a code owner September 5, 2025 18:14
@cockroach-teamcity

Copy link
Copy Markdown
Member

This change is Reviewable

@RaduBerinde

Copy link
Copy Markdown
Member Author

(pending merge of cockroachdb/datadriven#57)

@xxmplus xxmplus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 0 of 12 files reviewed, 1 unresolved discussion (waiting on @jbowens)


compaction_test.go line 986 at r1 (raw file):

		defer func() {
			if t.Failed() {
				t.Logf("db log:\n%s\n", dbLog.String())

unrelated newbie question: do we print severity in logs in Pebble? or do we leave it to crdb to do that? I looked it up and it seems golang support structured error message via https://pkg.go.dev/log/slog.

@RaduBerinde RaduBerinde left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

TFTR!

Reviewable status: 0 of 12 files reviewed, 1 unresolved discussion (waiting on @jbowens and @xxmplus)


compaction_test.go line 986 at r1 (raw file):

Previously, xxmplus (Andy Xu) wrote…

unrelated newbie question: do we print severity in logs in Pebble? or do we leave it to crdb to do that? I looked it up and it seems golang support structured error message via https://pkg.go.dev/log/slog.

The default implementation (below) doesn't, and neither does this test-only InMemLogger. CRDB implements its own logger which does take into account the type of call (Info/Error/Fatal).

func (defaultLogger) Infof(format string, args ...interface{}) {

This grabs a recent commit that issues periodic logs during slow
datadriven commands.
 - set up a 1 minute timeout for any datadriven compactions;
 - print db logs on test failures;
 - upgrade datadriven to include a recent change to periodically log
   when a single command takes a long time.

Informs #5277
@RaduBerinde
RaduBerinde merged commit 5a51e40 into cockroachdb:master Sep 16, 2025
8 checks passed
@RaduBerinde
RaduBerinde deleted the debug-compaction-timeout branch September 16, 2025 14:58
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