Skip to content

fix(metadata): roll deleted partitions and topics out of parent stats - #4046

Merged
hubcio merged 13 commits into
masterfrom
stats_update_on_delete_partitions
Sep 8, 2026
Merged

fix(metadata): roll deleted partitions and topics out of parent stats#4046
hubcio merged 13 commits into
masterfrom
stats_update_on_delete_partitions

Conversation

@numinnex

@numinnex numinnex commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Update the Stats from metadata layer when partitions get removed.

@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Sep 3, 2026
@hubcio

hubcio commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

please add integration test (either enhance current one if it exists or create new one)

@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.76995% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.02%. Comparing base (6c07954) to head (501881c).

Files with missing lines Patch % Lines
core/common/src/types/streaming_stats.rs 93.70% 8 Missing ⚠️
core/server/src/partition_reconciler.rs 95.34% 6 Missing ⚠️
core/partitions/src/iggy_partition.rs 93.84% 4 Missing ⚠️
core/server/src/http/metrics.rs 94.73% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #4046      +/-   ##
============================================
- Coverage     85.98%   85.02%   -0.96%     
  Complexity     1431     1431              
============================================
  Files          1247     1247              
  Lines        192967   188086    -4881     
  Branches     158323   153443    -4880     
============================================
- Hits         165915   159920    -5995     
- Misses        22963    23857     +894     
- Partials       4089     4309     +220     
Components Coverage Δ
Rust Core 85.72% <97.76%> (-1.17%) ⬇️
Java SDK 67.52% <ø> (ø)
C# SDK 77.08% <ø> (+0.01%) ⬆️
Python SDK 91.31% <ø> (ø)
PHP SDK 85.65% <ø> (ø)
Node SDK 96.26% <ø> (+0.02%) ⬆️
Go SDK 69.43% <ø> (+0.06%) ⬆️
Files with missing lines Coverage Δ
core/metadata/src/stm/stream.rs 92.89% <100.00%> (+1.27%) ⬆️
core/server/src/boot/mod.rs 87.32% <ø> (-0.14%) ⬇️
core/server/src/boot/recovery.rs 95.46% <100.00%> (ø)
core/server/src/responses.rs 92.14% <100.00%> (+0.03%) ⬆️
core/shard/src/lib.rs 83.82% <100.00%> (-0.13%) ⬇️
core/server/src/http/metrics.rs 96.62% <94.73%> (-0.28%) ⬇️
core/partitions/src/iggy_partition.rs 91.82% <93.84%> (+0.09%) ⬆️
core/server/src/partition_reconciler.rs 98.00% <95.34%> (-0.08%) ⬇️
core/common/src/types/streaming_stats.rs 91.83% <93.70%> (+1.17%) ⬆️

... and 139 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hubcio hubcio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one finding sits outside the diff, so it goes here.

core/shard/src/lib.rs:8180 - fence_partition_for_rebuild tombstones and enqueues ConfirmRemove without settling the partition counters. metadata never deleted the partition, so the registry entry survives and the rebuild's get-or-create hands back the fenced incarnation's numbers, still sitting in the parent totals. build_partition_fresh only zeroes on its error path, so journal repair then counts the same data twice.

the second exit at :8160-8178, where quarantine fails, never enqueues ConfirmRemove at all, so a settle in that arm would not reach it. both arms need one.

this path predates the pr, but the invariant it breaks is the one this pr establishes.

Comment thread core/server/src/partition_reconciler.rs Outdated
Comment thread core/server/src/partition_reconciler.rs Outdated
Comment thread core/server/src/boot/mod.rs Outdated
Comment thread core/server/src/http/metrics.rs Outdated
Comment thread core/common/src/types/streaming_stats.rs Outdated
Comment thread core/common/src/types/streaming_stats.rs Outdated
Comment thread core/metadata/src/stm/stream.rs Outdated
Comment thread core/metadata/src/stm/stream.rs Outdated
Comment thread core/integration/tests/server/scenarios/delete_stats_rollback_scenario.rs Outdated
Comment thread core/integration/tests/server/scenarios/delete_stats_rollback_scenario.rs Outdated
@github-actions github-actions Bot added S-waiting-on-author PR is waiting on author response and removed S-waiting-on-review PR is waiting on a reviewer labels Sep 4, 2026
@numinnex

numinnex commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

/ready

@github-actions github-actions Bot added S-waiting-on-review PR is waiting on a reviewer and removed S-waiting-on-author PR is waiting on author response labels Sep 5, 2026
@numinnex
numinnex force-pushed the stats_update_on_delete_partitions branch from 23bef90 to 38a74a5 Compare September 5, 2026 13:09

@hubcio hubcio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

retain_from_snapshot (stream.rs:722) evicts a partition entry without zeroing its stats, so the still-mounted stale incarnation's later ConfirmRemove takes those bytes a second time out of live siblings. No clamp fires, so nothing surfaces it. Reachable on a donor delete-then-recreate installed ahead of the receiver's frontier. Breaks the invariant at stream.rs:738, and created_revision has no test.

On unchanged lines, so no inline comment:

  • boot/recovery.rs:184 points at the zeroing loop this change deleted from boot/mod.rs. Retarget at Streams::from_snapshot.
  • streaming_stats.rs:313-326 and :469-482: zero_out_* cascades unconditionally, so the new settle runs an amount-0 rollback per deleted partition. if prev == 0 { return; } after each swap.
  • Nine public iggy_common return types change from (). A major break with no semver gate - worth a release note.

Comment thread core/metadata/src/stm/stream.rs Outdated
Comment thread core/server/src/partition_reconciler.rs Outdated
Comment thread core/server/src/partition_reconciler.rs
Comment thread core/common/src/types/streaming_stats.rs Outdated
Comment thread core/metadata/src/stm/stream.rs
Comment thread core/server/src/http/metrics.rs Outdated
Comment thread core/common/src/types/streaming_stats.rs Outdated
Comment thread core/common/src/types/streaming_stats.rs Outdated
Comment thread core/common/src/types/streaming_stats.rs
Comment thread core/integration/tests/server/scenarios/mod.rs Outdated
@github-actions github-actions Bot added S-waiting-on-author PR is waiting on author response and removed S-waiting-on-review PR is waiting on a reviewer labels Sep 7, 2026
@numinnex

numinnex commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

/ready

@github-actions github-actions Bot added S-waiting-on-review PR is waiting on a reviewer and removed S-waiting-on-author PR is waiting on author response labels Sep 8, 2026
@hubcio
hubcio merged commit a71ed74 into master Sep 8, 2026
99 checks passed
@hubcio
hubcio deleted the stats_update_on_delete_partitions branch September 8, 2026 09:26
@github-actions github-actions Bot removed the S-waiting-on-review PR is waiting on a reviewer label Sep 8, 2026
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.

3 participants