Re: [PATCH v2 2/8] vinyl: update compact priority usual way on range split/coalesce

[Konstantin Osipov <kostja@tarantool.org>]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/09/17 15:05]:
> When a few ranges are coalesced, we "force" compaction of the resulting
> range by raising its compaction priority to max (slice count). There's
> actually no point in that, because as long as the shape of the resulting
> LSM tree is OK, we don't need to do extra compaction work. Moreover, it
> actually doesn't work if a new slice is added to the resulting range by
> dump before it gets compacted, which is fairly likely, because then its
> compaction priority will be recalculated as usual. So let's simply call
> vy_range_update_compact_priority() for the resulting range.
> 
> -	/*
> -	 * Coalescing increases read amplification and breaks the log
> -	 * structured layout of the run list, so, although we could
> -	 * leave the resulting range as it is, we'd better compact it
> -	 * as soon as we can.
> -	 */
> -	result->compact_priority = result->slice_count;
> +	vy_range_update_compact_priority(result, &lsm->opts);

Looks like a drastic change of opinion to me :). I would leave
reasons you give in the old comment why we may need a compaction
and the new reasons you give in the CS comment why we may not need
a compaction in a consolidated comment for this line.
The patch itself is OK to push.

>  	vy_lsm_acct_range(lsm, result);
>  	vy_lsm_add_range(lsm, result);
>  	lsm->range_tree_version++;
> -- 
> 2.11.0
> 

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov