[tarantool-patches] Re: [PATCH v2 2/7] vinyl: don't exempt dropped indexes from dump and compaction

[Vladimir Davydov]

On Mon, Aug 19, 2019 at 11:47:00PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev at gmail.com> [19/08/19 19:57]:
> >  	vy_lsm_read_set_t read_set;
> > +	/**
> > +	 * Triggers run when the last reference to this LSM tree
> > +	 * is dropped and the LSM tree is about to be destroyed.
> > +	 * A pointer to this LSM tree is passed to the trigger
> > +	 * callback in the 'event' argument.
> > +	 */
> > +	struct rlist on_destroy;
> 
> Please explain in the comment that the compaction scheduler task takes a
> reference as well, so if the dropped index happens to be compacted
> at the moment, it will be dropped only when the compaction task
> finishes.

True. We could avoid that, but for now it isn't worth bothering about
IMO. I'll add a comment.

> 
> Why did you add a trigger - to avoid dependency
> loops between vy_lsm and vy_scheduler? But sounds like it's
> simpler to make vy_lsm aware in vy_scheduler and register/unregister
> itself in create/destroy. Why did you choose to add a trigger
> instead?

To avoid a dependency loop: vy_scheduler.[hc] depends on vy_lsm.[hc],
but not vice versa, which is nice IMO.

Besides, I think we could reuse the trigger for other purposes, e.g. to
force data file deletion when an index is finally released.