seeker: harden against changes in the data during binary search

During the search, the region between `upper_bound` and `target_pos`
should contain no data items.  The search lowers `upper_bound` and raises
`lower_bound` until they both point to the last item before `target_pos`.

The `lower_bound` is increased to the position of the last item returned
by a search (`high_pos`) when that item is lower than `target_pos`.
This avoids some loop iterations compared to a strict binary search
algorithm, which would increase `lower_bound` only as far as `probe_pos`.

When the search runs over live extent items, occasionally a new extent
will appear between `upper_bound` and `target_pos`.  When this happens,
`lower_bound` is bumped up to the position of one of the new items, but
that position is in the "unoccupied" space between `upper_bound` and
`target_pos`, where no items are supposed to exist, so `seek_backward`
throws an exception.

To cut down on the noise, only increase `lower_bound` as far as
`upper_bound`.  This avoids the exception without increasing the number
of loop iterations for normal cases.

In the exceptional cases, extra loop iterations are needed to skip over
the new items.  This raises the worst-case number of loop iterations
by one.

Signed-off-by: Zygo Blaxell <bees@furryterror.org>

diff --git a/include/crucible/seeker.h b/include/crucible/seeker.h
index 69d0336776410a9045701a97fc987346400ff361..298b268d336637814ec47b79b48f4ad624ece5c5 100644 (file)
--- a/include/crucible/seeker.h
+++ b/include/crucible/seeker.h
@@ -66,7 +66,7 @@ namespace crucible {
                // We need one loop for each bit of the search space to find the lower bound,
                // one loop for each bit of the search space to find the upper bound,
                // and one extra loop to confirm the boundary is correct.
-               for (size_t loop_count = min(numeric_limits<Pos>::digits * size_t(2) + 1, max_loops); loop_count; --loop_count) {
+               for (size_t loop_count = min((1 + numeric_limits<Pos>::digits) * size_t(2), max_loops); loop_count; --loop_count) {
                        SEEKER_DEBUG_LOG("fetch(probe_pos = " << probe_pos << ", target_pos = " << target_pos << ")");
                        auto result = fetch(probe_pos, target_pos);
                        const Pos low_pos = result.empty() ? end_pos : *result.begin();
@@ -128,9 +128,10 @@ namespace crucible {
                        }
                        if (high_pos < target_pos) {
                                // results are all too low, so probe_pos..high_pos is too low
-                               // raise the low bound to high_pos since it's above probe_pos
-                               SEEKER_DEBUG_LOG("lower_bound = high_pos " << high_pos);
-                               lower_bound = high_pos;
+                               // raise the low bound to high_pos but not above upper_bound
+                               const auto next_pos = min(high_pos, upper_bound);
+                               SEEKER_DEBUG_LOG("lower_bound = next_pos " << next_pos);
+                               lower_bound = next_pos;
                        }
                        // compute a new probe pos at the middle of the range and try again
                        // we can't have a zero-size range here because we would not have set found_low yet