From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 10 Oct 2025 11:22:41 GMT
Subject: Re: [PATCH 7/9] reftable/stack: add function to check if optimization is required
Message-ID: <aOjsgQge8Wqh4odB@pks.im>
In-Reply-To: <20251010-562-add-option-to-check-if-reference-backend-needs-repacking-v1-7-c7962be584fa@gmail.com>

On Fri, Oct 10, 2025 at 12:27:11PM +0200, Karthik Nayak wrote:
> The reftable backend for references, performs auto-compaction as part of

s/for references, //

> its regular flow. This allows it to stay optimized.

Maybe: "flow, which is required to keep the number of tables part of a
stack at bay."

> diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h
> index d70fcb705d..754e955206 100644
> --- a/reftable/reftable-stack.h
> +++ b/reftable/reftable-stack.h
> @@ -123,6 +123,9 @@ struct reftable_log_expiry_config {
>  int reftable_stack_compact_all(struct reftable_stack *st,
>  			       struct reftable_log_expiry_config *config);
>  
> +/* check if heuristic based compaction is required  */
> +int reftable_stack_compaction_required(struct reftable_stack *st, bool *required);

Let's use a full sentence here, starting with an upper-case letter and
ending with punctuation.

> diff --git a/reftable/stack.c b/reftable/stack.c
> index 9d9326ce0e..732141d46c 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -1652,6 +1652,24 @@ static int stack_segments_for_compaction(struct reftable_stack *st,
>  	return 0;
>  }
>  
> +int reftable_stack_compaction_required(struct reftable_stack *st, bool *required)
> +{
> +	struct segment seg;
> +	int err = 0;
> +
> +	if (st->merged->tables_len < 2) {
> +		*required = false;
> +		return 0;
> +	}
> +
> +	err = stack_segments_for_compaction(st, &seg);
> +	if (err)
> +		return err;
> +
> +	*required = segment_size(&seg) > 0;
> +	return 0;
> +}

Okay, makes sense. If we have at least one segment that we'd perform
compaction for we return true, otherwise false.

Patrick