Re: [PATCH 08/13] gc: keep track of available checkpoints

[Konstantin Osipov <kostja@tarantool.org>]

* Vladimir Davydov <vdavydov.dev@gmail.com> [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