The "power of 2" is in fact a multiple of 2 in unbound.sh

PtyMatt commented 5 years ago

Description of the problem

In the unbound.sh file, line 21, the calculation of the slabs value is no power of 2 as commented and also required by the unbound server, but is a multiplication by 2.

How to reproduce

On my laptop with 4 threads, when starting the Docker, I've got the following errors then an "exit 1" :

$ docker run --rm --name my-unbound -p 53:53/udp mvance/unbound:1.8.3
/opt/unbound/etc/unbound/unbound.conf:206: error: must be a power of 2
/opt/unbound/etc/unbound/unbound.conf:211: error: must be a power of 2
/opt/unbound/etc/unbound/unbound.conf:221: error: must be a power of 2
/opt/unbound/etc/unbound/unbound.conf:248: error: must be a power of 2
read /opt/unbound/etc/unbound/unbound.conf failed: 4 errors in configuration file
[1544780130] unbound[1:0] fatal error: Could not read config file: /opt/unbound/etc/unbound/unbound.conf. Maybe try unbound -dd, it stays on the commandline to see more errors, or unbound-checkconf
$ echo $?
1

This is because i get threads = 3 and finally slabs = 6, which is no power of 2!

How to fix

2 ways:

as shell doesn't have the power math function, use the awk command, with the gawk package added (not installed by default):
```
slabs=`echo 2 $threads | awk '{print $1 ** $2}'`
```
move to a bash script (so edit the shebang) as Debian provides it natively, so you can do the following (note the two wildcards '*' and the number '2' before):
```
slabs=$(( 2 ** $threads ))
```

Discussion

A shell script is more portable across distros, but eh, this is one of the containers advantage: you can use the image you want depending on what you need!

It's up to you.

Concerned versions

The 1.8.2 and the 1.8.3.

MatthewVance commented 5 years ago

Thanks for reporting this. A multiple of two was my intent. I need to revise the comment wording to be more clear. The wording was based off of the how to optimize unbound page.

From the research I did, using documentation about each slab flag, calomel.org, etc., the general recommendation and practice seems to be a value close the number of CPUs. Using exponentiation would quickly exceed this as the number of cores increases.

While confusing, I think their usage of "power if 2 number of slabs" actually intends for these values to be divisible by two.

If you are aware of anything that clearly recommends otherwise, please let me know. I struggled with how to implement this one when reading their docs which led me to research how others implemented.

On Fri, Dec 14, 2018, 4:22 AM Matthieu <notifications@github.com wrote:

Description of the problem

In the unbound.sh file, line 21, the calculation of the slabs value is no power of 2 as commented and also required by the unbound server, but is a multiplication by 2. How to reproduce

On my laptop with 4 threads, when starting the Docker, I've got the following errors then an "exit 1" :

$ docker run --rm --name my-unbound -p 53:53/udp mvance/unbound:1.8.3 /opt/unbound/etc/unbound/unbound.conf:206: error: must be a power of 2 /opt/unbound/etc/unbound/unbound.conf:211: error: must be a power of 2 /opt/unbound/etc/unbound/unbound.conf:221: error: must be a power of 2 /opt/unbound/etc/unbound/unbound.conf:248: error: must be a power of 2read /opt/unbound/etc/unbound/unbound.conf failed: 4 errors in configuration file [1544780130] unbound[1:0] fatal error: Could not read config file: /opt/unbound/etc/unbound/unbound.conf. Maybe try unbound -dd, it stays on the commandline to see more errors, or unbound-checkconf $ echo $? 1

This is because i get threads = 3 and finally slabs = 6, which is no power of 2! How to fix

2 ways:

as shell doesn't have the power math function, use the awk command, with the gawk package added (not installed by default):

slabs=echo 2 $threads | awk '{print $1 ** $2}'

move to a bash script (so edit the shebang) as Debian provides it natively, so you can do the following (note the two wildcards '*' and the number '2' before):

slabs=$(( 2 ** $threads ))

Discussion

A shell script is more portable across distros, but eh, this is one of the containers advantage: you can use the image you want depending on what you need!

It's up to you. Concerned versions

The 1.8.2 https://github.com/MatthewVance/unbound-docker/blob/master/1.8.2/unbound.sh and the 1.8.3 https://github.com/MatthewVance/unbound-docker/blob/master/1.8.3/unbound.sh .

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/MatthewVance/unbound-docker/issues/8, or mute the thread https://github.com/notifications/unsubscribe-auth/ACjDujxOAwrbLk5EyqwG2ntkGxE-j8JXks5u43vigaJpZM4ZTQmU .

PtyMatt commented 5 years ago

In fact this will become quickly overestimated, an 8 cores machine gives slabs equal to 256. This is "a lot" ;)

An automagically optimized conf isn't an easy work! but this is a proper one. I will get it back then bind the generated conf with this var adapted.

MatthewVance commented 5 years ago

Thanks Matthieu. It certainly is a challenge and I'm open to suggestions and improvements. Thanks again for taking the time to discuss this.

On Mon, Dec 17, 2018, 5:05 PM Matthieu <notifications@github.com wrote:

In fact this will become quickly overestimated, an 8 cores machine gives slabs equal to 256. This is "a lot" ;)

An automagically optimized conf isn't an easy work! but this is a proper one. I will get it back then bind the generated conf with this var adapted.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/MatthewVance/unbound-docker/issues/8#issuecomment-448032804, or mute the thread https://github.com/notifications/unsubscribe-auth/ACjDuqSbK1tcNFl4lhVMq9xSiZrDkRZnks5u6CNHgaJpZM4ZTQmU .

gymnae commented 5 years ago

I wish I could help here, but I just notice that this issue keeps me from using your otherwise amazing image atm

MatthewVance commented 5 years ago

This might help: https://github.com/MatthewVance/stubby-docker/blob/master/README.md#use-a-customized-unbound-configuration

On Mon, Dec 17, 2018, 5:16 PM Gymnae <notifications@github.com wrote:

I wish I could help here, but I just notice that this issue keeps me from using your otherwise amazing image atm

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/MatthewVance/unbound-docker/issues/8#issuecomment-448035348, or mute the thread https://github.com/notifications/unsubscribe-auth/ACjDunCq9H3XFnaJ7wCLhZOSfri5YfIDks5u6CXIgaJpZM4ZTQmU .

MatthewVance commented 5 years ago

I see a couple of options:

Simply remove the *--slabs.
Add some if/else to set reasonable values. For example, if number of processors is <x, do y; else do z. Something similar to https://forum.netgate.com/topic/55701/unbound-update-reinstall-issue/7.

I'm leaning towards option 1 unless I hear a strong case for option 2. The first option keeps the code less complex

MatthewVance commented 5 years ago

How about something like the following for option two? It uses some odd functions and requires a move to a bash script, but avoids the need to install anything else.

The calculation is based on how pfSense handles this in its unbound.inc. OPNsense appears to do something similar as well.

#!/bin/bash
nproc=$(nproc)
export nproc
if [ "$nproc" -gt 1 ]; then
    threads=$((nproc - 1))
    # Calculate base 2 log of the number of processors
    nproc_log=$(perl -e 'printf "%5.5f\n", log($ENV{nproc})/log(2);')

    # Round the logarithm to an integer
    rounded_nproc_log="$(printf '%.*f\n' 0 "$nproc_log")"

    # Set *-slabs to a power of 2 close to the num-threads value.
    # This reduces lock contention.
    slabs=$(( 2 ** rounded_nproc_log ))
else
    threads=1
    slabs=4
fi

MatthewVance / unbound-docker