Fix Kinesis empty segment commit loop after shard merge

[Vamsi-klu]

Why

After a Kinesis shard merge/split, servers correctly stop with endOfPartitionGroup and commit. The controller then re-probed end-of-shard via KinesisStreamMetadataProvider.consumedEndOfShard. On max empty non-EOP polls (including rate-limit empties), the provider assumed the shard was still live. That recreated a new consuming segment on the closed parent, which immediately committed empty — forever — and blocked child-shard admission (shardsEnded).

This is a production reliability bug for any realtime table on Kinesis that rescales shards.

Impact

• Stops unbounded empty segment commit loops after Kinesis rescale.
• Unblocks child shards so ingestion continues on the new lineage.
• Reduces controller/server thrash (ZK IdealState churn, empty builds).
• Localized to the Kinesis metadata provider — no LLC protocol redesign.

How

In KinesisStreamMetadataProvider:

1. Short-circuit when the shard is closed and the checkpoint sequence is already at/past endingSequenceNumber.
2. For closed shards, after empty-only probes (no messages seen), treat the shard as ended instead of fail-closed “still live”.
3. Soften handling of throttle/timeout empty batches so they do not burn the attempt budget the same way as real empty non-EOP responses.
4. Flip/extend unit tests that previously encoded the buggy “max attempts ⇒ still live” expectation.

Test plan

• Extended KinesisStreamMetadataProviderTest (closed shard + empty-only probes ⇒ parent removed; sequence short-circuit; child admission after parent end).
• ./mvnw -pl pinot-plugins/pinot-stream-ingestion/pinot-kinesis -am -Dtest=KinesisStreamMetadataProviderTest -Dsurefire.failIfNoSpecifiedTests=false test
• Manual/staging (optional): merge two Kinesis shards under a Pinot realtime table and confirm no empty-commit loop and children consume.

Related

fixes: #17209

Reviewers

Suggested: maintainers familiar with stream ingestion / Kinesis

---

Was generative AI tooling used to co-author this PR?

• Yes — Grok Build (xAI)

Generated-by: Grok Build (xAI)

[codecov-commenter]

Codecov Report

❌ Patch coverage is 53.33333% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.47%. Comparing base (2bcbdfe) to head (880ffbd).

Files with missing lines	Patch %	Lines
.../stream/kinesis/KinesisStreamMetadataProvider.java	53.33%	17 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master   #19086   +/-   ##
=========================================
  Coverage     65.47%   65.47%           
- Complexity     1421     1430    +9     
=========================================
  Files          3426     3426           
  Lines        217315   217354   +39     
  Branches      34509    34517    +8     
=========================================
+ Hits         142283   142318   +35     
- Misses        63513    63517    +4     
  Partials      11519    11519           

Flag	Coverage Δ
custom-integration1	?
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	?
java-25	65.47% <53.33%> (+<0.01%)	⬆️
temurin	65.47% <53.33%> (+<0.01%)	⬆️
unittests	65.47% <53.33%> (+<0.01%)	⬆️
unittests1	56.82% <ø> (-0.01%)	⬇️
unittests2	37.89% <53.33%> (+0.01%)	⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

[Vamsi-klu]

Ready for review — all required CI checks are green on this PR.

Issue: #17209

Could the following folks take a look when convenient?
@Jackie-Jiang

Formal GitHub "Request review" is unavailable from a fork contributor account on apache/pinot (API returns 404/403), so this is the review ping.

Thank you!

[Jackie-Jiang]

cc @swaminathanmanish @KKcorps

[Copilot]

Pull request overview

Fixes a controller-side reliability issue in the Kinesis stream ingestion path where, after shard split/merge, closed parent shards could be treated as still-live and repeatedly recreated as empty consuming segments (blocking child shard admission).

Changes:

• Add closed-shard short-circuiting based on checkpoint sequence number vs. endingSequenceNumber, and revise the end-of-shard probe loop to treat empty-only probes on closed shards as “ended” (with special handling for throttle/timeout empties).
• Introduce a dedicated max-empty-probe constant and a time/attempt budget to prevent unbounded probing behavior.
• Update and extend KinesisStreamMetadataProviderTest to cover the fixed behavior (parent removal, child admission, SN short-circuit, throttle/timeout behavior).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
pinot-plugins/pinot-stream-ingestion/pinot-kinesis/src/main/java/org/apache/pinot/plugin/stream/kinesis/KinesisStreamMetadataProvider.java	Changes end-of-shard detection logic for closed shards to avoid reopening parents and triggering infinite empty-commit loops.
pinot-plugins/pinot-stream-ingestion/pinot-kinesis/src/test/java/org/apache/pinot/plugin/stream/kinesis/KinesisStreamMetadataProviderTest.java	Updates existing tests and adds new cases validating closed-shard handling, short-circuiting, and throttle/timeout probe behavior.