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

kvcoord: add proxy tracing test #124043

Merged

Conversation

kvoli
Copy link
Collaborator

@kvoli kvoli commented May 13, 2024

Add TestProxyTracing, which asserts that when a partition partition is enabled a (1) request is proxied via another node to the leaseholder and (2) the trace captures the relevant event.

Resolves: #124036
Release note: None

Add `TestProxyTracing`, which asserts that when a partition partition is
enabled a (1) request is proxied via another node to the leaseholder and
(2) the trace captures the relevant event.

Resolves: cockroachdb#124036
Release note: None
@kvoli kvoli self-assigned this May 13, 2024
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@kvoli kvoli marked this pull request as ready for review May 13, 2024 15:46
@kvoli kvoli requested a review from a team as a code owner May 13, 2024 15:46
@kvoli kvoli requested a review from andrewbaptist May 13, 2024 15:46
Copy link
Collaborator

@andrewbaptist andrewbaptist left a comment

Choose a reason for hiding this comment

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

:lgtm:

Thanks for adding! I'll try and track down the failing SELECT later today.

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @kvoli)


pkg/kv/kvclient/kvcoord/dist_sender_server_test.go line 4890 at r1 (raw file):

	}

	// Wait until the leaseholder for the test table ranges are on n3.

nit: look at relocateLeases in failover.go for a faster way to do this using ALTER RANGE RELOCATE LEASE. This will remove the need for the lease queue at all and likely make the test complete faster.

Copy link
Collaborator Author

@kvoli kvoli left a comment

Choose a reason for hiding this comment

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

TYFTR!

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @andrewbaptist and @kvoli)


pkg/kv/kvclient/kvcoord/dist_sender_server_test.go line 4890 at r1 (raw file):

Previously, andrewbaptist (Andrew Baptist) wrote…

nit: look at relocateLeases in failover.go for a faster way to do this using ALTER RANGE RELOCATE LEASE. This will remove the need for the lease queue at all and likely make the test complete faster.

checkLeaseCount is force processing leases on n1. It seems almost equivalent here. The test passes fairly quickly so I'll leave it be for now.

@kvoli kvoli added the backport-24.1.x Flags PRs that need to be backported to 24.1. label May 14, 2024
@kvoli
Copy link
Collaborator Author

kvoli commented May 14, 2024

TYFTR

bors r=andrewbaptist

@craig craig bot merged commit bb4479c into cockroachdb:master May 14, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-24.1.x Flags PRs that need to be backported to 24.1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kvcoord: add dist sender proxy tracing test
3 participants