From: Karthik Nayak <karthik.188@gmail.com>
Date: Mon, 03 Nov 2025 16:35:31 GMT
Subject: Re: [PATCH 2/5] reftable/stack: add function to check if optimization is required
Message-ID: <CAOLa=ZQf_YC4-z8eOa=VaMdHFynfK-aWBC80sV5N4T5gzweq=Q@mail.gmail.com>
In-Reply-To: <aQi1c6ZLM-1dqrCI@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> On Fri, Oct 31, 2025 at 03:22:22PM +0100, Karthik Nayak wrote:
>> The reftable backend, performs auto-compaction as part of its regular
>
> s/,//
>
>> diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h
>> index d70fcb705d..a875149439 100644
>> --- a/reftable/reftable-stack.h
>> +++ b/reftable/reftable-stack.h
>> @@ -123,6 +123,11 @@ struct reftable_log_expiry_config {
>>  int reftable_stack_compact_all(struct reftable_stack *st,
>>  			       struct reftable_log_expiry_config *config);
>>
>> +/* Check if compaction is required. */
>> +int reftable_stack_compaction_required(struct reftable_stack *st,
>> +				       bool use_heuristics,
>> +				       bool *required);
>> +
>
> I think the documentation here could be improved a bit. Somebody not
> deeply familiar with reftables wouldn't know what `use_heuristics`
> really is supposed to mean.
>

I agree. I'll add in something.

>> diff --git a/reftable/stack.c b/reftable/stack.c
>> index 49387f9344..18fa41cd5c 100644
>> --- a/reftable/stack.c
>> +++ b/reftable/stack.c
>> @@ -1647,6 +1647,31 @@ static int stack_segments_for_compaction(struct reftable_stack *st,
>>  	return 0;
>>  }
>>
>> +int reftable_stack_compaction_required(struct reftable_stack *st,
>> +				       bool use_heuristics,
>> +				       bool *required)
>> +{
>> +	struct segment seg;
>> +	int err = 0;
>> +
>> +	if (st->merged->tables_len < 2) {
>> +		*required = false;
>> +		return 0;
>> +	}
>> +
>> +	if (!use_heuristics) {
>> +		*required = true;
>> +		return 0;
>> +	}
>> +
>> +	err = stack_segments_for_compaction(st, &seg);
>> +	if (err)
>> +		return err;
>> +
>> +	*required = segment_size(&seg) > 0;
>> +	return 0;
>> +}
>> +
>
> All of these conditions make sense.
>
> Patrick

Thanks!