receive: Add per-each-write limit to the allowed number of samples

[spaparaju]

Fix for #3964

• I added CHANGELOG entry for this change.
• Change is not relevant to the end user.

Changes

1. Added configurable 'receive.samples-limit-per-each-write' at Thanos Receive component to limit the number of allowed samples per each write API call
2. Update the pkg/receive/handler to enforce this limit
3. Added unit tests at pkg/receive/handler_test.go

Verification

Verified with unit tests at pkg/receive/handler_test.go

[kakkoyun]

Suggested change

	- [3963](https://github.com/thanos-io/thanos/pull/3963) add per-each-write limit to the allowed number of samples.
	- [3963](https://github.com/thanos-io/thanos/pull/3963) Receive: Add per-each-write limit to the allowed number of samples. (Default: 5000)

[brancz]

The default seems rather low. Remote write will soon be based on commits by Prometheus, which means that entire scrapes will be sent within one remote write request. Especially in Kubernetes land it's quite common to have 100k series per scrape (eg. apiserver and kube-state-metrics). I wonder, is the amount of samples of a single request really the concern? Wouldn't limiting the number of active series be the better protection mechanism? Or samples per second over a window?

Regardless of the strategy I would also expect this to be a per-tenant configuration. (although I guess starting with a default would be ok)

[squat]

@spaparaju any thoughts on @brancz's idea? To make this per-tenant, should we have the limit be specified in the hashring configurations?

[spaparaju]

The default seems rather low. Remote write will soon be based on commits by Prometheus, which means that entire scrapes will be sent within one remote write request. Especially in Kubernetes land it's quite common to have 100k series per scrape (eg. apiserver and kube-state-metrics). I wonder, is the amount of samples of a single request really the concern?
Yes, default seems low to me (after thinking. a bit more on the comment). Maybe 50K seems reasonable default.
This idea is to get a counterpart at the service / storage side to match the current Prometheus remoteWrite 'max_samples_per_send' and protection mechanism at an individual remoteWrite call (any one remoteWrite call can not bring a service down). This I guess helps to publish the contract on the service side to backoff the samples beyond limit per each send.
Wouldn't limiting the number of active series be the better protection mechanism? Or samples per second over a window?
Yes, tracking active time series, samples per time window are quite useful protection measures across multiple remoteWrite calls.

Regardless of the strategy I would also expect this to be a per-tenant configuration. (although I guess starting with a default would be ok)

[stale]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.