[TDB] Patches for file and memory usage growth issues

[Rusty Russell]

On Sat, 09 Apr 2011 23:01:15 -0400, simo <idra at samba.org> wrote:
> Hi all,
> during this week I have been working on a memory growth issue affecting
> sssd when storing large group of users in the LDB based cache.
> 
> It turns out that a lot of memory is being used by TDB.
> 
> Please take a look at the last 3 patches in this tree:
> http://git.samba.org/?p=idra/samba.git;a=shortlog;h=refs/heads/tdb-memsize
> 
> The first one should be uncontroversial, the original intent makes sense
> in a tdb database when you use small records, but when a rare huge
> record is saved in, growing the db by 100 times its size really is
> overkill. I also reduced the minimum overhead from 25% to 10% as with
> large TDBs (in the order of 500MB/1GB, growing by 25% is quite a lot).

Yep, that makes sense.  Perhaps just drop the heuristic to ignore record
size altogether?  Double until the database is > 1M, then add 10%?

Note that we repack on expand, so limiting expansions will end up
slowing down some bechmarks.  I don't think it's a problem IRL, though.

> The second one needs discussion. I noticed that a transaction can
> substantially increase the size of a tdb, in my tests I had a 1.2G size
> TDB actually grow to a ~1.7G after a backup ... by not using a
> transaction to store the backup copy the same 1.2G TDB shrunk down to
> ~900MB. I know that the reason we use a transaction is to avoid rsyncs
> of partial DBs, so maybe we want to add an option on whether to use a
> transaction for the 2nd db ? Comments welcome.

Hmm, that's odd.  Compressing the recovery area makes a great deal of
sense for tdb2 though.

Note that we overallocate all TDB entries by 25%.  (TDB2 only does that
when you expand an existing record).

> The third one I think is less controversial but still not something I
> would put in without discussion. This feature has the very nice property
> of being completely optional and needs an explicit flags to be passed at
> tdb_open(). This makes it relatively harmless and not very intrusive.

But it makes an incompatible database :(

OTOH it's fairly easy for the user to do this themselves and they know
what tradeoffs they want to make.

> Finally I also am going to try to come to a solution for the huge memory
> usage caused by tdb_repack(). This function actually needs a lot of
> improvement. Firstly because it uses a transaction to copy the database
> twice do the repack.
> This is inefficient in 3 ways.
> 1. Temporarily doubles the memory used (due to the second tdb in
> memory), which is a huge problem when your TDB file is *big* (ldb use
> cases with many users/groups).
> 2. It actually does 2 copies of the TDB back and forth after wiping all
> the records of the original tdb, all whithin a transaction (lots of
> memory used).
> 3. Due to the transaction it can sometimes increase the actual size of
> the tdb file instead of shrinking it. I have some ideas of why it is
> happening but haven't analyzed it fully yet (seem like the same issue
> affecting tdbbackup except we cannot avoid using a transaction here as
> quick dirty fix like I did in patch 2).
>
>
> So if anyone have ideas on how to deal with this they are really welcome
> as the repack is killing our machines (OOM Killer getting in action in
> some cases, plus slowness), although disabling tdb_repack is also not an
> option as the tdb becomes quickly fragmented with many big transactions
> containing huge records and grows even more.

I do... let me hack up a patch for you to look at...

Thanks!
Rusty.