[Unbound-users] patch implementing round robin rrsets

[Thijs Kinkhorst]

Hi Wouter,

On Wed, 07 Mar 2012 14:04:29 +0100, "W.C.A. Wijngaards"
<wouter at nlnetlabs.nl> wrote:
> Nice patch. :-)
> 
> I would like to accept it, because it is short and helpful and
> lightweight. There are the following suggestions below.

Thanks!
 
> The line of interest is this one:
> -		rr = random();
> 
> Now it seems that random() locks a mutex on Linux and may produce
> improper results on FreeBSD in threads (but still reasonably random).
>  So, I want to lock it less.  Also suggest to move the line one
> further (behind the variable declaration).
> +		rr = (data->count==1)?0:random();

This makes sense, as probably a large number of rrsets will have a count
of one.
 
> And then hope that on Linux, performance does not degrade
> multithreaded (it'll be copying the RR data while other threads lock
> this lock).
> 
> In general a threadsafe solution with threadlocal storage requires a
> lot more work.  An alternative is to be blatantly thread-unsafe (its
> non-security related pretty-random numbers anyway), and make a
> 	static int state = 1;
> and use a simple congruent random function, the rand function:
> 	state = state * 1103515245 + 12345;
> 	rr = (unsigned)(state/65536) % 32768;
> But then I worry about cache-line contention on the int state.

Yes. Of course the number doesn't need to be really random indeed. I can't
judge on the significancy of the performance impacts of the various
approaches: we don't have an extremely large setup, but we do serve
thousands of clients at any time, and we've not seen any trouble.

If it were up to me I'd say both approaches (added count check vs. int
state) would be perfectly fine for us, but I wouldn't go any further than
that because I doubt the benefits would outweigh the added complexity in
that case. So if you want to commit either version I'd already be very
happy.

> Depending on how this works out, this could end up in the mainline
> source, and the code needs to have the BSD license for that.  Or we
> can put the patch in the contrib directory of the source tarball
> (license of your choice).

The BSD license is just fine. The formal copyright holder of the work is
"Tilburg University", if that would be relevant.


-- 
Thijs Kinkhorst <thijs at uvt.nl> – LIS Unix

Universiteit van Tilburg – Library and IT Services
Bezoekadres > Warandelaan 2 • Tel. 013 466 3035 • G 236