Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Receive: Dont rebatch already replicated requests #5818

Closed
wants to merge 3 commits into from

Conversation

matej-g
Copy link
Collaborator

@matej-g matej-g commented Oct 25, 2022

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

Changes

Part of #5807.

Since change in #5604, we are already hashing each time series by endpoint and replica, meaning that replicated requests will now always be sent to the actual endpoint to which they are supposed to be written. This means the 're-batching' of series upon receiving a replicated request is redundant and can be skipped.

Verification

Tests are succeeding locally. I also added one more benchmark with replicated request to see changes:

name                                                                                   old time/op    new time/op    delta
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12                           1.00ms ± 7%    0.97ms ± 8%     ~     (p=0.123 n=10+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-with-replicated-12            992µs ± 7%     817µs ± 6%  -17.60%  (p=0.000 n=10+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12              1.15ms ±10%    1.14ms ± 7%     ~     (p=0.684 n=10+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12                          10.2ms ± 6%    10.0ms ±16%     ~     (p=0.165 n=10+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-with-replicated-12          10.2ms ± 5%     8.0ms ± 6%  -21.21%  (p=0.000 n=10+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12             12.1ms ± 7%    11.7ms ± 3%     ~     (p=0.146 n=10+8)
HandlerReceiveHTTP/typical_labels_under_1KB,_20000_of_them/OK-12                         41.3ms ± 4%    41.2ms ±11%     ~     (p=0.243 n=9+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_20000_of_them/OK-with-replicated-12         41.4ms ± 6%    34.2ms ±10%  -17.22%  (p=0.000 n=10+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_20000_of_them/conflict_errors-12            48.3ms ±11%    48.5ms ± 9%     ~     (p=0.739 n=10+10)
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12                    81.7ms ± 3%    85.8ms ±11%     ~     (p=0.203 n=8+10)
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-with-replicated-12    87.8ms ±16%    76.5ms ±18%  -12.88%  (p=0.001 n=10+10)
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12       89.4ms ±13%    87.0ms ±10%     ~     (p=0.529 n=10+10)

name                                                                                   old alloc/op   new alloc/op   delta
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12                           1.15MB ± 0%    1.15MB ± 0%     ~     (p=0.661 n=10+9)
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-with-replicated-12           1.15MB ± 0%    1.07MB ± 0%   -6.98%  (p=0.000 n=9+9)
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12              1.41MB ± 0%    1.41MB ± 0%     ~     (p=0.781 n=10+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12                          13.1MB ± 0%    13.1MB ± 0%     ~     (p=0.190 n=10+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-with-replicated-12          13.1MB ± 0%    11.5MB ± 0%  -12.27%  (p=0.000 n=10+9)
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12             15.5MB ± 0%    15.5MB ± 0%     ~     (p=0.315 n=10+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_20000_of_them/OK-12                         52.6MB ± 1%    52.8MB ± 1%     ~     (p=0.190 n=9+9)
HandlerReceiveHTTP/typical_labels_under_1KB,_20000_of_them/OK-with-replicated-12         52.9MB ± 2%    46.4MB ± 1%  -12.23%  (p=0.000 n=10+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_20000_of_them/conflict_errors-12            62.0MB ± 0%    62.0MB ± 0%     ~     (p=0.063 n=10+10)
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12                     110MB ± 0%     110MB ± 0%     ~     (p=0.950 n=9+10)
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-with-replicated-12     110MB ± 0%     110MB ± 0%     ~     (p=0.982 n=9+9)
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12        110MB ± 0%     110MB ± 0%     ~     (p=0.072 n=10+10)

name                                                                                   old allocs/op  new allocs/op  delta
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12                            3.10k ± 0%     3.10k ± 0%     ~     (all equal)
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-with-replicated-12            3.10k ± 0%     3.09k ± 0%   -0.35%  (p=0.000 n=10+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12               6.63k ± 0%     6.63k ± 0%     ~     (p=0.650 n=10+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12                           30.3k ± 0%     30.3k ± 0%     ~     (p=0.423 n=10+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-with-replicated-12           30.3k ± 0%     30.3k ± 0%   -0.06%  (p=0.000 n=10+9)
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12              65.1k ± 0%     65.1k ± 0%     ~     (p=1.000 n=10+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_20000_of_them/OK-12                           121k ± 0%      121k ± 0%     ~     (p=0.763 n=10+9)
HandlerReceiveHTTP/typical_labels_under_1KB,_20000_of_them/OK-with-replicated-12           121k ± 0%      121k ± 0%   -0.02%  (p=0.000 n=10+10)
HandlerReceiveHTTP/typical_labels_under_1KB,_20000_of_them/conflict_errors-12              260k ± 0%      260k ± 0%     ~     (p=0.064 n=10+10)
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12                      88.8 ± 2%      87.7 ± 1%     ~     (p=0.154 n=10+10)
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-with-replicated-12      89.7 ± 2%      84.3 ± 4%   -5.99%  (p=0.000 n=9+10)
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12          218 ± 1%       219 ± 1%   +0.61%  (p=0.042 n=9+9)

Note

We should make obvious to our users that this change is not fully compatible between older and newer versions of receive. In particular if any user would be running a mix of pre-0.28.0 receivers and receivers with this change (which can happen during a rollout), the batching would not work correctly. It is therefore important to instruct users to first update their receivers to version to either 0.28.0 or 0.29.0 and only then to newer version with this change.

fpetkovski
fpetkovski previously approved these changes Oct 25, 2022
Copy link
Contributor

@fpetkovski fpetkovski left a comment

Choose a reason for hiding this comment

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

👍

@fpetkovski
Copy link
Contributor

Hopefully in the long term we can end up with your idea of calculating all replicas for all batches in one place, but this should be good enough for now.

jessicalins
jessicalins previously approved these changes Oct 25, 2022
Copy link
Contributor

@jessicalins jessicalins left a comment

Choose a reason for hiding this comment

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

LGTM :)

saswatamcode
saswatamcode previously approved these changes Oct 25, 2022
Copy link
Member

@saswatamcode saswatamcode left a comment

Choose a reason for hiding this comment

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

LGTM!

Signed-off-by: Matej Gera <matejgera@gmail.com>
Signed-off-by: Matej Gera <matejgera@gmail.com>
Signed-off-by: Matej Gera <matejgera@gmail.com>
@saswatamcode
Copy link
Member

Can this be rebased? 🙂

@matej-g
Copy link
Collaborator Author

matej-g commented Nov 29, 2022

Closing as this has been superseded by #5910

@matej-g matej-g closed this Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants