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

v1.17: Parameterize max streams per ms (backport of #707) #1105

Merged
merged 2 commits into from
Apr 30, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Apr 29, 2024

Problem

The upper layer like jato-relayer may want different PPS for tpu and tpu_forward than the default value. Our validator may use different values for tpu vs forward. This changes makes it possible.

Summary of Changes

Make it parameter instead of the hard coded

Fixes #


This is an automatic backport of pull request #707 done by [Mergify](https://mergify.com).

@mergify mergify bot requested review from a team as code owners April 29, 2024 19:02
@mergify mergify bot added the conflicts label Apr 29, 2024
Copy link
Author

mergify bot commented Apr 29, 2024

Cherry-pick of f2aa4f0 has failed:

On branch mergify/bp/v1.17/pr-707
Your branch is up to date with 'origin/v1.17'.

You are currently cherry-picking commit f2aa4f0741.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   streamer/src/quic.rs

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	both modified:   client/src/connection_cache.rs
	both modified:   core/src/tpu.rs
	both modified:   quic-client/tests/quic_client.rs
	both modified:   streamer/src/nonblocking/quic.rs
	deleted by us:   streamer/src/nonblocking/stream_throttle.rs

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Make PPS a parameter instead of the hard coded

(cherry picked from commit f2aa4f0)

# Conflicts:
#	client/src/connection_cache.rs
#	core/src/tpu.rs
#	quic-client/tests/quic_client.rs
#	streamer/src/nonblocking/quic.rs
#	streamer/src/nonblocking/stream_throttle.rs
@codecov-commenter
Copy link

codecov-commenter commented Apr 29, 2024

Codecov Report

Attention: Patch coverage is 92.30769% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 81.6%. Comparing base (a0020cf) to head (c6119ce).

Additional details and impacted files
@@            Coverage Diff            @@
##            v1.17    #1105     +/-   ##
=========================================
- Coverage    81.6%    81.6%   -0.1%     
=========================================
  Files         806      806             
  Lines      219335   219345     +10     
=========================================
- Hits       179108   179067     -41     
- Misses      40227    40278     +51     

@t-nelson
Copy link

this is doing far more than the title implies. when did we decide to backport ema? this is too much

@lijunwangs
Copy link

this is doing far more than the title implies. when did we decide to backport ema? this is too much

stream_throttle.rs is a merge issue -- this PR does not need it. Removed.

@lijunwangs lijunwangs marked this pull request as draft April 30, 2024 02:54
@lijunwangs lijunwangs marked this pull request as ready for review April 30, 2024 04:37
@t-nelson
Copy link

this is doing far more than the title implies. when did we decide to backport ema? this is too much

stream_throttle.rs is a merge issue -- this PR does not need it. Removed.

woot!

@lijunwangs lijunwangs merged commit a7cfe9b into v1.17 Apr 30, 2024
33 checks passed
@lijunwangs lijunwangs deleted the mergify/bp/v1.17/pr-707 branch April 30, 2024 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants