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: Cast snapshot-recd bytes to int in disagg-rebalance #109142

Merged
merged 1 commit into from
Aug 21, 2023

Conversation

itsbilal
Copy link
Member

@itsbilal itsbilal commented Aug 21, 2023

Previously we were reading a float value as an int, which would trip up the Scan() method if the float value was large enough to be wired over in scientified notation eg. 2.3456E7. This change ensures that Cockroach prints out the value as an integer to avoid the scan-time error in the roachtest.

Fixes #109114.

Epic: none

Release note: None

@itsbilal itsbilal requested review from a team and jbowens August 21, 2023 15:39
@itsbilal itsbilal requested a review from a team as a code owner August 21, 2023 15:39
@itsbilal itsbilal requested review from srosenberg and renatolabs and removed request for a team August 21, 2023 15:39
@cockroach-teamcity
Copy link
Member

This change is Reviewable

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:

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @itsbilal, @renatolabs, and @srosenberg)


pkg/cmd/roachtest/tests/disagg_rebalance.go line 117 at r1 (raw file):

			var bytesSnapshotted int64
			if err := db.QueryRow(
				"SELECT metrics['range.snapshots.rcvd-bytes']::DECIMAL FROM crdb_internal.kv_store_status WHERE node_id = $1 LIMIT 1",

should we cast to an INT if we're going to scan into an integer int64 type?

Previously we were reading a float value as an int, which
would trip up the Scan() method if the float value was
large enough to be wired over in scientified notation eg.
`2.3456E7`. This change ensures that Cockroach prints
out the value as an int to avoid the scan-time error
in the roachtest.

Fixes cockroachdb#109114.

Epic: none

Release note: None
@itsbilal itsbilal force-pushed the disagg-rebalance-dec-cast branch from 3eebffb to 06f6a0f Compare August 21, 2023 17:40
@itsbilal itsbilal changed the title roachtest: Cast snapshot-recd bytes to decimal in disagg-rebalance roachtest: Cast snapshot-recd bytes to int in disagg-rebalance Aug 21, 2023
Copy link
Member Author

@itsbilal itsbilal 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: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @jbowens, @renatolabs, and @srosenberg)


pkg/cmd/roachtest/tests/disagg_rebalance.go line 117 at r1 (raw file):

Previously, jbowens (Jackson Owens) wrote…

should we cast to an INT if we're going to scan into an integer int64 type?

Good point, done. Shouldn't matter in practice but doesn't hurt to be safe.

@itsbilal
Copy link
Member Author

TFTR!

bors r=jbowens

@craig craig bot merged commit 604a90a into cockroachdb:master Aug 21, 2023
@craig
Copy link
Contributor

craig bot commented Aug 21, 2023

Build succeeded:

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.

roachtest: disagg-rebalance/aws/n4cpu4 failed
3 participants