Skip to content

ccl/sqlproxyccl: add connection migration-related metrics#77700

Merged
craig[bot] merged 2 commits intocockroachdb:masterfrom
jaylim-crl:jay/sqlproxy-add-transfer-latency
Mar 14, 2022
Merged

ccl/sqlproxyccl: add connection migration-related metrics#77700
craig[bot] merged 2 commits intocockroachdb:masterfrom
jaylim-crl:jay/sqlproxy-add-transfer-latency

Conversation

@jaylim-crl
Copy link
Copy Markdown
Contributor

@jaylim-crl jaylim-crl commented Mar 11, 2022

ccl/sqlproxyccl: add metric that measures connection migration latency

Previously, we added support for connection migration in #76805. This commit
adds a new proxy.conn_migration.attempted.latency metric that tracks latency
for attempted connection migrations. Having this metric would be beneficial
as we can now know how long users were blocked during connection migrations.

Release justification: Low-risk metric-only change within sqlproxy.

Release note: None

ccl/sqlproxyccl: add metric that records the transfer response message size

Informs #76000, and follow up to #76805.

This commit adds a new proxy.conn_migration.transfer_response.message_size
metric that will track the distribution of the transfer response message size.
This will be used to tune a value for the SQL cluster setting:
sql.session_transfer.max_session_size.

Release justification: Low-risk metric-only change within sqlproxy.

Release note: None

@jaylim-crl jaylim-crl requested review from a team as code owners March 11, 2022 20:00
@jaylim-crl jaylim-crl requested review from jeffswenson and removed request for a team March 11, 2022 20:00
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@jaylim-crl jaylim-crl force-pushed the jay/sqlproxy-add-transfer-latency branch from 2f232a8 to c1ddc64 Compare March 13, 2022 17:06
@jaylim-crl jaylim-crl changed the title ccl/sqlproxyccl: add metric that measures connection migration latency ccl/sqlproxyccl: add connection migration-related metrics Mar 13, 2022
@jaylim-crl jaylim-crl force-pushed the jay/sqlproxy-add-transfer-latency branch from c1ddc64 to ba4c58e Compare March 13, 2022 19:25
Copy link
Copy Markdown
Collaborator

@jeffswenson jeffswenson left a comment

Choose a reason for hiding this comment

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

LGTM

@jaylim-crl jaylim-crl force-pushed the jay/sqlproxy-add-transfer-latency branch from ba4c58e to 0e7266b Compare March 14, 2022 14:11
Copy link
Copy Markdown
Contributor Author

@jaylim-crl jaylim-crl left a comment

Choose a reason for hiding this comment

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

TFTR!

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


pkg/ccl/sqlproxyccl/conn_migration.go, line 140 at r2 (raw file):

Previously, JeffSwenson (Jeff Swenson) wrote…

It would be nice to add the transfer latency to the log statements in the defer. That way we can dig into the source of unexpectedly high/low latency.

Done.

Previously, we added support for connection migration in cockroachdb#76805. This commit
adds a new `proxy.conn_migration.attempted.latency` metric that tracks latency
for attempted connection migrations. Having this metric would be beneficial
as we can now know how long users were blocked during connection migrations.

Release justification: Low-risk metric-only change within sqlproxy.

Release note: None
…e size

Informs cockroachdb#76000, and follow up to cockroachdb#76805.

This commit adds a new proxy.conn_migration.transfer_response.message_size
metric that will track the distribution of the transfer response message size.
This will be used to tune a value for the SQL cluster setting:
sql.session_transfer.max_session_size.

Release justification: Low-risk metric-only change within sqlproxy.

Release note: None
@jaylim-crl jaylim-crl force-pushed the jay/sqlproxy-add-transfer-latency branch from 0e7266b to 454b896 Compare March 14, 2022 14:13
@jaylim-crl
Copy link
Copy Markdown
Contributor Author

bors r=JeffSwenson

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Mar 14, 2022

Build succeeded:

@craig craig bot merged commit 51cbdce into cockroachdb:master Mar 14, 2022
@jaylim-crl jaylim-crl deleted the jay/sqlproxy-add-transfer-latency branch March 14, 2022 17:43
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