Re: [tarantool-patches] Re: [PATCH 09/13] memtx: enter small delayed free mode from snapshot iterator

[Vladimir Davydov <vdavydov.dev@gmail.com>]

On Tue, Aug 13, 2019 at 01:27:49AM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [19/08/10 23:22]:
> > We must enable SMALL_DELAYED_FREE_MODE to safely use a memtx snapshot
> > iterator. Currently, we do that in checkpoint related callbacks, but if
> > we want to reuse snapshot iterators for other purposes, e.g. feeding
> > a read view to a newly joined replica, we better hide this code behind
> > snapshot iterator constructors.
> 
> this is not enough, you may have multiple replicas joining, and
> you need multiple checkpoint support  for that. 
> 
> Currently delayed free mode only supports one active checkpoint.

Yeah, sure. If replicas keep joining, deleted tuples won't be freed.
That's a pitfall. The right way to fix it is to patch the small
allocator to support generations, but it's going to take a while.
Since the number of replicas is limited and joining a new replica is
a rare event, I'd prefer to commit this for now and then look into
implementation of generational garbage collection in the small
allocator.

> 
> > +void
> > +memtx_enter_delayed_free_mode(struct memtx_engine *memtx)
> > +{
> > +	memtx->snapshot_version++;
> > +	if (memtx->delayed_free_mode++ == 0)
> > +		small_alloc_setopt(&memtx->alloc, SMALL_DELAYED_FREE_MODE, true);
> > +}
> 
> Just adding a counter will easily never free any memory if it
> never drops below 1.

Well, it will free memory allocated after the counter was incremented
(that's what snapshot_version is about), but in general you're right -
we need to deal with it somehow.