Skip to content

Use latest LLC segment (not first) for partition assignment; skip if it is on tier#18468

Merged
Jackie-Jiang merged 4 commits into
apache:masterfrom
deepthi912:fix/strict-realtime-existing-assignment-tier-aware-v2
May 11, 2026
Merged

Use latest LLC segment (not first) for partition assignment; skip if it is on tier#18468
Jackie-Jiang merged 4 commits into
apache:masterfrom
deepthi912:fix/strict-realtime-existing-assignment-tier-aware-v2

Conversation

@deepthi912
Copy link
Copy Markdown
Collaborator

Summary

BaseStrictRealtimeSegmentAssignment.getExistingAssignment returned the first LLC
segment it found for a partition. When multi-tier dedup tables rebalance and a partition's
CONSUMING segment moves to a new instance pool, the older segment lingering in
idealState can be picked first — causing the next CONSUMING segment to land on
cold tier instead of hot tiers.

This PR, scoped to MultiTierStrictRealtimeSegmentAssignment, changes the behavior to:

  1. Pick the latest LLC segment for the partition (highest sequence number),
    not the first one encountered. Iteration order of currentAssignment is
    lexicographic on segment name (Helix ZNRecord.getMapFields() is a TreeMap),
    which diverges from numeric sequence order — so explicit comparison is required.
  2. Honor tier placement: if the chosen segment has been moved to a tier
    (SegmentZKMetadata.getTier() != null), it lives on a different instance pool
    than the CONSUMING segment, so return null and let assignSegment fall back
    to the assignment derived from instance partitions.

The base class is unchanged in behavior — the only modification is visibility
(privateprotected) to allow the subclass override. Single-tier strict
realtime assignment is untouched.

deepthi912 and others added 3 commits May 11, 2026 12:07
…Assignment

In BaseStrictRealtimeSegmentAssignment.getExistingAssignment, iterate over all
entries and select the LLC segment with the highest sequence number for the
target partition instead of returning the first match. Then look up the chosen
segment's ZK metadata: if it has already been moved to a tier, its instance
pool differs from the CONSUMING pool, so return null and fall back to the
assignment derived from instance partitions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…egmentAssignment

Revert getExistingAssignment in the base class to its original first-match
behavior; only change is visibility (private -> protected) so subclasses can
override it.

In MultiTierStrictRealtimeSegmentAssignment, override getExistingAssignment to
pick the LLC segment with the highest sequence number for the partition, then
consult ZK metadata: if that segment has been moved to a tier it lives on a
different instance pool than a new CONSUMING segment, so return null and fall
back to the assignment derived from instance partitions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nd honors tier

Two new tests on the dedup (multi-tier) path:
* testMultiTierAssignSegmentUsesLatestExistingAssignment seeds partition 0 with
  an older segment on the old CONSUMING instances and a newer segment on the
  new CONSUMING instances, then verifies a new CONSUMING segment lands on the
  new instances. The "first match" behavior would have produced the old
  instances, so the test catches a regression of the override.
* testMultiTierAssignSegmentSkipsExistingWhenLatestOnTier marks the latest
  partition-0 segment as living on a tier in ZK metadata and verifies the new
  CONSUMING segment falls back to the assignment decided by new instance
  partitions instead of reusing the tiered segment's instances.

A small helix-manager helper accepts a set of "tiered" segment names so we can
stub SegmentZKMetadata.getTier() per segment.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Jackie-Jiang Jackie-Jiang added bug Something is not working as expected upsert Related to upsert functionality tiered-storage Related to tiered storage support labels May 11, 2026
@deepthi912 deepthi912 added the dedup Changes related to realtime ingestion dedup handling label May 11, 2026
* Drop uploaded-segment tracking from the override — only committed LLC
  segments matter for picking the latest segment per partition.
* Return null (and log a warning) when the latest segment's ZK metadata is
  unavailable, in addition to the existing tier-based skip; the prior tier
  case is also raised from info to warn so both branches are observable.
* Test nit: replace Collections.emptySet() with Set.of() in the helix-manager
  factory helper.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 11, 2026

Codecov Report

❌ Patch coverage is 74.07407% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.64%. Comparing base (5ebd445) to head (2f7781e).

Files with missing lines Patch % Lines
...ment/MultiTierStrictRealtimeSegmentAssignment.java 74.07% 4 Missing and 3 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master   #18468       +/-   ##
=============================================
+ Coverage     34.99%   63.64%   +28.64%     
- Complexity      869     1684      +815     
=============================================
  Files          3256     3256               
  Lines        199548   199575       +27     
  Branches      30986    30994        +8     
=============================================
+ Hits          69840   127020    +57180     
+ Misses       123570    62418    -61152     
- Partials       6138    10137     +3999     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-21 63.64% <74.07%> (+28.64%) ⬆️
temurin 63.64% <74.07%> (+28.64%) ⬆️
unittests 63.64% <74.07%> (+28.64%) ⬆️
unittests1 55.70% <ø> (?)
unittests2 34.99% <74.07%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Jackie-Jiang Jackie-Jiang merged commit 83728e0 into apache:master May 11, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something is not working as expected dedup Changes related to realtime ingestion dedup handling tiered-storage Related to tiered storage support upsert Related to upsert functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants