From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 20 Aug 2019 11:12:27 +0300 From: Vladimir Davydov Subject: Re: [tarantool-patches] Re: [PATCH v2 2/7] vinyl: don't exempt dropped indexes from dump and compaction Message-ID: <20190820081227.GQ13834@esperanza> References: <67030c30de327fbceaf2c204eec11a3745869533.1566233187.git.vdavydov.dev@gmail.com> <20190819204700.GC21602@atlas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190819204700.GC21602@atlas> To: Konstantin Osipov Cc: tarantool-patches@freelists.org List-ID: On Mon, Aug 19, 2019 at 11:47:00PM +0300, Konstantin Osipov wrote: > * Vladimir Davydov [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.