STAR-865 Refactor and port metrics code from cndb by djatnieks · Pull Request #247 · datastax/cassandra

djatnieks · 2021-09-08T21:31:31Z

Changes are mainly around:

First commit: ClientRequestMetrics and subclasses from cndb-884 and riptano/bdp#19515
and
Second commit: Porting MicrometerChunkCacheMetrics from CNDB-780 - Add Micrometer metrics for the chunk cache

Some differences from bdp/cndb are mention in code comments.

and riptano/bdp/#19515 Co-authored-by: Zhao Yang <jasonstack.zhao@gmail.com> Co-authored-by: Jake Luciani <tjake@users.noreply.github.com>

CNDB-161 Add MicrometerMetrics class CNDB-780 Add Micrometer metrics for the chunk cache Co-authored-by: Stefania Alborghetti <stefania.alborghetti@datastax.com>

mfleming

Nice! This looks good to me, Dan. There's only one comment about using CassandraRelevantProperties in a couple of places.

djatnieks · 2021-09-09T21:55:09Z

+              .column(RECENT_REQUEST_RATE_PER_SECOND, metrics.requestsFifteenMinuteRate())
+              .column(RECENT_HIT_RATE_PER_SECOND, metrics.hitsFifteenMinuteRate());
+    }
+


Wanted to point out that this differs from the original; there the CacheMetrics constructor is explicitly calling CachesSystemView.addRowData to register cache metrics into the system view.

In this code ChunkCache instance metrics are added by CachesTable.data() using ChunkCache.instance.metrics, which is ChunkCacheMetrics interface; so by having addRow taking that type we will get metrics for any implementing class.

I added ChunkCacheMetrics.requestsFifteenMinuteRate and hitsFifteenMinuteRate for that; only CodahaleChunkCacheMetrics provides those values. MicrometerChunkCacheMetrics will return zeros.

djatnieks · 2021-09-09T22:06:41Z

+    long requestsFifteenMinuteRate();
+
+    long hitsFifteenMinuteRate();


As mentioned in CachesTable, these were added in addition to the ported code.

djatnieks · 2021-09-09T22:08:57Z

@mfleming I applied the changes from your review and also added some comments to point out a couple areas where the ported code differs from the original. Thanks!

mfleming

lgtm!

sonarqubecloud · 2021-09-15T21:31:47Z

Kudos, SonarCloud Quality Gate passed!

6 Bugs
0 Vulnerabilities
0 Security Hotspots
10 Code Smells

82.1% Coverage
0.0% Duplication

* STAR-865: Porting metrics from cndb-884, riptano/bdp@03b23db6a5697baaf71d46d661c0ac1c908bc33e and riptano/bdp/#19515 Co-authored-by: Zhao Yang <jasonstack.zhao@gmail.com> Co-authored-by: Jake Luciani <tjake@users.noreply.github.com> * STAR-865: Porting MicrometerChunkCacheMetrics from: CNDB-161 Add MicrometerMetrics class CNDB-780 Add Micrometer metrics for the chunk cache Co-authored-by: Stefania Alborghetti <stefania.alborghetti@datastax.com> (cherry picked from commit 5e0d889) fix ConcurrencyFactorTest the metrics require reset if we want to measure, especially the max values

* STAR-865: Porting metrics from cndb-884, riptano/bdp@03b23db6a5697baaf71d46d661c0ac1c908bc33e and riptano/bdp/#19515 Co-authored-by: Zhao Yang <jasonstack.zhao@gmail.com> Co-authored-by: Jake Luciani <tjake@users.noreply.github.com> * STAR-865: Porting MicrometerChunkCacheMetrics from: CNDB-161 Add MicrometerMetrics class CNDB-780 Add Micrometer metrics for the chunk cache Co-authored-by: Stefania Alborghetti <stefania.alborghetti@datastax.com> (cherry picked from commit 5e0d889) fix ConcurrencyFactorTest the metrics require reset if we want to measure, especially the max values (cherry picked from commit 3f89514) (cherry picked from commit c51b4c4)

* STAR-865: Porting metrics from cndb-884, riptano/bdp@03b23db6a5697baaf71d46d661c0ac1c908bc33e and riptano/bdp/#19515 Co-authored-by: Zhao Yang <jasonstack.zhao@gmail.com> Co-authored-by: Jake Luciani <tjake@users.noreply.github.com> * STAR-865: Porting MicrometerChunkCacheMetrics from: CNDB-161 Add MicrometerMetrics class CNDB-780 Add Micrometer metrics for the chunk cache Co-authored-by: Stefania Alborghetti <stefania.alborghetti@datastax.com> (cherry picked from commit 5e0d889) fix ConcurrencyFactorTest the metrics require reset if we want to measure, especially the max values (cherry picked from commit 3f89514) (cherry picked from commit c51b4c4) (cherry picked from commit bc8b4ed)

* STAR-865: Porting metrics from cndb-884, riptano/bdp@03b23db and riptano/bdp/#19515 Co-authored-by: Zhao Yang <jasonstack.zhao@gmail.com> Co-authored-by: Jake Luciani <tjake@users.noreply.github.com> * STAR-865: Porting MicrometerChunkCacheMetrics from: CNDB-161 Add MicrometerMetrics class CNDB-780 Add Micrometer metrics for the chunk cache Co-authored-by: Stefania Alborghetti <stefania.alborghetti@datastax.com> (cherry picked from commit 5e0d889) fix ConcurrencyFactorTest the metrics require reset if we want to measure, especially the max values (cherry picked from commit 3f89514) (cherry picked from commit c51b4c4) (cherry picked from commit bc8b4ed)

* STAR-865: Porting metrics from cndb-884, riptano/bdp@03b23db and riptano/bdp/#19515 Co-authored-by: Zhao Yang <jasonstack.zhao@gmail.com> Co-authored-by: Jake Luciani <tjake@users.noreply.github.com> * STAR-865: Porting MicrometerChunkCacheMetrics from: CNDB-161 Add MicrometerMetrics class CNDB-780 Add Micrometer metrics for the chunk cache Co-authored-by: Stefania Alborghetti <stefania.alborghetti@datastax.com> (cherry picked from commit 5e0d889) fix ConcurrencyFactorTest the metrics require reset if we want to measure, especially the max values (cherry picked from commit 3f89514) (cherry picked from commit c51b4c4) (cherry picked from commit bc8b4ed) (cherry picked from commit d45d8e2)

* STAR-865: Porting metrics from cndb-884, riptano/bdp@03b23db6a5697baaf71d46d661c0ac1c908bc33e and riptano/bdp/#19515 Co-authored-by: Zhao Yang <jasonstack.zhao@gmail.com> Co-authored-by: Jake Luciani <tjake@users.noreply.github.com> * STAR-865: Porting MicrometerChunkCacheMetrics from: CNDB-161 Add MicrometerMetrics class CNDB-780 Add Micrometer metrics for the chunk cache Co-authored-by: Stefania Alborghetti <stefania.alborghetti@datastax.com> (cherry picked from commit 5e0d889) fix ConcurrencyFactorTest the metrics require reset if we want to measure, especially the max values (cherry picked from commit 3f89514) (cherry picked from commit c51b4c4) (cherry picked from commit bc8b4ed) (cherry picked from commit d45d8e2) (cherry picked from commit 022df3a)

* STAR-865: Porting metrics from cndb-884, riptano/bdp@03b23db and riptano/bdp/#19515 Co-authored-by: Zhao Yang <jasonstack.zhao@gmail.com> Co-authored-by: Jake Luciani <tjake@users.noreply.github.com> * STAR-865: Porting MicrometerChunkCacheMetrics from: CNDB-161 Add MicrometerMetrics class CNDB-780 Add Micrometer metrics for the chunk cache Co-authored-by: Stefania Alborghetti <stefania.alborghetti@datastax.com> (cherry picked from commit 5e0d889) fix ConcurrencyFactorTest the metrics require reset if we want to measure, especially the max values (cherry picked from commit 3f89514) (cherry picked from commit c51b4c4) (cherry picked from commit bc8b4ed) (cherry picked from commit d45d8e2) (cherry picked from commit 022df3a) STAR-865 Rebase compile fixes: * Add dependencies for micrometer-core and LatencyUtils * Delete ClientRequestsMetricsHolder and replace usages with ClientRequestsMetricsProvider

* STAR-865: Porting metrics from cndb-884, riptano/bdp@03b23db and riptano/bdp/#19515 Co-authored-by: Zhao Yang <jasonstack.zhao@gmail.com> Co-authored-by: Jake Luciani <tjake@users.noreply.github.com> * STAR-865: Porting MicrometerChunkCacheMetrics from: CNDB-161 Add MicrometerMetrics class CNDB-780 Add Micrometer metrics for the chunk cache Co-authored-by: Stefania Alborghetti <stefania.alborghetti@datastax.com> (cherry picked from commit 5e0d889) fix ConcurrencyFactorTest the metrics require reset if we want to measure, especially the max values (cherry picked from commit 3f89514) (cherry picked from commit c51b4c4) (cherry picked from commit bc8b4ed) (cherry picked from commit d45d8e2) (cherry picked from commit 022df3a) STAR-865 Rebase compile fixes: * Add dependencies for micrometer-core and LatencyUtils * Delete ClientRequestsMetricsHolder and replace usages with ClientRequestsMetricsProvider STAR-865 Fix FailedBootstrapTest

* STAR-865: Porting metrics from cndb-884, riptano/bdp@03b23db and riptano/bdp/#19515 Co-authored-by: Zhao Yang <jasonstack.zhao@gmail.com> Co-authored-by: Jake Luciani <tjake@users.noreply.github.com> * STAR-865: Porting MicrometerChunkCacheMetrics from: CNDB-161 Add MicrometerMetrics class CNDB-780 Add Micrometer metrics for the chunk cache Co-authored-by: Stefania Alborghetti <stefania.alborghetti@datastax.com> (cherry picked from commit 5e0d889) fix ConcurrencyFactorTest the metrics require reset if we want to measure, especially the max values (cherry picked from commit 3f89514) (cherry picked from commit c51b4c4) (cherry picked from commit bc8b4ed) (cherry picked from commit d45d8e2) (cherry picked from commit 022df3a) STAR-865 Rebase compile fixes: * Add dependencies for micrometer-core and LatencyUtils * Delete ClientRequestsMetricsHolder and replace usages with ClientRequestsMetricsProvider STAR-865 Fix FailedBootstrapTest (Rebase of commit f0f3e3a)

djatnieks and others added 2 commits September 3, 2021 11:08

STAR-865: Porting metrics from cndb-884, riptano/bdp@03b23db

37bd3d1

and riptano/bdp/#19515 Co-authored-by: Zhao Yang <jasonstack.zhao@gmail.com> Co-authored-by: Jake Luciani <tjake@users.noreply.github.com>

STAR-865: Porting MicrometerChunkCacheMetrics from:

75ad4e4

CNDB-161 Add MicrometerMetrics class CNDB-780 Add Micrometer metrics for the chunk cache Co-authored-by: Stefania Alborghetti <stefania.alborghetti@datastax.com>

mfleming self-requested a review September 9, 2021 06:05

mfleming suggested changes Sep 9, 2021

View reviewed changes

Comment thread src/java/org/apache/cassandra/metrics/ChunkCacheMetrics.java Outdated

Comment thread src/java/org/apache/cassandra/metrics/CoordinatorClientRequestMetricsProvider.java Outdated

STAR-865 Apply review comments

1f03782

djatnieks commented Sep 9, 2021

View reviewed changes

Comment thread src/java/org/apache/cassandra/metrics/ChunkCacheMetrics.java

djatnieks commented Sep 9, 2021

View reviewed changes

djatnieks requested a review from mfleming September 9, 2021 22:07

mfleming approved these changes Sep 13, 2021

View reviewed changes

djatnieks added 3 commits September 15, 2021 12:41

STAR-865 Add tests to improve test coverage

5416d65

STAR-865 Fix chunk cache size in test method

526fefe

STAR-865 Add CachesTableTest

c738476

djatnieks merged commit 5e0d889 into ds-trunk Sep 16, 2021

djatnieks deleted the STAR-865-metrics branch September 16, 2021 16:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STAR-865 Refactor and port metrics code from cndb#247

STAR-865 Refactor and port metrics code from cndb#247
djatnieks merged 6 commits intods-trunkfrom
STAR-865-metrics

djatnieks commented Sep 8, 2021

Uh oh!

mfleming left a comment

Uh oh!

Uh oh!

Uh oh!

djatnieks Sep 9, 2021

Uh oh!

Uh oh!

djatnieks Sep 9, 2021

Uh oh!

djatnieks commented Sep 9, 2021

Uh oh!

mfleming left a comment

Uh oh!

sonarqubecloud Bot commented Sep 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		long requestsFifteenMinuteRate();

		long hitsFifteenMinuteRate();

Conversation

djatnieks commented Sep 8, 2021

Uh oh!

mfleming left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

djatnieks Sep 9, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

djatnieks Sep 9, 2021

Choose a reason for hiding this comment

Uh oh!

djatnieks commented Sep 9, 2021

Uh oh!

mfleming left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Sep 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants