Skip to content
This repository was archived by the owner on Mar 27, 2021. It is now read-only.

Conversation

@malish8632
Copy link
Contributor

@malish8632 malish8632 commented Aug 20, 2020

This change will drop metric with row key length longer than 4096 bytes.
It will also create counter metric so we could alert on high numbers.

@lmuhlha
Copy link
Member

lmuhlha commented Aug 20, 2020

Seems like checkstyle is still failing :(

@codecov
Copy link

codecov bot commented Aug 20, 2020

Codecov Report

Merging #686 into master will increase coverage by 0.01%.
The diff coverage is 71.42%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #686      +/-   ##
============================================
+ Coverage     53.88%   53.90%   +0.01%     
- Complexity     2962     2966       +4     
============================================
  Files           726      726              
  Lines         19426    19454      +28     
  Branches       1279     1282       +3     
============================================
+ Hits          10468    10486      +18     
- Misses         8513     8521       +8     
- Partials        445      447       +2     
Impacted Files Coverage Δ Complexity Δ
...istics/semantic/SemanticMetricBackendReporter.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...potify/heroic/metric/bigtable/BigtableBackend.java 90.38% <55.55%> (-1.25%) 59.00 <1.00> (+2.00) ⬇️
...oic/statistics/noop/NoopMetricBackendReporter.java 92.30% <100.00%> (+0.64%) 7.00 <1.00> (+1.00)
...m/spotify/heroic/test/AbstractMetricBackendIT.java 98.36% <100.00%> (+0.13%) 29.00 <2.00> (+2.00)
...com/spotify/heroic/aggregation/simple/MinBucket.kt 44.44% <0.00%> (-11.12%) 4.00% <0.00%> (-1.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa29634...076a0d2. Read the comment docs.

Copy link
Contributor

@sming sming left a comment

Choose a reason for hiding this comment

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

mainly just a compile-time alternative suggestion for the hugeRowKey flag and assertion - which only trips you up at run/debug time.

Copy link
Contributor

@sming sming left a comment

Choose a reason for hiding this comment

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

LGTM

@malish8632 malish8632 merged commit 9c9c6ca into master Aug 24, 2020
@malish8632 malish8632 deleted the rowkey-length branch August 24, 2020 17:24
@malish8632 malish8632 changed the title Drop metric with row key length bigger then BT limit Drop metric with row key length bigger than BT limit Aug 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants