Re: [PATCH v3 2/5] reftable/stack: add function to check if optimization is required

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> The reftable backend performs auto-compaction as part of its regular
> flow, which is required to keep the number of tables part of a stack at
> bay. This allows it to stay optimized.
>
> Compaction can also be triggered voluntarily by the user via the 'git
> pack-refs' or the 'git refs optimize' command. However, currently there
> is no way for the user to check if optimization is required without
> actually performing it.
>
> Extract out the heuristics logic from 'reftable_stack_auto_compact()'
> into an internal function 'update_segment_if_compaction_required()'.
> Then use this to add and expose `reftable_stack_compaction_required()`
> which will allow users to check if the reftable backend can be
> optimized.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  reftable/reftable-stack.h       | 11 +++++++++++
>  reftable/stack.c                | 42 ++++++++++++++++++++++++++++++++++++-----
>  t/unit-tests/u-reftable-stack.c | 12 ++++++++++--
>  3 files changed, 58 insertions(+), 7 deletions(-)

The required change is surprisingly small, which is a good sign.

> diff --git a/reftable/stack.c b/reftable/stack.c
> index 49387f9344..826500abed 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -1647,19 +1647,51 @@ static int stack_segments_for_compaction(struct reftable_stack *st,
>  	return 0;
>  }
>  
> -int reftable_stack_auto_compact(struct reftable_stack *st)
> +static int update_segment_if_compaction_required(struct reftable_stack *st,
> +						 struct segment *seg,
> +						 bool use_heuristics,
> +						 bool *required)
>  {

Am I correct to understand that "use_heuristics" is almost a synonym
to "maintain geometric progression" in the context of this patch?
Are we expecting other heuristics in the future, in which case, this
may not be a single "bool" but a set of flag bits, and until then
s/heuristics/geometric/ might make it a better name for the
parameter?

Thanks.