You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iggy.apache.org by "krishvishal (via GitHub)" <gi...@apache.org> on 2026/08/21 06:04:49 UTC
[PR] feat(simulator): drive the workload under network and replica faults [iggy]
krishvishal opened a new pull request, #3942:
URL: https://github.com/apache/iggy/pull/3942
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(simulator): drive the workload under network and replica faults [iggy]
Posted by "krishvishal (via GitHub)" <gi...@apache.org>.
krishvishal merged PR #3942:
URL: https://github.com/apache/iggy/pull/3942
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(simulator): drive the workload under network and replica faults [iggy]
Posted by "numinnex (via GitHub)" <gi...@apache.org>.
numinnex commented on code in PR #3942:
URL: https://github.com/apache/iggy/pull/3942#discussion_r3861341222
##########
core/metadata/src/impls/metadata.rs:
##########
@@ -1170,21 +1175,48 @@ where
// guard, not here.
self.checkpoint_if_needed(consensus, journal).await;
- // Backup: gap check (op == current_op + 1).
- // Primary: sequencer pre-advanced by push_prepare_entry (guards
- // sibling on_request races during journal.append await).
- // TODO: promote the backup gap warn below to a hard assert or a
- // repair-session trigger (message repair has landed; the drop-and-
- // wait-for-retransmit path is the last soft handling left here).
+ // Backup: gap check against the JOURNAL head, not the sequencer.
+ //
+ // The two frontiers can disagree. The sequencer is pre-advanced on the
+ // primary by `push_prepare_entry` and re-synced on a backup only after a
+ // successful append, so a replica can carry a sequencer one ahead of what
+ // its WAL holds. Gating admission on it then rejects the very prepare that
+ // would heal the log: a backup with `last_op = 44` refused op 45 because
+ // its sequencer said to expect 46. The primary retransmits that op for the
+ // life of the process, every backup logs an out-of-order gap, it never
+ // reaches a commit quorum, and its client is never answered.
+ //
+ // `max(last_op, snapshot_op)`, never `last_op` alone. A state transfer
+ // installs a snapshot that IS ops `..=snapshot_op` applied and truncates the
+ // WAL above that floor rather than refilling below it, so `last_op` reads the
+ // receiver as needing an op the snapshot already contains and no peer will
+ // send again. That drop
+ // never heals: an offer built on a quiet cluster carries `commit_op ==
+ // snapshot_seq`, so the install lands `commit_min == commit_max`, and
+ // `maybe_request_metadata_repair`, the only path that refills the head,
+ // arms on `commit_min < commit_max`. With the other backup down the primary
+ // needs this replica's ack to commit anything, so the plane stops on a
+ // cluster still inside its quorum.
+ //
+ // The journal is the only frontier that answers "what can be appended
+ // next", which is what this check is for, and the hash-chain verification
+ // below is stated against it too. A prepare at or below the head that this
+ // replica already holds was re-acked and returned above. What reaches HERE
+ // is the next op or a gap, and not every gap is fillable: metadata repair
+ // covers only `commit_min + 1 ..= commit_max`, so an interior hole below
+ // the head and a forward gap above `commit_max` both sit outside it.
let is_backup = consensus.is_follower();
if is_backup {
- if header.op != current_op + 1 {
+ let handle = journal.handle();
+ let journal_head = handle.last_op().unwrap_or(0).max(handle.snapshot_op());
+ if header.op != journal_head + 1 {
Review Comment:
This drop arms no repair, and the PR's own fault config wedges behind it. Repro: `workload-fuzz --plane metadata --crash-prob 0.01 --restart-prob 0.05 --ticks 30000 --seed 1` (also seeds 2 and 5) fails `did not drain`.
Shape: a restarted backup adopts StartView at head N and arms repair up to N; live prepares N+1.. arrive during the repair and are dropped here. The other backup crashes, so `commit_max` is pinned (the primary needs this ack) and `on_commit` never sees `CommitOutcome::Advanced` to re-arm. The stalled-repair retry at `core/shard/src/lib.rs:5077` has no else branch for `from_op > to_op`, so the finished session is never cleared and the `is_none()` gate at `:5379` blocks a new one. The primary retransmits only `commit_max + 1`, which lands here forever. 2/3 live, zero progress.
Pre-existing (the old sequencer gate drops the same op), so not a defect of this PR, but the advertised config is red on roughly a third of seeds. Worth an issue and a mention in the PR body. Fix looks small: clear the session when `from_op > to_op`, and level-trigger the arm in `tick_metadata` when `commit_min < commit_max` with no session.
##########
core/simulator/src/workload/oracle.rs:
##########
@@ -117,29 +139,227 @@ pub fn drive_to_quiesce(sim: &mut Simulator, workload: &mut Workload, max_ticks:
true
}
-/// Post-drain consensus checks that hold today.
+/// Why the run did not drain, as a multi-line report.
///
-/// Asserts no live replica is ahead of the leader, and (on a serial run) that
-/// the shadow equals the metadata committed on the leader. See the module docs
-/// for why full cross-replica equality is deferred.
+/// A failed drain is either a wedge or a merely slow cluster, and the bare boolean
+/// [`drive_to_quiesce`] returns cannot tell them apart. With crashes, restarts and
+/// packet loss in play that distinction is the whole diagnosis, so name what is
+/// outstanding and what every live replica believes.
+#[must_use]
+pub fn quiesce_failure_report(sim: &Simulator, workload: &Workload) -> String {
+ let mut report = format!(
+ "did not drain: {} request(s) still outstanding (seed={:#x})\n",
+ workload.total_in_flight(),
+ workload.options.seed,
+ );
+ for row in workload.outstanding_summary() {
+ let _ = writeln!(
+ report,
+ " outstanding client={} request={} action={:?} last_target=replica {} \
+ attempts={}",
+ row.client, row.request, row.action, row.target, row.attempts,
+ );
+ }
+ let _ = writeln!(report, " resends issued: {}", workload.resends());
+ for replica_idx in 0..sim.replica_count {
+ if sim.is_crashed(replica_idx) {
+ let _ = writeln!(report, " replica {replica_idx}: CRASHED");
+ continue;
+ }
+ let _ = write!(report, " replica {replica_idx}: live");
+ // Metadata plane first: a rejoining replica is quorum-invisible until its
+ // view probe completes, so its status separates "the cluster is slow" from
+ // "the cluster has no quorum despite enough live replicas".
+ if let Some(consensus) = sim.replicas[usize::from(replica_idx)].shards[0]
+ .plane
+ .metadata()
+ .consensus
+ .as_ref()
+ {
+ // The three ways `is_caught_up_primary` stays shut on what otherwise
+ // reads as a healthy Normal primary, silently dropping every request.
+ let _ = write!(
+ report,
+ " | metadata status={:?} view={} log_view={} commit={}..{} barrier={} \
+ transferring={} primary={}",
+ consensus.status(),
+ consensus.view(),
+ consensus.log_view(),
+ consensus.commit_min(),
+ consensus.commit_max(),
Review Comment:
Consider printing the metadata repair session here too (armed or not, `from_op..to_op`). On the seed-1 wedge this shows `commit=1278..1300` for the stuck backup, and the session state is what points at the shard retry path rather than the gap drop.
##########
core/simulator/src/lib.rs:
##########
@@ -2078,909 +2015,1228 @@ mod tests {
sim.init_partition(ns_a);
sim.register_client_with_primary(&client);
- // Phase 1: Create-heavy to populate the shadow.
- let mut options = WorkloadOptions::new(0x5EED_0002, replica_count, vec![ns_a]);
- options.weights = ActionWeights::new(&[(Action::CreateStream, 100)]);
+ let mut options = WorkloadOptions::new(seed, replica_count, vec![ns_a]);
+ options.weights = ActionWeights::partition_only();
let mut wl = Workload::new(options);
- for _tick in 0..3_000u32 {
- if let Some((target, msg)) = wl.build_request(&client) {
- sim.submit_request(client.client_id(), target, msg.into_generic());
- }
- for reply in sim.step() {
- let cmds = wl.on_reply(&reply);
- apply_sim_commands(&mut sim, &cmds);
- }
- }
- let created = wl.auditor.stats().commits_per_action[Action::CreateStream as usize];
- assert!(created > 0, "Create-only workload produced no commits");
- assert_eq!(
- wl.shadow.stream_names.len() as u64,
- created,
- "shadow stream count diverged from CreateStream commits"
- );
- // Phase 2: Create/Delete mix. DeleteStream sample succeeds only if
- // shadow.pick_stream_name returns Some (the wiring under test).
- wl.options.weights =
- ActionWeights::new(&[(Action::CreateStream, 30), (Action::DeleteStream, 70)]);
- for _tick in 0..3_000u32 {
- if let Some((target, msg)) = wl.build_request(&client) {
- sim.submit_request(client.client_id(), target, msg.into_generic());
- }
- for reply in sim.step() {
- let cmds = wl.on_reply(&reply);
- apply_sim_commands(&mut sim, &cmds);
- }
- }
- let deleted = wl.auditor.stats().commits_per_action[Action::DeleteStream as usize];
- let created_total = wl.auditor.stats().commits_per_action[Action::CreateStream as usize];
+ let clients = [client];
+ let replies = workload::run(&mut sim, &mut wl, &clients, 3_000, u64::MAX);
+ assert!(replies > 0, "lossy workload produced no replies");
assert!(
- deleted > 0,
- "DeleteStream never committed; shadow-driven sampling is broken \
- (sample would return None unless pick_stream_name found a live name)"
+ wl.resends() > 0,
+ "no request timed out at 5% packet loss, so the resend path never ran; \
+ raise the loss rate or lower request_timeout_ticks"
);
- let expected_live = created_total.saturating_sub(deleted);
- assert_eq!(
- wl.shadow.stream_names.len() as u64,
- expected_live,
- "shadow.stream_names.len() ({}) != creates ({}) - deletes ({}) = {}",
- wl.shadow.stream_names.len(),
- created_total,
- deleted,
- expected_live,
+
+ assert!(
+ oracle::drive_to_quiesce(&mut sim, &mut wl, 20_000),
+ "{}",
+ oracle::quiesce_failure_report(&sim, &wl),
);
+ oracle::assert_converged(&sim, &mut wl);
}
- /// Drive workload with 4 concurrent clients over two namespaces; assert:
- ///
- /// - Every client observes at least one commit (no starvation).
- /// - Per-(client, namespace) commit-monotonic invariant holds.
- /// - Commits interleave across clients.
- #[test]
- fn multi_client_interleaves_commits() {
+ fn workload_hash_for_seed(seed: u64) -> u64 {
+ workload_hash(seed, 1).0
+ }
+
+ /// Reply-trace and executor-schedule hashes for a full workload run at
+ /// `shards_per_replica` shards. Shared by the single-shard locked
+ /// baseline and the multi-shard replay tests.
+ fn workload_hash(seed: u64, shards_per_replica: u16) -> (u64, u64) {
Review Comment:
`workload_hash` never touches `FaultInjector`, `run_with_faults` or `resubmit_due`, so the new fault path has no same-seed replay test; an injector drawing from `rand::random` would pass `cargo test -p simulator`.
Deleted without a replacement: both `*_solo_replica_restarts_*` tests (the only asserts that restart replay rebuilds the STM and client table), `prepare_timestamps_replay_with_seed`, `multi_shard_replay_is_deterministic` (the schedule hash this fn returns is now discarded), `shell_on_consensus_schedule_matches_shell_off`, and `crash_injection_spares_primary_and_keeps_quorum` (`spare_primary` / `min_survivors` are now unasserted). `SimSuperblock::set_yield_writes` is orphaned by the same deletion.
Suggest restoring the solo-restart pair and adding one test that runs `run_with_faults` twice on a seed and compares reply hash, `schedule_hash()`, crashes and restarts.
##########
core/simulator/src/lib.rs:
##########
@@ -854,7 +1145,11 @@ impl Simulator {
.collect();
materialised.sort_unstable_by_key(IggyNamespace::inner);
for namespace in materialised {
- materialise_partition(&self.replicas[idx], namespace);
+ materialise_partition(
Review Comment:
Ordering on restart: `new_shard` replays the WAL before this call seeds the namespace, while a fresh boot seeds first (`:519`). `CreateStream::apply` takes `vacant_key()` at apply time (`core/metadata/src/stm/stream.rs:1607`), so a restarted replica ends up with `wl-*` streams at slabs 0-1 and the fillers above, the reverse of its peers. Same committed log, different slab ids.
Nothing observes it today (the state checker compares headers, the entity oracle filters by name), but a `DeleteStream` hitting the impersonating slab would surface as a drain failure blamed on the cluster. Seeding before the replay loop in `new_shard`, or journaling the seed, closes it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(simulator): drive the workload under network and replica faults [iggy]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #3942:
URL: https://github.com/apache/iggy/pull/3942#issuecomment-5365996903
## [Codecov](https://app.codecov.io/gh/apache/iggy/pull/3942?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 83.42%. Comparing base ([`c0c7493`](https://app.codecov.io/gh/apache/iggy/commit/c0c74931b1727169daea06dcd438478640ce09f3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`f53b33c`](https://app.codecov.io/gh/apache/iggy/commit/f53b33cf781733df97ba36100c551add9e2e4f7c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #3942 +/- ##
============================================
- Coverage 83.87% 83.42% -0.45%
Complexity 1358 1358
============================================
Files 1212 1210 -2
Lines 166843 166442 -401
Branches 134306 134112 -194
============================================
- Hits 139937 138858 -1079
- Misses 23266 23943 +677
- Partials 3640 3641 +1
```
| [Components](https://app.codecov.io/gh/apache/iggy/pull/3942/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [Rust Core](https://app.codecov.io/gh/apache/iggy/pull/3942/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `84.67% <ø> (+0.02%)` | :arrow_up: |
| [Java SDK](https://app.codecov.io/gh/apache/iggy/pull/3942/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `66.67% <ø> (ø)` | |
| [C# SDK](https://app.codecov.io/gh/apache/iggy/pull/3942/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.54% <ø> (ø)` | |
| [Python SDK](https://app.codecov.io/gh/apache/iggy/pull/3942/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.13% <ø> (ø)` | |
| [PHP SDK](https://app.codecov.io/gh/apache/iggy/pull/3942/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `84.48% <ø> (ø)` | |
| [Node SDK](https://app.codecov.io/gh/apache/iggy/pull/3942/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `89.09% <ø> (-6.76%)` | :arrow_down: |
| [Go SDK](https://app.codecov.io/gh/apache/iggy/pull/3942/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.32% <ø> (ø)` | |
| [Files with missing lines](https://app.codecov.io/gh/apache/iggy/pull/3942?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [core/binary\_protocol/src/primitives/consumer.rs](https://app.codecov.io/gh/apache/iggy/pull/3942?src=pr&el=tree&filepath=core%2Fbinary_protocol%2Fsrc%2Fprimitives%2Fconsumer.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9iaW5hcnlfcHJvdG9jb2wvc3JjL3ByaW1pdGl2ZXMvY29uc3VtZXIucnM=) | `100.00% <ø> (ø)` | |
| [core/metadata/src/impls/metadata.rs](https://app.codecov.io/gh/apache/iggy/pull/3942?src=pr&el=tree&filepath=core%2Fmetadata%2Fsrc%2Fimpls%2Fmetadata.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9tZXRhZGF0YS9zcmMvaW1wbHMvbWV0YWRhdGEucnM=) | `86.52% <ø> (+0.02%)` | :arrow_up: |
| [core/partitions/src/iggy\_partition.rs](https://app.codecov.io/gh/apache/iggy/pull/3942?src=pr&el=tree&filepath=core%2Fpartitions%2Fsrc%2Figgy_partition.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9wYXJ0aXRpb25zL3NyYy9pZ2d5X3BhcnRpdGlvbi5ycw==) | `87.51% <ø> (ø)` | |
| [core/partitions/src/journal.rs](https://app.codecov.io/gh/apache/iggy/pull/3942?src=pr&el=tree&filepath=core%2Fpartitions%2Fsrc%2Fjournal.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9wYXJ0aXRpb25zL3NyYy9qb3VybmFsLnJz) | `89.39% <ø> (ø)` | |
| [core/partitions/src/lib.rs](https://app.codecov.io/gh/apache/iggy/pull/3942?src=pr&el=tree&filepath=core%2Fpartitions%2Fsrc%2Flib.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9wYXJ0aXRpb25zL3NyYy9saWIucnM=) | `0.00% <ø> (ø)` | |
| [core/shard/src/lib.rs](https://app.codecov.io/gh/apache/iggy/pull/3942?src=pr&el=tree&filepath=core%2Fshard%2Fsrc%2Flib.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zaGFyZC9zcmMvbGliLnJz) | `81.39% <ø> (-0.01%)` | :arrow_down: |
| [core/simulator/src/bin/workload-fuzz.rs](https://app.codecov.io/gh/apache/iggy/pull/3942?src=pr&el=tree&filepath=core%2Fsimulator%2Fsrc%2Fbin%2Fworkload-fuzz.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zaW11bGF0b3Ivc3JjL2Jpbi93b3JrbG9hZC1mdXp6LnJz) | `0.00% <ø> (ø)` | |
| [core/simulator/src/client.rs](https://app.codecov.io/gh/apache/iggy/pull/3942?src=pr&el=tree&filepath=core%2Fsimulator%2Fsrc%2Fclient.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zaW11bGF0b3Ivc3JjL2NsaWVudC5ycw==) | `66.73% <ø> (+0.84%)` | :arrow_up: |
| [core/simulator/src/deps.rs](https://app.codecov.io/gh/apache/iggy/pull/3942?src=pr&el=tree&filepath=core%2Fsimulator%2Fsrc%2Fdeps.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zaW11bGF0b3Ivc3JjL2RlcHMucnM=) | `73.95% <ø> (+0.24%)` | :arrow_up: |
| [core/simulator/src/executor/mod.rs](https://app.codecov.io/gh/apache/iggy/pull/3942?src=pr&el=tree&filepath=core%2Fsimulator%2Fsrc%2Fexecutor%2Fmod.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zaW11bGF0b3Ivc3JjL2V4ZWN1dG9yL21vZC5ycw==) | `90.42% <ø> (-0.03%)` | :arrow_down: |
| ... and [36 more](https://app.codecov.io/gh/apache/iggy/pull/3942?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
... and [54 files with indirect coverage changes](https://app.codecov.io/gh/apache/iggy/pull/3942/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org