| Message ID | ae2130ffda2135bdea11ea6764c08e98a2cc4905.1710706118.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | refs: introduce `--auto` to pack refs as needed | expand |
Patrick Steinhardt <ps@pks.im> writes: > diff --git a/reftable/stack.c b/reftable/stack.c > index eaa8bb9c99..ba15c48ddd 100644 > --- a/reftable/stack.c > +++ b/reftable/stack.c > @@ -1003,7 +1003,7 @@ static int stack_compact_range(struct reftable_stack *st, > LOCK_NO_DEREF); > if (err < 0) { > if (errno == EEXIST) > - err = 1; > + err = REFTABLE_LOCK_ERROR; > else > err = REFTABLE_IO_ERROR; > goto done; > The comment at the top of the function now needs to be re-written with this change.
On Wed, Mar 20, 2024 at 03:14:10PM -0700, Karthik Nayak wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > diff --git a/reftable/stack.c b/reftable/stack.c > > index eaa8bb9c99..ba15c48ddd 100644 > > --- a/reftable/stack.c > > +++ b/reftable/stack.c > > @@ -1003,7 +1003,7 @@ static int stack_compact_range(struct reftable_stack *st, > > LOCK_NO_DEREF); > > if (err < 0) { > > if (errno == EEXIST) > > - err = 1; > > + err = REFTABLE_LOCK_ERROR; > > else > > err = REFTABLE_IO_ERROR; > > goto done; > > > > The comment at the top of the function now needs to be re-written with > this change. Good catch, will fix. Patrick
diff --git a/reftable/stack.c b/reftable/stack.c index eaa8bb9c99..ba15c48ddd 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -1003,7 +1003,7 @@ static int stack_compact_range(struct reftable_stack *st, LOCK_NO_DEREF); if (err < 0) { if (errno == EEXIST) - err = 1; + err = REFTABLE_LOCK_ERROR; else err = REFTABLE_IO_ERROR; goto done; @@ -1025,7 +1025,7 @@ static int stack_compact_range(struct reftable_stack *st, table_name.buf, LOCK_NO_DEREF); if (err < 0) { if (errno == EEXIST) - err = 1; + err = REFTABLE_LOCK_ERROR; else err = REFTABLE_IO_ERROR; goto done; @@ -1075,7 +1075,7 @@ static int stack_compact_range(struct reftable_stack *st, LOCK_NO_DEREF); if (err < 0) { if (errno == EEXIST) - err = 1; + err = REFTABLE_LOCK_ERROR; else err = REFTABLE_IO_ERROR; goto done; @@ -1187,7 +1187,7 @@ static int stack_compact_range_stats(struct reftable_stack *st, struct reftable_log_expiry_config *config) { int err = stack_compact_range(st, first, last, config); - if (err > 0) + if (err == REFTABLE_LOCK_ERROR) st->stats.failures++; return err; }
Compaction of a reftable stack may fail gracefully when there is a concurrent process that writes to the reftable stack and which has thus locked either the "tables.list" file or one of the tables. This is expected and can be handled gracefully by some of the callers which invoke compaction. Thus, to indicate this situation to our callers, we return a positive return code from `stack_compact_range()` and bubble it up to the caller. This kind of error handling is somewhat awkward though as many callers in the call chain never even think of handling positive return values. Thus, the result is either that such errors are swallowed by accident, or that we abort operations with an unhelpful error message. Make the code more robust by always using negative error codes when compaction fails, with `REFTABLE_LOCK_ERROR` for the described benign error case. Note that only a single callsite knew to handle positive error codes gracefully in the first place. Subsequent commits will touch up some of the other sites to handle those errors better. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- reftable/stack.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)