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

Be less nice #798

Merged
merged 4 commits into from
Jun 4, 2020
Merged

Be less nice #798

merged 4 commits into from
Jun 4, 2020

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented May 28, 2020

By submitting this pull request, I confirm the following:

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

10


Set nice value of pihole-FTL to increase DNS server performance. The new niceness default to -10 (lower is less nice = getting more CPU cycles for our own). The value can be changed through a config option. Changing the niceness can be avoided by setting NICE=0 (this will set 0 as niceness, which is the default) or NICE=-999 (this will skip trying to set the niceness, altogether).

If the user is not allowed to change their niceness, FTL will log:

NICE: Cannot change niceness to -10 (permission denied)

If the user specified an invalid niceness, it will be clamped to the according extremum (+/- 20):

NICE: Set process niceness to -20 (asked for -100)

If everything is fine, the output is:

NICE: Set process niceness to -12

TODO (pi-hole/core): Add CAP_SYS_NICE to FTL so it is allowed to change its own niceness when when running as user pihole. Without this, only root can change the niceness of processes.

…rformance.

Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER added this to the v5.1 milestone May 28, 2020
@DL6ER DL6ER requested a review from a team May 28, 2020 18:41
Signed-off-by: DL6ER <dl6er@dl6er.de>
@@ -367,6 +370,20 @@ void read_FTLconf(void)
if(buffer != NULL && strcasecmp(buffer, "false") == 0)
config.block_esni = false;

// NICE
// Shall we change the nice of the current process?
// defaults to: -10 (can be disabled by setting value to -999)
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason for the default to be -10? This is a question we will be asked many times.

Copy link
Member Author

@DL6ER DL6ER May 28, 2020

Choose a reason for hiding this comment

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

Is there a reason for the default to be -10?

No

Quoting man sched:

  With the advent of the CFS scheduler in kernel 2.6.23, Linux adopted
  an algorithm that causes relative differences in nice values to have
  a much stronger effect.  **In the current implementation, each unit of
  difference in the nice values of two processes results in a factor of
  1.25 in the degree to which the scheduler favors the higher priority
  process.**  This causes very low nice values (+19) to truly provide
  little CPU to a process whenever there is any other higher priority
  load on the system, and makes high nice values (-20) deliver most of
  the CPU to applications that require it (e.g., some audio applica‐
  tions).

This means that a niceness of -10 will result in roughly 9x more cycles given to pihole-FTL compared to other processes when they are all asking for 100% CPU usage.

…change the nicencess.

Signed-off-by: DL6ER <dl6er@dl6er.de>
@yubiuser
Copy link
Member

Out of curiosity: Will I see DNS server performance increase in my small home network where my CPU is already dying of boredom? Or rephrased: how many clients do I need before I'll profit from this PR?

@DL6ER
Copy link
Member Author

DL6ER commented May 29, 2020

@yubiuser It will not affect you at all when you run Pi-hole as the only process on your machine. However, when installing Pi-hole next other compute-intense, i.e., CPU-hungry, processes like a database or e-mail server, maybe a VPN service, etc., then this change will ensure that Pi-hole receives a greater share of the available computing power (it will be less equally distributed among the processes, hence, less "nice" to them).

PromoFaux
PromoFaux previously approved these changes Jun 3, 2020
@PromoFaux
Copy link
Member

Oh. Merge conflicts

Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER
Copy link
Member Author

DL6ER commented Jun 4, 2020

@PromoFaux Merge conflict resolved

@DL6ER DL6ER merged commit e91b7a7 into development Jun 4, 2020
@DL6ER DL6ER deleted the new/be_less_nice branch June 4, 2020 11:39
@DL6ER DL6ER mentioned this pull request Jul 5, 2020
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-5-1-released/35577/1

@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/raspberry-pi-4-hohe-last-keine-domain-auflosung/37152/4

@AlaricSenat AlaricSenat mentioned this pull request Nov 6, 2020
5 tasks
cyounkins added a commit to cyounkins/docker-pi-hole that referenced this pull request Jan 20, 2021
See pi-hole/FTL#798

Without this capability, users will see `WARNING: Required Linux capability CAP_SYS_NICE not available` in FTL log
cyounkins added a commit to cyounkins/docker-pi-hole that referenced this pull request Jan 20, 2021
See pi-hole/FTL#798

Without this capability, users will see `WARNING: Required Linux capability CAP_SYS_NICE not available` in FTL log

Signed-off-by: Craig Younkins <cyounkins@gmail.com>
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.

5 participants