Skip to content

receive: treat an unset tenant_matcher_type as exact in shard size overrides - #9003

Open
SaiPisey2 wants to merge 2 commits into
thanos-io:mainfrom
SaiPisey2:fix/shuffle-sharding-empty-tenant-matcher
Open

SaiPisey2 wants to merge 2 commits into
thanos-io:mainfrom
SaiPisey2:fix/shuffle-sharding-empty-tenant-matcher

Conversation

@SaiPisey2

Copy link
Copy Markdown

Changes

Fixes #8983.

getShardSize switched on the matcher type and only handled the explicit exact value:

switch override.TenantMatcherType {
case TenantMatcherTypeExact:
    ...
case TenantMatcherGlob:

An override that leaves tenant_matcher_type unset therefore matches no case and is skipped, so the tenant silently falls back to the default shard size — losing the routing determinism the override exists to provide.

Empty already means exact everywhere else in the package: it is documented on TenantMatcherTypeExact ("This is also the default one"), and isExactMatcher treats "" and "exact" alike for tenantSet.match. This uses that same helper so the two paths agree.

Verification

Added TestShuffleShardHashringGetShardSize, covering an override with no matcher type, one with an explicit exact, a glob, and a tenant with no override. The unset case returns 2 instead of 3 on main and passes here.

Note three tests fail on main in my environment (darwin/arm64), unrelated to this change and present before it: TestRelabelWithUnsetValidationScheme, TestKetamaHashringEvenNodeSpread and writecapnp.TestMarshalWriteRequest.

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

SaiPisey2 added a commit to SaiPisey2/thanos that referenced this pull request Sep 1, 2026
Signed-off-by: SaiPisey2 <piseysai0202@gmail.com>
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.57%. Comparing base (bc87f18) to head (0460f48).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #9003   +/-   ##
=======================================
  Coverage   64.57%   64.57%           
=======================================
  Files         289      289           
  Lines       37410    37410           
=======================================
  Hits        24156    24156           
  Misses      11155    11155           
  Partials     2099     2099           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

…errides

getShardSize switched on the matcher type and only handled the explicit
"exact" value, so an override that left tenant_matcher_type unset matched
no case and was skipped. The tenant silently fell back to the default
shard size, which breaks the determinism the override is there to
provide.

An empty matcher type already means exact everywhere else: it is
documented on TenantMatcherTypeExact and handled by isExactMatcher, which
tenantSet.match uses. Use the same helper here.

Signed-off-by: SaiPisey2 <piseysai0202@gmail.com>
Signed-off-by: SaiPisey2 <piseysai0202@gmail.com>
@SaiPisey2
SaiPisey2 force-pushed the fix/shuffle-sharding-empty-tenant-matcher branch from 8dee2c9 to 0460f48 Compare September 18, 2026 08:33
@SaiPisey2

Copy link
Copy Markdown
Author

Rebased onto main. The conflict was only the changelog, where #9014 landed in the same ### Fixed block; both entries are kept, in PR order.

26 checks pass. Ready for review whenever someone has a moment.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Empty tenant_matcher_type in shuffle sharding override breaks shard_size determinism

1 participant