Skip to content

Add regression test for tenant-boundary shard split data loss - #375

Open
udnay wants to merge 1 commit into
mainfrom
failing-at-shard-split
Open

Add regression test for tenant-boundary shard split data loss#375
udnay wants to merge 1 commit into
mainfrom
failing-at-shard-split

Conversation

@udnay

@udnay udnay commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a focused reproduction for the shard split data-loss bug when isolating a heavy tenant via tenant-boundary splits.

The test:

  • creates a filesystem-backed SlateDB shard,
  • enqueues 256 jobs for a heavy tenant,
  • simulates the two-step split flow used to isolate that tenant,
  • verifies the tenant’s jobs exist after clone,
  • runs after_split_cleanup_defunct_data,
  • asserts the tenant’s jobs still exist after cleanup.

Current behavior fails at the final assertion: the tenant has 256 jobs before cleanup and 0 after cleanup. This localizes the loss to post-split cleanup rather than SlateDB cloning.

Root Cause Exercised

The reproduction shows the mismatch between raw tenant-ID split boundaries and hash-space cleanup checks. The isolated child range is built as a raw string interval like:

[heavy-tenant, heavy-tenant-next)

but cleanup checks:

hash_tenant(tenant) in shard_range

So cleanup treats the isolated tenant as outside its own child shard and deletes its data as defunct.

Test Plan

  • cargo fmt --check passes.
  • cargo test split_at_named_heavy_tenant_preserves_jobs_after_cleanup --test split_aware_processing_tests fails as expected until the split-boundary bug is fixed:
    • T1 before cleanup: 256 jobs
    • T2 after cleanup: 0 jobs
    • cleanup logs show keys_deleted=1025 keys_scanned=1025

Note

Low Risk
Test-only change with no production code paths modified; CI may show a new failing test until the underlying bug is fixed.

Overview
Adds split_at_named_heavy_tenant_preserves_jobs_after_cleanup in split_aware_processing_tests.rs, a filesystem-backed end-to-end repro for losing a heavy tenant’s jobs after tenant-boundary shard splits.

The test enqueues 256 jobs for heavy-tenant, runs the two-step clone flow (ShardFactory + clone_closed_shard), opens the isolated child, and runs after_split_cleanup_defunct_data. It documents the mismatch: ShardRange::contains (lexicographic tenant ID) vs contains_tenant (hash space), which is how cleanup decides what is defunct—so the isolated tenant can be wiped despite being the split target.

The test is expected to fail today (256 jobs before cleanup, 0 after) until cleanup/split-boundary semantics are fixed; it pins the bug to post-split cleanup, not cloning.

Reviewed by Cursor Bugbot for commit 9498ad1. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary

Adds a focused reproduction for the shard split data-loss bug when isolating a heavy tenant via tenant-boundary splits.

The test:

- creates a filesystem-backed SlateDB shard,
- enqueues 256 jobs for a heavy tenant,
- simulates the two-step split flow used to isolate that tenant,
- verifies the tenant’s jobs exist after clone,
- runs after_split_cleanup_defunct_data,
- asserts the tenant’s jobs still exist after cleanup.

Current behavior fails at the final assertion: the tenant has 256 jobs before cleanup and 0 after cleanup. This localizes the loss to post-split cleanup rather than SlateDB cloning.

Root Cause Exercised

The reproduction shows the mismatch between raw tenant-ID split boundaries and hash-space cleanup checks. The isolated child range is built as a raw string interval like:

[heavy-tenant, heavy-tenant-next)

but cleanup checks:

hash_tenant(tenant) in shard_range

So cleanup treats the isolated tenant as outside its own child shard and deletes its data as defunct.

Test Plan

- cargo fmt --check passes.
- cargo test split_at_named_heavy_tenant_preserves_jobs_after_cleanup --test split_aware_processing_tests fails as expected until the split-boundary bug is fixed:
    - T1 before cleanup: 256 jobs
    - T2 after cleanup: 0 jobs
    - cleanup logs show keys_deleted=1025 keys_scanned=1025
@udnay

udnay commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

This change is part of the following stack:

Change managed by git-spice.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9498ad1361

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +150 to +153
assert_eq!(
count_job_info_keys_for_tenant(isolated.db(), HEAVY_TENANT).await,
HEAVY_TENANT_JOB_COUNT,
"T2: split-at-tenant cleanup must not delete the named heavy tenant's jobs"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Don't land a regression test that fails unconditionally

When this test is run in CI or as part of the full suite, this assertion is false with the code in this commit: the test itself verifies the isolated range does not contain the heavy tenant by contains_tenant, and after_split_cleanup_defunct_data uses that predicate to delete out-of-range keys, so the count drops to 0. Please either land the production fix with this regression test or mark the test ignored until the fix is included; otherwise the suite is expected to fail.

Useful? React with 👍 / 👎.

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.

1 participant