[RFC] [CTDB] Optimized memory handling.

[Swen Schillig]

On Thu, 2017-12-14 at 08:47 +0100, Swen Schillig via samba-technical
wrote:
> On Thu, 2017-12-14 at 10:42 +1100, Amitay Isaacs via samba-technical
> wrote:
> > On Wed, Dec 13, 2017 at 10:52 AM, Jeremy Allison via samba-
> > technical
> > <samba-technical at lists.samba.org> wrote:
> > > On Tue, Dec 12, 2017 at 12:49:08PM +0100, Swen Schillig via
> > > samba-
> > > technical wrote:
> > > > ...
> > > > The currently used magic numbers for the pool sizes are still
> > > > under
> > > > investigation...hints and recommendations are welcome.
> > > > 
> > > > However, initial tests showed big improvements.
> > > 
> > > Can you give more details on what you mean by "big improvements"
> > > ?
> > > Some numbers would be good :-).
> > > 
> > > Thanks,
> > > 
> > > Jeremy.
> > > 
> > 
> > Even without seeing the proof I can imagine the improvements in
> > packet
> > handling using the talloc pool.  That code is in the hot path and
> > avoiding extra malloc() calls will definitely result in reducing
> > the
> > load on ctdb daemon.  However, as Jeremy mentioned, it would be
> > good
> > to verify with real numbers.
> > ...
> > 
> > Amitay.
> > 
> 
> Thank you guys for having a look, it's really appreciated.
> 
> As for the numbers, I do not have any "real" numbers yet.
> But an isolated test of the code in question did show improvements of
> approx. 40% of pool-alloc-code vs. non-pool-alloc-code.
> This means just for the areas in question and not running a 
> real-life load.
> Anyhow, as Amitay stated, this is the hot path and if we're "only"
> getting 5% in the end that would be "a big improvement" :-)
> 
> I'm in the process of getting a cluster ready to perform 
> real-life-tests to get an idea of some good initial pool-size values,
> (especially for the data pool).
> 
> @Amitay: I will try and transfer the changes to sock_io today so you
> guys can have a look again.... making sure the direction is right.
> 
> Regarding the "static" data pool. 
> I'm not a fan of those either, but I couldn't find a decent context
> to
> attach the pool to. Therefore, I decided to setup "one" pool once
> for all data-buffer requirements instead of being dependent on some
> upper structure which is not related in any way to the data mem.
> 
> Cheers Swen
Now that's embarrassing.

The changes I proposed are almost all there in 
Amitay's code (socket_io.c) already.
I should have read more before posting, sorry.

>From the patches/code I posted so far, I guess I have to re-order the
changes for them to make sense again.
First memory-opt (now in socket_io.c) and then ctdb_io.c adoptions.

A few comments on the code.
1) why do we have to free the memory for queue->buf everytime ?
   I don't think this is necessary (talloc_realloc if needed) and
   would save time again. Once done with the queue it'll be
   free'd anyway.
2) I still believe we don't need a destructor.
   There's nothing done which is either required (queue-> = -1) or
   not done automatically (TALLOC_FREE(queue->fde)).
3) @Amitay: I really like the removal (in comparison to ctdb_io) of 
   the really expensive "data" memory / memcpy thing, that on it's own 
   should save time already. I just have to make sure to change all
   the callbacks for ctdb_io to NOT free that memory anymore.

Attached you can find two patches, the memory-opt patch is pretty small
and straight forward. I could prepare a committable patch by tomorrow,
if it's wanted. Please let me know if you'd be ok with the 2 addtl.
changes I suggested above( no destructor / no mem-free).

Regarding the other patch (ctdb_sock_io replacement) this is WIP and
won't come out of this state until next year.
It does compile but has most likely a good few bugs in it.

Please comment.

Cheers Swen
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: memory_opt.patch
Type: text/x-patch
Size: 1742 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20171214/23a99228/memory_opt.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ctdb_sock_io.patch
Type: text/x-patch
Size: 21760 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20171214/23a99228/ctdb_sock_io.bin>