Skip to content

VSR: Reverse ring replication for odd ops - #2761

Merged
matklad merged 3 commits into
mainfrom
matklad/lord-of-the-there-and-back-again
Feb 24, 2025
Merged

VSR: Reverse ring replication for odd ops#2761
matklad merged 3 commits into
mainfrom
matklad/lord-of-the-there-and-back-again

Conversation

@matklad

@matklad matklad commented Feb 21, 2025

Copy link
Copy Markdown
Member

Alternate ring replication direction such that, if a single replica is down, the replicas after it learn that they are missing a prepare, and can repair it.

We originally reverted this commit as it wasn't playing well with our repair, which insisted on everything being done strictly in order. We have since relaxed our repair to be more concurrent:

We don't yet have a deterministic performance test here yet, so I used the following script to test the behavior

#!/usr/bin/env -S deno run --allow-all
import $ from "jsr:@david/dax";

$.setPrintCommand(true);
const working_directory = "zig-out/cluster";
const replica_count = 6;
const replica_indexes = [...Array(replica_count)].map((_, index) => index);
const cluster = 0;
const addresses = [...Array(replica_count)].map((_, index) => `${3000 + index}`)
    .join(",");

await $`./zig/zig build -Drelease`;
try {
    await Deno.remove(working_directory, { recursive: true });
} catch {

}
await Deno.mkdir(working_directory);

await Promise.all(replica_indexes.map((replica) => {
    return $`./tigerbeetle format
            --cluster=${cluster} --replica=${replica} --replica-count=${replica_count}
            ${working_directory}/${cluster}_${replica}.tigerbeetle`;
}));

const dead_replica = 2;
const processes = [];
for (let replica = 0; replica < replica_count; replica += 1) {
    if (replica == dead_replica) continue;
    const process = $`./tigerbeetle start
            --addresses=${addresses}
            ${working_directory}/${cluster}_${replica}.tigerbeetle
            2> ./${working_directory}/${cluster}_${replica}.log`.spawn();
    processes.push(process);
}

await $`./tigerbeetle benchmark --print-batch-timings
         --id-order=random --seed=3 --transfer-batch-size=1000 --transfer-count=200_000
         --addresses=${addresses}
         --clients=4`;

console.log("ok.");

In all-healthy cluster, benchmark finishes in about 7 seconds for me, with or without bidirectional replication.

If a single replica is down:

  • with unidirectional, it takes either 7 seconds still (last replica is down) or takes a lot longer, with individual requests taking up to 12 seconds
  • with bidirectional, it takes 30 seconds regardless of which replica is down.

Just so that its easier to tweak this logic during experiments.
sentientwaffle
sentientwaffle previously approved these changes Feb 21, 2025
@sentientwaffle

Copy link
Copy Markdown
Member

This should help with #2739

@sentientwaffle

Copy link
Copy Markdown
Member

Wait, I'm confused!

If a single replica is down:

  • with unidirectional, it takes either 7 seconds still (last replica is down) or takes a lot longer, with individual requests taking up to 12 seconds
  • with bidirectional, it takes 30 seconds regardless of which replica is down.

Is this saying that bidirectional is slower?

@matklad
matklad added this pull request to the merge queue Feb 21, 2025
@matklad
matklad removed this pull request from the merge queue due to a manual request Feb 21, 2025
@matklad

matklad commented Feb 21, 2025

Copy link
Copy Markdown
Member Author

Actually, that's not precise, let me describe it fully. Let's say we have replicas 0 thru 5, with 0 being the primary. Then, depending on the replica down, we'll get the following performance table:

💀
R1 30s
R2 30s
R3 7s 7 s
R4 7s 30s
R5 7s 30s

(here, ∞ means not the literal infinity, but "too long to wait for test to finish", with individual batch latencies larger than 10 seconds)

This is precisely what's expected: with unidirectional, the cluster basically doesn't notice if the last couple of replicas are eaten by the monsters, but a problem at the Strat of the ring leads to pathological behavior.

With bidirectional both sides of the ring are equally important, so latency is elevated, unless the replica precisely the opposite of primal dies --- in that case, we still have replication quorum in either direction.

(table from memory, but let me re-run the experiments just to confirm)

@matklad

matklad commented Feb 21, 2025

Copy link
Copy Markdown
Member Author

Yup, the table above is correct!

@matklad

matklad commented Feb 22, 2025

Copy link
Copy Markdown
Member Author

Will merge on Monday after the release, just in case.

Replica starts and recoveres the journal:

```
[debug] (journal): 1: recover_slot: recovered slot=0001 label=@m decision=eql operation=vsr.Operation.pulse op=65 view=5
[warn] (journal): 1: recover_slot: recovered slot=0002 label=@g decision=fix operation=vsr.Operation(133) op=34 view=2
````

It then truncates slot 1 due to view range:

```
recover_slots: drop header view_range=2..2 view=5 op=65 checksum=155146884554224562275508079586419833690
```

The second slot is then fixed, repairing the headers:

```
[debug] (journal): 1: recover_slots: dirty=14 faulty=1
[debug] (journal): 1: write_sectors: ring=vsr.journal.Ring.headers offset=0 len=4096 locked
[debug] (journal): 1: write_sectors: ring=vsr.journal.Ring.headers offset=0 len=4096 unlocked
[info] (replica): 1: transition_to_normal_from_recovering_status: view=2 backup
```

Because headers are written sector-wise, this also replaces the
truncated header with the reserved one. As a result, the assert in state
checker trips.

Use wal_prepares, rather wal_headers for assert

Seed: ./zig/zig build vopr -- --lite 8336595450297446987
@matklad

matklad commented Feb 24, 2025

Copy link
Copy Markdown
Member Author

VOPR lite failure is false positive, I've added the fix here to keep the seed on the branch where it actually fails.

@matklad
matklad added this pull request to the merge queue Feb 24, 2025
Merged via the queue into main with commit ba6828b Feb 24, 2025
@matklad
matklad deleted the matklad/lord-of-the-there-and-back-again branch February 24, 2025 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants