From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 5 Oct 2018 00:59:32 +0300 From: Konstantin Osipov Subject: Re: [PATCH 08/13] gc: keep track of available checkpoints Message-ID: <20181004215932.GH22855@chai> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: To: Vladimir Davydov Cc: tarantool-patches@freelists.org List-ID: * Vladimir Davydov [18/10/05 00:11]: > Currently, the checkpoint iterator is in fact a wrapper around > memtx_engine::snap_dir while the garbage collector knows nothing about > checkpoints. This feels like encapsulation violation. Let's keep track > of all available checkpoints right in the garbage collector instead > and export gc_ API to iterate over checkpoints. > +void > +gc_add_checkpoint(const struct vclock *vclock) > +{ > + struct gc_checkpoint *last_checkpoint = gc_last_checkpoint(); > + if (last_checkpoint != NULL && > + vclock_sum(&last_checkpoint->vclock) == vclock_sum(vclock)) { > + /* > + * No new checkpoint was actually created. > + * Rerun the garbage collector to delete old > + * files in case box.cfg.checkpoint_count > + * was changed. > + */ > + gc_run(); You only compare with the last checkpoint, not all existing checkpoints. When do you need this? When do you add an existing checkpoint and know it could be a duplicate of the last one? Is it initial recovery? Why do you need to deal with it this way? Please explain. Otherwise the patch is OK to push. -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov