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

default staked client in LocalCluster #716

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

apfitzge
Copy link

Problem

Summary of Changes

  • LocalCluster connection cache defaults to use a client id with stake override to treat it like staked connection

Fixes #

@apfitzge
Copy link
Author

For comparison, local-cluster 2 went from 27m -> 14m with this change. 5 went from 19m -> 10m.
Others were about the same

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.9%. Comparing base (72ee270) to head (f62c0ff).
Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #716   +/-   ##
=======================================
  Coverage    81.9%    81.9%           
=======================================
  Files         851      851           
  Lines      230726   230726           
=======================================
+ Hits       189009   189037   +28     
+ Misses      41717    41689   -28     

@apfitzge apfitzge marked this pull request as ready for review April 10, 2024 19:28
Copy link

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

match on a bool kinda makes my eye twitch. lgtm otherwise

@apfitzge
Copy link
Author

match on a bool kinda makes my eye twitch. lgtm otherwise

I just copied the match from the original location below. 🤔

@apfitzge apfitzge merged commit e91a5e2 into anza-xyz:master Apr 10, 2024
38 checks passed
@apfitzge apfitzge deleted the local_cluster_staked_client branch April 10, 2024 20:33
Copy link

mergify bot commented Apr 10, 2024

Backports to the stable branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule.

Copy link

mergify bot commented Apr 10, 2024

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

mergify bot pushed a commit that referenced this pull request Apr 10, 2024
* default staked client in LocalCluster

* fix underflow

(cherry picked from commit e91a5e2)

# Conflicts:
#	local-cluster/tests/local_cluster.rs
mergify bot pushed a commit that referenced this pull request Apr 10, 2024
* default staked client in LocalCluster

* fix underflow

(cherry picked from commit e91a5e2)

# Conflicts:
#	local-cluster/tests/local_cluster.rs
@t-nelson
Copy link

match on a bool kinda makes my eye twitch. lgtm otherwise

I just copied the match from the original location below. 🤔

yeah saw that. my eye twitched on the red lines even

@apfitzge apfitzge mentioned this pull request Apr 10, 2024
@apfitzge
Copy link
Author

Created an commmented on #726, didn't wanna delay backports on a style change since this is blocking other backports.
But yeah, we should definitely be using an if here.

t-nelson pushed a commit that referenced this pull request Apr 10, 2024
* default staked client in LocalCluster (#716)

* default staked client in LocalCluster

* fix underflow

(cherry picked from commit e91a5e2)

# Conflicts:
#	local-cluster/tests/local_cluster.rs

* resolve conflicts

---------

Co-authored-by: Andrew Fitzgerald <apfitzge@gmail.com>
t-nelson pushed a commit that referenced this pull request Apr 10, 2024
* default staked client in LocalCluster (#716)

* default staked client in LocalCluster

* fix underflow

(cherry picked from commit e91a5e2)

# Conflicts:
#	local-cluster/tests/local_cluster.rs

* resolve conflicts

---------

Co-authored-by: Andrew Fitzgerald <apfitzge@gmail.com>
anwayde pushed a commit to firedancer-io/agave that referenced this pull request Jul 23, 2024
… (anza-xyz#724)

* default staked client in LocalCluster (anza-xyz#716)

* default staked client in LocalCluster

* fix underflow

(cherry picked from commit e91a5e2)

# Conflicts:
#	local-cluster/tests/local_cluster.rs

* resolve conflicts

---------

Co-authored-by: Andrew Fitzgerald <apfitzge@gmail.com>
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.

3 participants