Skip to content

ec: reads of erasure-coded data are not balanced by device latency #1071

Open

Description

@aderk

For replicated extents, bch2_bkey_pick_read_device() chooses a pointer
with ptr_better(): at random, weighted by the square of each device's
read latency (cur_latency[READ], an EWMA). A device slower than the
others gets fewer reads, but is still sampled.

An erasure-coded extent has one pointer, into a data bucket of its stripe
(k data and p parity buckets on k + p devices), and every read goes to
it. The other buckets are read only when do_ec_reconstruct is set: after
a read or checksum error on that pointer, or while its device is
offline. bch2_ec_read_extent() then reads the extent's range from all
k + p buckets and reconstructs it. Neither path looks at latency, and
neither treats evacuating devices specially: dev_failed() (offline or
evacuating) has no callers.

So an EC read waits for its device whenever that device is slower than
the rest of the stripe: in a pool of mixed devices, under uneven load,
during a scrub, reconcile or SMART self-test on it, or while it retries a
bad sector (bounded by the drive's error recovery limit, commonly 7 s).
Internal reads do the same: ec_old_stripe_read() reads every bucket of a
stripe being repaired or reused, including one on an evacuating device.
On a pool here, evacuating a failing drive was limited by that drive's
sector retries, although each stripe's other buckets held the data.

Reproducer: koverstreet/ktest#130. Five devices, 3+1
stripes, a 96M file; device 0 holds a bucket of 35-38 stripes. A 500 ms
dm-delay stands in for a sector retry. On testing 4a415b7 (v1.39.6
within 1 MiB):

O_DIRECT read, device 0 evacuating     20 MiB from it (others 18-20)
evacuating it (stripe repair)          27 MiB from it (its buckets: 18.5 MiB)
device 0 behind a 500 ms dm-delay      36 of 96 1M reads wait ~500 ms

Options, by increasing complexity (none prototyped):

  1. Use dev_failed() in bch2_bkey_pick_read_device(): an EC pointer on an
    evacuating device sets do_ec_reconstruct, and ptr_better() prefers a
    replica that is not on one.
  2. Stripe repair and reuse: when at most p buckets are on evacuating
    devices, reconstruct them from the others instead of reading them, and
    read them only if that fails. Related:
    ec: stripe reuse reads the whole old stripe, keeps only carried blocks #1070.
  3. bch2_ec_read_extent(): read k buckets instead of k + p (skip the
    target and, when p > 1, the slowest others), and read the rest only on
    a checksum failure. This lowers the cost of every reconstruct,
    including those from options 1, 4 and 5.
  4. Make reconstruct a candidate in bch2_bkey_pick_read_device(), weighted
    like a replica: its latency is the highest of the k devices it would
    read, scaled for the extra IO. This covers every EC read, including a
    device that is only busier than the others.
  5. Hedged reads: if a direct read misses a per-device deadline (such as
    the read threshold bch2_congested_acct() computes, 4x a low latency
    quantile), start a reconstruct and complete from whichever finishes
    first. The direct read needs a bounce buffer, since its bio can't be
    cancelled. This covers stalls the EWMA hasn't caught up with, such as
    a single sector retry.

Activity

  1. koverstreet commented on Sep 30, 2026

    @koverstreet
    Owner

    I'm uncomfortable with the idea of ec read around because reconstruct reads are inherently much more expensive, and if we start doing them under load... you see the issue?

    If we apply this I'll want to be really conservative with it

  2. aderk commented on Sep 30, 2026

    @aderk
    Author

    @koverstreet I agree with your concern. Let me make an argument that this is necessary and not some micro-optimization in rare cases. (I will admit reconstruct reads are basically irrelevant with a healthy, homogenous pool -- but let's have bcachefs perform well when those aren't true.)
    Relevant use cases:

    1. Bad sectors on a single failing drive will stall user IO for the pool and evacuate due to waiting on a single drive's retries. The way around this is setting device offline, but you're removing data from the recovery equation at this point. In my opinion, even with sector retry set to 0.1sec a failing hdd drive head (ie ~100s of GB of bad sectors) requires device offline for evacuate or pool IO, which seems unacceptable to me (especially when ~95% of the data is still perfectly, quickly readable).
    2. For disparate drive sizes under load, the larger ones will get relatively overloaded (for placement balancing) and bcachefs leaves performance on the table.
    3. For a pool with N homogenous hdds and bg maintenance (scrub, internal firmware ops) running on 1 hdd, 1/N reads is going to have unnecessarily high latency. p90 latency will always be the highest latency drive (or for N>10 say p99, etc -- you get the point: p[1-1/N] latency).

    My mental model is a pool of hard drives has latency of ~50ms across the pool, but one of them is at 200ms (or for bad sectors ~7000ms). And I think bcachefs should always perform well when possible (e.g. during single device scrub) and not frustrate users (e.g. evacuate always works quickly not "why is it stuck on this one thing that isn't necessary!").

    I'd propose comparing the direct_read_dev_latency with max(reconstruct_dev_latency)*PENALTY_FACTOR -- where penalty factor is 1.1, 1.5, or 2 (Hell make it 10 if you want) -- and then pick the lower one using the squared weighting system. With this selection, reconstruct reads would never occur on a healthy, balanced-utilization pool. For P+Q, there's some combinatorics to deal with that seem manageable IMO.

    doing them under load... you see the issue?

    Yes it is a tradeoff. Doing several (bucket) reads vs 1 extent read is certainly not efficient.

    However, if I may nitpick this model: load is not uniform across drives and encapsulating the complexity in terms of a scalar pool utilization only holds in a balanced, homogenous environment. If a pool can deliver a read in 50ms (albeit with extra reads), why add to the overloaded drive's queue and wait 200ms. This per-drive load model is already implied by the effort to balance regular replica reads (rather than choosing at random).

    (EDIT: The design ended up being quite different than what this paragraph proposes, see #1098.)
    I think the elegant way to do it is to only read the ranges necessary in the data buckets (each data read would be a max of the reconstruct size plus a max of two times the max extent size due to boundary extents needing to be checksummed) and then checksum the whole parity bucket. However in a model of seek time dominates, I think it would be fine to read all the (D+P) buckets whole, checksum, and then discard the majority and reconstruct what is needed.

  3. koverstreet commented on Sep 30, 2026

    @koverstreet
    Owner

    Yeah, partial failure is a good point. And we do have the latency signals - ok, go ahead, I'll take it :)

  4. koverstreet commented on Sep 30, 2026

    @koverstreet
    Owner

    one other thought: if we're trying ec reconstruct first, if that fails we'll still want to be able to try a normal read, that'll require tweaking how we track retries

  5. aderk commented on Oct 5, 2026

    @aderk
    Author

    (edit: remove spurious GH rendering)

    @koverstreet, thanks for the heads up on retries. Here's a heads up on how this is all landing, in dependency order:

    Bugs found along the way, independent of each other and of read-around:

    • #1090: a reconstruct that races with its stripe going away fails the read. Fix #1095, test ktest#133.
    • #1091: reconcile pushes fs default options onto reflinked data, one piece of #927. Fix #1096, test ktest#134.
    • #1092: a resumed stripe update moves extents into the wrong block. Fix #1097, test ktest#135.

    Groundwork read-around builds on:

    • #1088: RAID6 recovery of two lost blocks is wrong past the first page, which P+Q read-arounds hit. Fix #1093, test ktest#131. #1098 carries this commit until it merges.
    • #1089: bch2_latency_acct() drops multi-second samples. Fix #1094, test ktest#132. Read-around works without it, but a failing device's estimate climbs about half as fast.

    Read-around:

    • #1098: the implementation, test ktest#136 (replacing ktest#130). Design and numbers in the PR description.
    • Next: a read-around that reads a bad block queues a scrub of its stripe. PR to follow, on top of #1098.

    If this is too much at a time, or you prefer diffs to PRs etc, I welcome your feedback. Thanks!

  6. aderk commented on Oct 5, 2026

    @aderk
    Author

    Follow-up is up: #1100 queues a scrub of the stripe when a read finds a bad block in it, tests in ktest#137. #1098 also gained a commit: read-arounds now promote as direct reads do, tested in ktest#136.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions