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

remove solana-sys-tuner binary and all references #31682

Merged
merged 1 commit into from
May 17, 2023

Conversation

t-nelson
Copy link
Contributor

Problem

solana-sys-tuner has become more of a support burden than tool of convenience, suggesting that it has outlived its utility

Summary of Changes

remove it and all references to it

@t-nelson t-nelson requested review from mvines and joeaba May 16, 2023 22:05
@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Merging #31682 (bbc3a6e) into master (16381d8) will increase coverage by 0.0%.
The diff coverage is 82.7%.

@@           Coverage Diff           @@
##           master   #31682   +/-   ##
=======================================
  Coverage    81.9%    81.9%           
=======================================
  Files         737      736    -1     
  Lines      205985   205966   -19     
=======================================
  Hits       168719   168719           
+ Misses      37266    37247   -19     

Copy link
Contributor

@joeaba joeaba left a comment

Choose a reason for hiding this comment

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

Thanks!

@t-nelson t-nelson merged commit b422ac0 into solana-labs:master May 17, 2023
@t-nelson t-nelson deleted the rsst branch May 17, 2023 00:23
wen-coding pushed a commit to wen-coding/solana that referenced this pull request May 18, 2023
@ruuda
Copy link
Contributor

ruuda commented Jun 8, 2023

FYI for others who end up here and who wonder what we now need to do manually that solana-sys-tuner did previously.

Aside from the sysctls that are easily handled through /etc/sysctl.conf, sys-tuner used to set the priority of the solana-poh-serv thread. However, a git grep poh-serv does not bring up anything on current master, it seems the thread no longer exists, at least not under that name? It did exist at the time in 076e384, when solana-sys-tuner was introduced. Does this mean that this feature has been broken for a long time? It looks like #7494 removed the call to that function because it was “delaying scheduling of other critical functionality”. So the code in solana-sys-tuner was both broken and dead.

If it is still needed to bump the thread priority, it seems a bit weird to me to do that from an external program. The thread itself could easily set its own priority through

let sched_param = libc::sched_param {
    sched_priority: 99,
};
let sched_retval = unsafe {
    libc::pthread_setschedparam(libc::pthread_self(), libc::SCHED_OTHER, &sched_param)
};
match sched_retval {
    0 => eprintln!("PoH thread priority upgraded successfully."),
    libc::EPERM => eprintln!("PoH thread was not allowed to change its scheduling params. Consider granting CAP_SYS_NICE."),
    _ => eprintln!("Unknown error occurred while setting thread priority."),
};

It could even set SCHED_RR instead of SCHED_OTHER, maybe some of the other high-priority tasks need to all be pinned to different cores to ensure they don’t delay each other.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants