Add `VStreamerCount` stat to `vttablet` by timvaillancourt · Pull Request #11978 · vitessio/vitess

timvaillancourt · 2022-12-15T20:59:25Z

Description

This PR adds the ~~VStreamersActive~~ VStreamerCount stats metric to vttablet to make the # of active streams more observable. There may be a way to infer this using rates from a combination of VStreamersCreated and other metrics but I was unable to do so myself (in Prometheus/Grafana) after many attempts so this feels like a rough edge

The length of the VStream engine streamers map is used as the number of active streams. This is done without a lock but I'm happy to add one

Related Issue(s)

Checklist

"Backport to:" labels have been added if this change should be back-ported
Tests are not required
Documentation: Add VStreamerCount metric from vitessio/vitess pr #11978 website#1291

vitess-bot · 2022-12-15T20:59:27Z

mattlord · 2022-12-15T22:30:01Z

Just a note that we'll need to document this new metric here on the vreplication metrics page: vitessio/website#1267

@timvaillancourt we should try and have you push directly to that PR. I can help get you setup with any access you need and walk you through it as needed.

mattlord

Thank you! I also noticed this missing metric info today: https://vitess.slack.com/archives/C0PQY0PTK/p1671132033807179?thread_ts=1670952842.374699&cid=C0PQY0PTK

I had to look at the tablets' /debug/status endpoints to see where the vstreamer was running.

I would note that this is not only about vtgate<->vttablet vstreams but also vttablet<->vttablet vstreams such as are used for VReplication workflows after the copy phase. Just something to keep in mind as we think about this.

go/vt/vttablet/tabletserver/vstreamer/engine.go

timvaillancourt · 2022-12-15T23:41:27Z

@mattlord thanks for the review! One question re: the mutex added and website PR on the way tomorrow 👍

timvaillancourt · 2022-12-15T23:43:15Z

Hrm, re-requesting review from Matt dropped the other reviewers. My apologies 🤷

mattlord

LGTM! ❤️

timvaillancourt · 2022-12-16T17:52:32Z

@timvaillancourt we should try and have you push directly to that PR. I can help get you setup with any access you need and walk you through it as needed.

@mattlord thanks! Website PR created here: vitessio/website#1291

rohit-nayak-ps · 2022-12-18T15:38:49Z

go/vt/vttablet/tabletserver/vstreamer/engine.go

Can we instead use a stats.Gauge as a new field vstreamerCount in vstreamer.Engine? Then in vse.Stream(): vse.vstreamerCount.Add(1) while creating a stream and vse.vstreamerCount.Add(-1). stats.Gauge is an atomicInt64. We then avoid the mutex requirement.

@rohit-nayak-ps good idea, I'll make that change 👍

@rohit-nayak-ps / @mattlord changes made in 5d889e0 and 2af4045

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

mattlord

LGTM (again)! 😄

rohit-nayak-ps

lgtm

Thanks! this is a useful new metric.

* Add `VStreamersActive` stat to `vttablet` Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Improve desc Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Add PR suggestions Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Move to *stats.Gauge Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Single defer Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Add `VStreamerCount` stat to `vttablet` (vitessio#11978) * Add `VStreamersActive` stat to `vttablet` Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Improve desc Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Add PR suggestions Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Move to *stats.Gauge Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Single defer Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Add `Uptime` metric (vitessio#12712) * Add `Uptime` metric to `vtgate`+`vttablet` Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * move to go/vt/servenv/status.go Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Use nanoseconds for uptime Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Move Uptime metrics to servenv.go, remove dupe start time.Time Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Use serverStart time.Time Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Implement the RowsColumnTypeScanType interface in the go sql driver for Vitess in order to get the column types (vitessio#12007) Signed-off-by: Johan Oskarsson <joskarsson@slack-corp.com> Signed-off-by: Johan Oskarsson <joskarsson@slack-corp.com> Co-authored-by: Johan Oskarsson <joskarsson@slack-corp.com> * Add vstream metrics to vtgate (vitessio#13098) * Add vstream metrics to vtgate Signed-off-by: twthorn <thomaswilliamthornton@gmail.com> * Update unit test name and use cell variable Signed-off-by: twthorn <thomaswilliamthornton@gmail.com> * Reset metrics for TestVStreamsCreatedAndLagMetrics, fix data race issue Signed-off-by: twthorn <thomaswilliamthornton@gmail.com> --------- Signed-off-by: twthorn <thomaswilliamthornton@gmail.com> * add lock to flaky TestValidateVersionShard test Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * typo Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Signed-off-by: Johan Oskarsson <joskarsson@slack-corp.com> Signed-off-by: twthorn <thomaswilliamthornton@gmail.com> Co-authored-by: Johan Oskarsson <johan@oskarsson.nu> Co-authored-by: Johan Oskarsson <joskarsson@slack-corp.com> Co-authored-by: Thomas Thornton <thomaswilliamthornton@gmail.com>

timvaillancourt force-pushed the stats-VStreamersActive branch from a6c3dc8 to ab21e4f Compare December 15, 2022 21:02

timvaillancourt marked this pull request as ready for review December 15, 2022 21:03

timvaillancourt requested review from harshit-gangal, rohit-nayak-ps, shlomi-noach and systay as code owners December 15, 2022 21:03

timvaillancourt force-pushed the stats-VStreamersActive branch from 094cfd9 to 9e3997b Compare December 15, 2022 21:30

mattlord self-requested a review December 15, 2022 22:21

mattlord added Type: Enhancement Logical improvement (somewhere between a bug and feature) Trivial Component: VReplication labels Dec 15, 2022

mattlord reviewed Dec 15, 2022

View reviewed changes

go/vt/vttablet/tabletserver/vstreamer/engine.go Outdated Show resolved Hide resolved

go/vt/vttablet/tabletserver/vstreamer/engine.go Outdated Show resolved Hide resolved

go/vt/vttablet/tabletserver/vstreamer/engine.go Outdated Show resolved Hide resolved

timvaillancourt changed the title ~~Add VStreamersActive stat to vttablet~~ Add VStreamerCount stat to vttablet Dec 15, 2022

timvaillancourt requested review from mattlord and removed request for harshit-gangal, rohit-nayak-ps, shlomi-noach and systay December 15, 2022 23:42

timvaillancourt mentioned this pull request Dec 15, 2022

Add 'VStreamerCount' metric to vttablet slackhq/vitess#32

Merged

3 tasks

mattlord requested a review from rohit-nayak-ps December 16, 2022 00:18

mattlord approved these changes Dec 16, 2022

View reviewed changes

timvaillancourt mentioned this pull request Dec 16, 2022

Add VStreamerCount metric from vitessio/vitess pr #11978 vitessio/website#1291

Merged

rohit-nayak-ps reviewed Dec 18, 2022

View reviewed changes

timvaillancourt added 2 commits December 19, 2022 20:59

Add VStreamersActive stat to vttablet

0ce0caf

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

Improve desc

e780c9b

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

timvaillancourt added 3 commits December 19, 2022 20:59

Add PR suggestions

717af3d

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

Move to *stats.Gauge

e5a4db1

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

Single defer

eb4e4ae

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

timvaillancourt force-pushed the stats-VStreamersActive branch from 2af4045 to eb4e4ae Compare December 19, 2022 19:59

mattlord approved these changes Dec 20, 2022

View reviewed changes

mattlord requested a review from rohit-nayak-ps December 20, 2022 00:44

rohit-nayak-ps approved these changes Dec 20, 2022

View reviewed changes

rohit-nayak-ps merged commit 47a7cf8 into vitessio:main Dec 20, 2022

timvaillancourt mentioned this pull request Dec 20, 2022

Use stats.Gauge w/o mutex for VStreamerCount stat slackhq/vitess#36

Merged

3 tasks

timvaillancourt deleted the stats-VStreamersActive branch December 20, 2022 18:55

timvaillancourt mentioned this pull request Mar 30, 2023

slack-vitess-r14.0.4: cherry-picks, part 1 slackhq/vitess#59

Closed

4 tasks

timvaillancourt mentioned this pull request Mar 30, 2023

slack-vitess-r14.0.5: cherry-picks, run full tests on slack-vitess*, add retries to CI workflows slackhq/vitess#60

Merged

4 tasks

timvaillancourt mentioned this pull request May 21, 2024

slack-vitess-r15.0.5: misc required backports, pt. 1 slackhq/vitess#361

Merged

4 tasks

Conversation

timvaillancourt commented Dec 15, 2022 • edited by mattlord Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue(s)

Checklist

Uh oh!

vitess-bot bot commented Dec 15, 2022 • edited by mattlord Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review Checklist

General

If a new flag is being introduced:

If a workflow is added or modified:

Bug fixes

Non-trivial changes

New/Existing features

Backward compatibility

Uh oh!

mattlord commented Dec 15, 2022

Uh oh!

mattlord left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

timvaillancourt commented Dec 15, 2022

Uh oh!

timvaillancourt commented Dec 15, 2022

Uh oh!

mattlord left a comment

Choose a reason for hiding this comment

Uh oh!

timvaillancourt commented Dec 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rohit-nayak-ps Dec 18, 2022

Choose a reason for hiding this comment

Uh oh!

timvaillancourt Dec 18, 2022

Choose a reason for hiding this comment

Uh oh!

timvaillancourt Dec 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattlord left a comment

Choose a reason for hiding this comment

Uh oh!

rohit-nayak-ps left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

timvaillancourt commented Dec 15, 2022 •

edited by mattlord

Loading

vitess-bot bot commented Dec 15, 2022 •

edited by mattlord

Loading

timvaillancourt commented Dec 16, 2022 •

edited

Loading

timvaillancourt Dec 19, 2022 •

edited

Loading