When we compact the reftable stack we first acquire the lock for the
"tables.list" file and then reload the stack to check that it is still
up-to-date. This is done by calling `stack_uptodate()`, which knows to
return zero in case the stack is up-to-date, a positive value if it is
not and a negative error code on unexpected conditions.
We don't do proper error checking though, but instead we only check
whether the returned error code is non-zero. If so, we simply bubble it
up the calling stack, which means that callers may see an unexpected
positive value.
Fix this issue by translating to `REFTABLE_OUTDATED_ERROR` instead.
Handle this situation in `reftable_addition_commit()`, where we perform
a best-effort auto-compaction.
All other callsites of `stack_uptodate()` know to handle a positive
return value and thus don't need to be fixed.
Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
---
reftable/stack.c | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/reftable/stack.c b/reftable/stack.c
index 1ce4d90cb8..af0f94d882 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -579,9 +579,11 @@ int reftable_new_stack(struct reftable_stack **dest, const char *dir,
return err;
}
-/* -1 = error
- 0 = up to date
- 1 = changed. */
+/*
+ * Check whether the given stack is up-to-date with what we have in memory.
+ * Returns 0 if so, 1 if the stack is out-of-date or a negative error code
+ * otherwise.
+ */
static int stack_uptodate(struct reftable_stack *st)
{
char **names = NULL;
@@ -850,10 +852,13 @@ int reftable_addition_commit(struct reftable_addition *add)
* control. It is possible that a concurrent writer is already
* trying to compact parts of the stack, which would lead to a
* `REFTABLE_LOCK_ERROR` because parts of the stack are locked
- * already. This is a benign error though, so we ignore it.
+ * already. Similarly, the stack may have been rewritten by a
+ * concurrent writer, which causes `REFTABLE_OUTDATED_ERROR`.
+ * Both of these errors are benign, so we simply ignore them.
*/
err = reftable_stack_auto_compact(add->stack);
- if (err < 0 && err != REFTABLE_LOCK_ERROR)
+ if (err < 0 && err != REFTABLE_LOCK_ERROR &&
+ err != REFTABLE_OUTDATED_ERROR)
goto done;
err = 0;
}
@@ -1215,9 +1220,24 @@ static int stack_compact_range(struct reftable_stack *st,
goto done;
}
+ /*
+ * Check whether the stack is up-to-date. We unfortunately cannot
+ * handle the situation gracefully in case it's _not_ up-to-date
+ * because the range of tables that the user has requested us to
+ * compact may have been changed. So instead we abort.
+ *
+ * We could in theory improve the situation by having the caller not
+ * pass in a range, but instead the list of tables to compact. If so,
+ * we could check that relevant tables still exist. But for now it's
+ * good enough to just abort.
+ */
err = stack_uptodate(st);
- if (err)
+ if (err < 0)
goto done;
+ if (err > 0) {
+ err = REFTABLE_OUTDATED_ERROR;
+ goto done;
+ }
/*
* Lock all tables in the user-provided range. This is the slice of our
--
2.51.0.rc1.215.g0f929dcec7.dirty
