kvserver: Expose Bytes Written to WAL #104890

raggar · 2023-06-14T15:34:11Z

This change exposes the WAL.BytesWritten WAL.BytesIn metric from Pebble to the Cockroach Side. Proto changes seemed to have been made previously:

cockroach/pkg/util/log/eventpb/storage_events.proto

Lines 113 to 114 in e68a36c

    
           // wal_bytes_written is the number of bytes written to the WAL (counter). 
        
           uint64 wal_bytes_written = 28 [(gogoproto.jsontag) = ",omitempty"];

Fixes: #104531
Release note: None

blathers-crl · 2023-06-14T15:34:15Z

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

cockroach-teamcity · 2023-06-14T15:34:21Z

This change is

RaduBerinde

We should add a graph for it. Grep for cr.store.rocksdb.flushed-bytes in storage.tsx and add a similar graph.

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

jbowens

Reviewed all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @RahulAggarwal1016)

pkg/kv/kvserver/metrics.go line 2017 at r1 (raw file):

	}

	metaWalBytesWritten = metric.Metadata{

nit: capitalize as WAL since WAL is an acronym.

https://github.com/golang/go/wiki/CodeReviewComments#initialisms

pkg/kv/kvserver/metrics.go line 2022 at r1 (raw file):

		Measurement: "Events",
		Unit:        metric.Unit_COUNT,
	}

let's add a metric for Metrics.WAL.BytesIn too

pkg/kv/kvserver/metrics.go line 2260 at r1 (raw file):

	RaftStorageReadBytes      *metric.Counter

	// WAL even metrics

typo?

pkg/kv/kvserver/metrics.go line 2261 at r1 (raw file):

	// WAL even metrics
	WalBytesWritten *metric.Gauge

nit: capitalize as WAL since WAL is an acronym.

https://github.com/golang/go/wiki/CodeReviewComments#initialisms

pkg/kv/kvserver/metrics.go line 2827 at r1 (raw file):

		// WAL metrics
		WalBytesWritten: metric.NewGauge(metaWalBytesWritten),

nit: no need to separate into a distinct block, can include with the above block of all storage metrics.

raggar

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

pkg/kv/kvserver/metrics.go line 2017 at r1 (raw file):

Previously, jbowens (Jackson Owens) wrote…

nit: capitalize as WAL since WAL is an acronym.

https://github.com/golang/go/wiki/CodeReviewComments#initialisms

Done

raggar

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

pkg/kv/kvserver/metrics.go line 2022 at r1 (raw file):

Previously, jbowens (Jackson Owens) wrote…

let's add a metric for Metrics.WAL.BytesIn too

Done

raggar

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

pkg/kv/kvserver/metrics.go line 2260 at r1 (raw file):

Previously, jbowens (Jackson Owens) wrote…

typo?

yup that was a typo

raggar

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

pkg/kv/kvserver/metrics.go line 2261 at r1 (raw file):

Previously, jbowens (Jackson Owens) wrote…

nit: capitalize as WAL since WAL is an acronym.

https://github.com/golang/go/wiki/CodeReviewComments#initialisms

Done

raggar

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

pkg/kv/kvserver/metrics.go line 2827 at r1 (raw file):

Previously, jbowens (Jackson Owens) wrote…

nit: no need to separate into a distinct block, can include with the above block of all storage metrics.

Done

raggar

done

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

RaduBerinde

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jbowens and @RahulAggarwal1016)

pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeGraphs/dashboards/storage.tsx line 236 at r3 (raw file):

    <LineGraph
      title="WAL Bytes In"

Not sure the "bytes in" metric is useful enough for a graph, it's up to @jbowens. I think having both would just raise questions from users.

jbowens

Reviewed 1 of 2 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @RaduBerinde)

pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeGraphs/dashboards/storage.tsx line 236 at r3 (raw file):

Previously, RaduBerinde wrote…

Not sure the "bytes in" metric is useful enough for a graph, it's up to @jbowens. I think having both would just raise questions from users.

agreed

raggar

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jbowens and @RaduBerinde)

pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeGraphs/dashboards/storage.tsx line 236 at r3 (raw file):

Previously, jbowens (Jackson Owens) wrote…

agreed

should I remove all the 'byte in' changes or just the graph? Does every time series metric need a graph in order to show up?

jbowens · 2023-06-15T14:53:55Z

should I remove all the 'byte in' changes or just the graph?

just the graph

Does every time series metric need a graph in order to show up?

a time series metric without a graph in the DB Console will still be exported (eg, to prometheus) and may be graphed on demand using the custom chart tool.

raggar

makes sense, change made!

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jbowens and @RaduBerinde)

RaduBerinde

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

raggar

Is there a command I can use to format the .tsx file? I think it fails linting

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @jbowens and @RaduBerinde)

This change exposes the `WAL.BytesWritten` and `WAL.BytesIn` metric from Pebble to the Cockroach Side. Fixes: cockroachdb#104531 Release note: None

raggar · 2023-06-16T21:04:15Z

bors r+

craig · 2023-06-16T22:09:45Z

Build succeeded:

Bazel Essential CI (Cockroach)

raggar requested a review from a team as a code owner June 14, 2023 15:34

raggar changed the title ~~Expose WAL.BytesWritten~~ kvserver: Expose Bytes Written to WAL metric Jun 14, 2023

raggar requested a review from jbowens June 14, 2023 15:35

RaduBerinde requested a review from a team June 14, 2023 16:51

RaduBerinde reviewed Jun 14, 2023

View reviewed changes

jbowens reviewed Jun 14, 2023

View reviewed changes

raggar force-pushed the rahul_expose_bytes_metric branch from e68a36c to 2b1fe00 Compare June 14, 2023 18:05

raggar requested a review from a team June 14, 2023 18:05

raggar force-pushed the rahul_expose_bytes_metric branch from 2b1fe00 to 1ba4675 Compare June 14, 2023 18:06

raggar changed the title ~~kvserver: Expose Bytes Written to WAL metric~~ kvserver: Expose Bytes Written to WAL Jun 14, 2023

raggar commented Jun 14, 2023

View reviewed changes

raggar requested review from RaduBerinde and jbowens June 14, 2023 18:09

RaduBerinde reviewed Jun 14, 2023

View reviewed changes

jbowens reviewed Jun 14, 2023

View reviewed changes

raggar commented Jun 15, 2023

View reviewed changes

raggar force-pushed the rahul_expose_bytes_metric branch from 1ba4675 to c2ca0c2 Compare June 15, 2023 15:02

raggar requested review from jbowens and RaduBerinde June 15, 2023 15:02

raggar commented Jun 15, 2023

View reviewed changes

RaduBerinde approved these changes Jun 15, 2023

View reviewed changes

raggar commented Jun 15, 2023

View reviewed changes

kvserver: Expose WAL logical and total bytes written

b485fd9

This change exposes the `WAL.BytesWritten` and `WAL.BytesIn` metric from Pebble to the Cockroach Side. Fixes: cockroachdb#104531 Release note: None

raggar force-pushed the rahul_expose_bytes_metric branch from c2ca0c2 to b485fd9 Compare June 16, 2023 15:08

craig bot merged commit 6e641ff into cockroachdb:master Jun 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kvserver: Expose Bytes Written to WAL #104890

kvserver: Expose Bytes Written to WAL #104890

raggar commented Jun 14, 2023 •

edited

Loading

blathers-crl bot commented Jun 14, 2023

cockroach-teamcity commented Jun 14, 2023

RaduBerinde left a comment

jbowens left a comment

raggar left a comment

raggar left a comment

raggar left a comment

raggar left a comment

raggar left a comment

raggar left a comment

RaduBerinde left a comment

jbowens left a comment

raggar left a comment

jbowens commented Jun 15, 2023

raggar left a comment

RaduBerinde left a comment

raggar left a comment

raggar commented Jun 16, 2023

craig bot commented Jun 16, 2023

	// wal_bytes_written is the number of bytes written to the WAL (counter).
	uint64 wal_bytes_written = 28 [(gogoproto.jsontag) = ",omitempty"];

kvserver: Expose Bytes Written to WAL #104890

kvserver: Expose Bytes Written to WAL #104890

Conversation

raggar commented Jun 14, 2023 • edited Loading

blathers-crl bot commented Jun 14, 2023

cockroach-teamcity commented Jun 14, 2023

RaduBerinde left a comment

Choose a reason for hiding this comment

jbowens left a comment

Choose a reason for hiding this comment

raggar left a comment

Choose a reason for hiding this comment

raggar left a comment

Choose a reason for hiding this comment

raggar left a comment

Choose a reason for hiding this comment

raggar left a comment

Choose a reason for hiding this comment

raggar left a comment

Choose a reason for hiding this comment

raggar left a comment

Choose a reason for hiding this comment

RaduBerinde left a comment

Choose a reason for hiding this comment

jbowens left a comment

Choose a reason for hiding this comment

raggar left a comment

Choose a reason for hiding this comment

jbowens commented Jun 15, 2023

raggar left a comment

Choose a reason for hiding this comment

RaduBerinde left a comment

Choose a reason for hiding this comment

raggar left a comment

Choose a reason for hiding this comment

raggar commented Jun 16, 2023

craig bot commented Jun 16, 2023

raggar commented Jun 14, 2023 •

edited

Loading