[RFC] [CTDB] Optimized memory handling.
Swen Schillig
swen at vnet.ibm.com
Thu Dec 14 13:46:00 UTC 2017
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>
More information about the samba-technical
mailing list