Skip to content

Fix Kinesis empty segment commit loop after shard merge - #19086

Open
Vamsi-klu wants to merge 1 commit into
apache:masterfrom
Vamsi-klu:fix/17209-kinesis-empty-segment-commit-loop
Open

Fix Kinesis empty segment commit loop after shard merge#19086
Vamsi-klu wants to merge 1 commit into
apache:masterfrom
Vamsi-klu:fix/17209-kinesis-empty-segment-commit-loop

Conversation

@Vamsi-klu

Copy link
Copy Markdown
Contributor

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)

After shard merge/split, fail-closed end-of-shard probes treated closed drained
shards as still live, so the controller recreated empty consuming segments
forever and blocked child shards. Closed-shard short-circuit and empty-only
probes now mark the parent ended.
@codecov-commenter

codecov-commenter commented Jul 25, 2026

Copy link
Copy Markdown

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

Copy link
Copy Markdown
Contributor Author

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!

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.

Repeated empty segments commits for Kinesis merged Shards

2 participants