Implement ingester API to return stream rates #7176

MasslessParticle · 2022-09-15T21:56:33Z

This PR introduces an API on the Ingester that reports all current streams in memory and their current rate in bytes/second. The reponse from the API contains a slice of StreamRate structs:

type StreamRate struct {
	StreamHash        uint64 `protobuf:"varint,1,opt,name=streamHash,proto3" json:"streamHash,omitempty"`
	StreamHashNoShard uint64 `protobuf:"varint,2,opt,name=streamHashNoShard,proto3" json:"streamHashNoShard,omitempty"`
	Rate              int64  `protobuf:"varint,3,opt,name=rate,proto3" json:"rate,omitempty"`
}

In the interest of space on the caller and in transmission, only hashes and rates are sent.

StreamHash will be used to deduplicate replicas
StreamHashNoShard will be used to aggregate a sharded stream's total rate.

Rate calculation will be in a follow on PR

grafanabot · 2022-09-15T22:11:08Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0.2%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

jeschkies

Interesting. I've always wondered how rate limiting was implemented. I wish the algorithm would expose the actual rate :/

jeschkies · 2022-09-16T16:01:57Z

pkg/ingester/stream.go

@@ -387,6 +397,7 @@ func (s *stream) validateEntries(entries []logproto.Entry, isReplay bool) ([]log
 		}
 	}

+	s.rateCalculator.Record(int64(totalBytes))


Why is this not using validBytes?

This preserves the current behavior where stream rate limits are calculated against all the bytes in the stream, not just the valid ones. I assume we do it this way because the rate limit is to protect the ingester and we process every entry, valid or not.

jeschkies · 2022-09-16T16:04:09Z

pkg/ingester/rate_calculator.go

+	t := time.NewTicker(1000 / bucketCount * time.Millisecond)
+	for range t.C {
+		rate := c.sample.Swap(0)
+		c.storeNewRate(rate)
+	}


Is this never stopped?

pkg/logproto/logproto.proto

pkg/ingester/rate_calculator_test.go

pkg/ingester/instance_test.go

pkg/ingester/rate_calculator.go

**What this PR does / why we need it**: Stop the rateCalculator timer when stream is about to be removed. **Which issue(s) this PR fixes**: N/A

grafanabot · 2022-09-26T23:10:03Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0.2%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-10-03T21:25:19Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0.1%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-10-03T22:48:03Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

-           ingester	-0.2%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

DylanGuedes

lgtm

just a small observation that shouldn't change anything

pkg/ingester/ingester.go

grafanabot · 2022-10-05T16:31:41Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

-           ingester	-0.3%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

This PR introduces an API on the Ingester that reports all current streams in memory and their current rate in bytes/second. The reponse from the API contains a slice of `StreamRate` structs: ```go type StreamRate struct { StreamHash uint64 `protobuf:"varint,1,opt,name=streamHash,proto3" json:"streamHash,omitempty"` StreamHashNoShard uint64 `protobuf:"varint,2,opt,name=streamHashNoShard,proto3" json:"streamHashNoShard,omitempty"` Rate int64 `protobuf:"varint,3,opt,name=rate,proto3" json:"rate,omitempty"` } ``` In the interest of space on the caller and in transmission, only hashes and rates are sent. - `StreamHash` will be used to deduplicate replicas - `StreamHashNoShard` will be used to aggregate a sharded stream's total rate. Co-authored-by: Dylan Guedes <djmgguedes@gmail.com>

Implement ingester API to return stream rates

7623be3

MasslessParticle requested review from DylanGuedes and owen-d September 15, 2022 21:56

MasslessParticle requested a review from a team as a code owner September 15, 2022 21:56

pull-request-size bot added the size/XXL label Sep 15, 2022

MasslessParticle added 2 commits September 15, 2022 15:58

reuse buf to avoid allocations

b7d52bd

lint

a48423f

jeschkies reviewed Sep 16, 2022

View reviewed changes

DylanGuedes reviewed Sep 16, 2022

View reviewed changes

DylanGuedes added 3 commits September 26, 2022 10:12

Test that RateCalculator rates becomes 0 later.

4a45fac

Remove unnecessary Println statement.

f944f35

Call stop for recordSample timer.

e7b1f1f

MasslessParticle commented Sep 26, 2022

View reviewed changes

pkg/ingester/rate_calculator.go Outdated Show resolved Hide resolved

DylanGuedes and others added 4 commits September 26, 2022 15:29

Ingester stream api stop (#7248)

18deefc

**What this PR does / why we need it**: Stop the rateCalculator timer when stream is about to be removed. **Which issue(s) this PR fixes**: N/A

Merge branch 'main' into tpatterson/ingester-stream-api

b372426

review feedback

c084286

break import cycle

2e7ac28

MasslessParticle mentioned this pull request Sep 29, 2022

Revert "Hide duplicates caused by stream sharding (#7005)" #7031

Merged

reduce rate resolution to save CPU

79d62f9

MasslessParticle added 2 commits October 3, 2022 16:38

remove rate calculations

0b0c42d

Merge branch 'main' into tpatterson/ingester-stream-api

b2b88bf

DylanGuedes approved these changes Oct 4, 2022

View reviewed changes

pkg/ingester/ingester.go Show resolved Hide resolved

MasslessParticle added 2 commits October 5, 2022 10:11

Merge branch 'main' into tpatterson/ingester-stream-api

cf05283

todo for future improvements

2824738

MasslessParticle merged commit e3e4e27 into main Oct 5, 2022

MasslessParticle deleted the tpatterson/ingester-stream-api branch October 5, 2022 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement ingester API to return stream rates #7176

Implement ingester API to return stream rates #7176

MasslessParticle commented Sep 15, 2022 •

edited

Loading

grafanabot commented Sep 15, 2022

jeschkies left a comment

jeschkies Sep 16, 2022

MasslessParticle Sep 20, 2022

jeschkies Sep 16, 2022

grafanabot commented Sep 26, 2022

grafanabot commented Oct 3, 2022

grafanabot commented Oct 3, 2022

DylanGuedes left a comment

grafanabot commented Oct 5, 2022

Implement ingester API to return stream rates #7176

Implement ingester API to return stream rates #7176

Conversation

MasslessParticle commented Sep 15, 2022 • edited Loading

grafanabot commented Sep 15, 2022

jeschkies left a comment

Choose a reason for hiding this comment

jeschkies Sep 16, 2022

Choose a reason for hiding this comment

MasslessParticle Sep 20, 2022

Choose a reason for hiding this comment

jeschkies Sep 16, 2022

Choose a reason for hiding this comment

grafanabot commented Sep 26, 2022

grafanabot commented Oct 3, 2022

grafanabot commented Oct 3, 2022

DylanGuedes left a comment

Choose a reason for hiding this comment

grafanabot commented Oct 5, 2022

MasslessParticle commented Sep 15, 2022 •

edited

Loading