fix: rank unknown buckets by client position

[shikhar]

Summary

• Remove the fixed unknown-bucket score penalty.
• Score buckets without observations by client position only, so preferred unknown buckets can gather stats before later known buckets.
• Update stats tests for the new unknown-bucket ordering behavior.

Tests

• cargo +nightly fmt
• cargo nextest run object_store::stats
• cargo clippy --all-features --all-targets -- -D warnings --allow deprecated
• cargo nextest run
• cargo deny check

[greptile-apps]

Greptile Summary

Removes the fixed 5000-point UNKNOWN_BUCKET_PENALTY so that buckets with no observations are ranked by position index alone (idx × POSITION_PENALTY), rather than being buried below a large static penalty. This lets a client's preferred unknown bucket be explored before a later known-but-slow bucket, which was not possible before.

• Scoring change: unknown-bucket score drops from base + 5000 to base; the crossover point at which an unknown position-N bucket overtakes a known position-0 bucket falls from ~700 ms to ~200 ms observed latency.
• Test updates: test_realistic_s3_latencies now expects unknown tertiary to rank ahead of a 395 ms secondary; test_unknown_buckets_use_position_only adds a reverse-order assertion confirming unknown position-0 always goes first; test_regional_bucket_ordering_preserved comment corrected to reflect an exact score tie (200 ms = 2000 pts = 1 × POSITION_PENALTY) rather than a penalty-dominated ordering.

Confidence Score: 4/5

Safe to merge; the change is a single-function tweak with comprehensive test coverage of the new ordering semantics.

The implementation is minimal and correct — removing one constant and one addition, with no risk of underflow or misuse. All affected test scenarios are updated and the new behavior (unknown buckets scored by position only) is exercised from multiple angles. The one thing worth keeping in mind is that test_regional_bucket_ordering_preserved now relies on an exact arithmetic tie (200 ms observed latency = 1 × POSITION_PENALTY = 2000 pts), so the test is marginally more load-bearing than it appears at first glance, but the ExponentialDecayHistogram mean for 10 identical 200 ms samples will always truncate to exactly 2000 integer points.

src/object_store/stats.rs — the new score tie in test_regional_bucket_ordering_preserved warrants a second read to confirm the chosen 200 ms value is deliberately pinned to POSITION_PENALTY.

Important Files Changed

Filename	Overview
src/object_store/stats.rs	Removes UNKNOWN_BUCKET_PENALTY (5000 pts) so unobserved buckets score by position only; updates all test expectations accordingly. The change is self-contained and correct; tests verify the new ordering semantics thoroughly.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["score(now, bucket, idx)"] --> B{"by_bucket.get(bucket)"}
    B -- "None (unknown)" --> C["score = idx × 2000"]
    B -- "Some(stats)" --> D["Acquire Mutex lock"]
    D --> E["lat = mean_latency_µs / 100"]
    D --> F{"consecutive_failures ≥ 5?"}
    F -- "Yes" --> G["err = 1_000_000 (circuit open)"]
    F -- "No" --> H["err = error_rate × 100_000"]
    E --> I["score = idx×2000 + err + lat"]
    G --> I
    H --> I

    style C fill:#ffd700,stroke:#b8860b
    style G fill:#ff6b6b,stroke:#cc0000

Loading

_{Reviews (1): Last reviewed commit: "fix: rank unknown buckets by client posi..." | Re-trigger Greptile}