Skip to content

Conversation

annrpom
Copy link
Contributor

@annrpom annrpom commented Sep 17, 2025

Fixes: #4342

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@annrpom annrpom force-pushed the crash-during-open branch 8 times, most recently from d560342 to 953efc6 Compare September 18, 2025 18:55
@annrpom annrpom marked this pull request as ready for review September 19, 2025 17:54
@annrpom annrpom requested a review from a team as a code owner September 19, 2025 17:54
@annrpom annrpom requested a review from jbowens September 19, 2025 17:54
@annrpom annrpom force-pushed the crash-during-open branch 6 times, most recently from c845805 to dbcb91a Compare September 23, 2025 16:58
Copy link
Collaborator

@jbowens jbowens left a comment

Choose a reason for hiding this comment

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

:lgtm:

@jbowens reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions


open_test.go line 2244 at r11 (raw file):

	for valid := iter.First(); valid; valid = iter.Next() {
		key := string(iter.Key())
		value := make([]byte, len(iter.Value()))

nit: could use slices.Clone(iter.Value())


open_test.go line 2251 at r11 (raw file):

	// Verify that found data matches expected data.
	require.True(t, len(foundKeys) > 0, "no keys found after crash")

nit: could use require.NotEmpty(t, foundKeys, "no keys found after crash") (so the printed message during a failure is a little more clear)

@annrpom annrpom force-pushed the crash-during-open branch from dbcb91a to 44ced27 Compare October 2, 2025 17:54
Copy link
Contributor Author

@annrpom annrpom left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @jbowens)


open_test.go line 2244 at r11 (raw file):

Previously, jbowens (Jackson Owens) wrote…

nit: could use slices.Clone(iter.Value())

done


open_test.go line 2251 at r11 (raw file):

Previously, jbowens (Jackson Owens) wrote…

nit: could use require.NotEmpty(t, foundKeys, "no keys found after crash") (so the printed message during a failure is a little more clear)

done

@annrpom
Copy link
Contributor Author

annrpom commented Oct 2, 2025

TFTR! ('-')7

@annrpom annrpom merged commit 347d5dc into cockroachdb:master Oct 2, 2025
8 checks passed
@annrpom annrpom deleted the crash-during-open branch October 2, 2025 18:08
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.

metamorphic: test crash during Open
3 participants