Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v23.2.x] storage: mutex compaction and prefix truncation #17254

Conversation

vbotbuildovich
Copy link
Collaborator

Backport of PR #17019

A crash was observed in CI caused by storage layer metadata
inconsistency and accompanied by data loss.

```
Assert failure: (src/v/raft/consensus.cc:2434) 'last_included_term.has_value()' Unable to get term for snapshot last included offset: 9499, log: {
    offsets: {start_offset:9457, committed_offset:11164, committed_offset_term:2, dirty_offset:11164, dirty_offset_term:2},
    is_closed: false, segments: [
        {size: 12, [
            {offset_tracker:{term:2, base_offset:9500, committed_offset:9669, dirty_offset:9669},
```

Notice that `start_offset` as reported by the storage layer is lower
than the `base_offset` of the first segment. `start_offset` must be
great-or-equal.

In the logs the following lines were observed:

```
Compacting 2 adjacent segments: [
   Segment 1: {offset_tracker:{term:1, base_offset:9190, committed_offset:9456, dirty_offset:9456} ...
   Segment 2: {offset_tracker:{term:1, base_offset:9457, committed_offset:9499, dirty_offset:9499} ...
```

Followed shortly after by:

```
log_eviction_stm.cc:164 - requested to write raft snapshot (prefix_truncate) at 9456

disk_log_impl.cc:917 - Final compacted segment {offset_tracker:{term:1, base_offset:9190, committed_offset:9499, dirty_offset:9499}
segment_utils.cc:483 - swapping compacted segment temp file /var/lib/redpanda/data/kafka/test-topic/0_28/9190-1-v1.log.compaction.staging with the segment /var/lib/redpanda/data/kafka/test-topic/0_28/9190-1-v1.log

disk_log_impl.cc:2403 - Removing "/var/lib/redpanda/data/kafka/test-topic/0_28/9190-1-v1.log" (remove_prefix_full_segments, {offset_tracker:{term:1, base_offset:9190, committed_offset:9456, dirty_offset:9456} ...
```

This points to a race condition between adjacent segment compaction and
prefix truncation. Segment 2 was "folded" into Segment 1. When prefix
truncation tried to remove the pre-compaction Segment 1 it ended up
removing data for Segment 2 as well. The result is data loss and
metadata inconsistency.

We fix this by introducing mutual exclusion of truncation and compaction
routine similar to what we do in suffix truncation.

---

I believe this can also be fixed by just taking a read/write lock on the
segment but the change is more intrusive as it requires additional
refactoring.

(cherry picked from commit d2ee527)
@vbotbuildovich vbotbuildovich added this to the v23.2.x-next milestone Mar 22, 2024
@vbotbuildovich vbotbuildovich added the kind/backport PRs targeting a stable branch label Mar 22, 2024
@piyushredpanda piyushredpanda merged commit 3126ca8 into redpanda-data:v23.2.x Mar 23, 2024
23 checks passed
@piyushredpanda piyushredpanda modified the milestones: v23.2.x-next, v23.2.28 Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/redpanda kind/backport PRs targeting a stable branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants