[PATCHES] Cleanup dead records in messages.tdb

[Jeremy Allison]

On Wed, Apr 02, 2014 at 05:21:51PM -0700, Christof Schmitt wrote:
> On Wed, Apr 02, 2014 at 04:57:03PM -0700, Jeremy Allison wrote:
> > On Wed, Apr 02, 2014 at 10:51:47AM +0200, Volker Lendecke wrote:
> > > On Tue, Apr 01, 2014 at 01:49:13PM -0700, Christof Schmitt wrote:
> > > > When a smbd process dies, pending messages.tdb records for this process
> > > > will not get cleaned up. I have seen one case where the messages.tdb
> > > > file grew very large after I/O requests were delayed and smbd child
> > > > processes died.
> > > > 
> > > > These patches implement cleanup of messages.tdb records that is
> > > > triggered after a smbd dies; any record with an invalid key or a
> > > > non-existing PID will get deleted.
> > > 
> > > Can we avoid the traverse in the normal case? Is it really
> > > that the smbds in your case left corrupt keys behind? In
> > > remove_child_pid we know which child died, so we could
> > > directly check and delete its record. Traverses always scare
> > > me :-)
> > 
> > OK, here is a version that doesn't traverse, but
> > only cleans up the records for the affected
> > pid.
> > 
> > Christof, it's based on your idea and Volker's
> > comments. Can you review and let me know what
> > you think ?
> 
> I assume that the !procid_is_local check is just a defensive
> programming, the function should never get called for a remote PID. A

Indeed. Defense is good :-).

> minor question would be why you have messaging_tdb_cleanup return a
> status, only to ignore it in the caller.

We ignore it *now*, as there's not much
we can really do with it (maybe DEBUG
message ?). But it is good pactice to
return NTSTATUS error codes whenever
possible, so I always try and do so.

> Overall it looks good:
> Reviewed-by: Christof Schmitt <cs at samba.org>

Thanks ! I'll push tomorrow ! Do you want to
log a bug so we can get it back-ported to
4.0.next and 4.1.next ?

Cheers,

Jeremy.