KAFKA-17108: Add EarliestPendingUpload offset spec in ListOffsets API by abhijeetk88 · Pull Request #16584 · apache/kafka

abhijeetk88 · 2024-07-12T17:18:50Z

This is the first part of the implementation of
KIP-1023

The purpose of this pull request is for the broker to start returning
the correct offset when it receives a -6 as a timestamp in a ListOffsets
API request.

Added unit tests for the new timestamp.

Reviewers: Kamal Chandraprakash kamal.chandraprakash@gmail.com

kamalcph

Thanks for the patch! Left minor comments to address.

Should we need to add an OffsetSpec on the client side to make the earliest_pending_upload_offset queryable from command line tools similar to 6e32448?

kamalcph · 2024-08-05T09:04:43Z

+            val epochResult: Optional[Integer] =
+              if (leaderEpochCache.isDefined) {
+                val epochOpt = leaderEpochCache.get.epochForOffset(earliestPendingUploadOffset)
+                if (epochOpt.isPresent) {
+                  Optional.of(epochOpt.getAsInt)
+                } else {
+                  Optional.empty()
+                }
+              } else {
+                Optional.empty()
+              }
+            Some(new TimestampAndOffset(RecordBatch.NO_TIMESTAMP, earliestPendingUploadOffset, epochResult))


Suggested change

val epochResult: Optional[Integer] =

if (leaderEpochCache.isDefined) {

val epochOpt = leaderEpochCache.get.epochForOffset(earliestPendingUploadOffset)

if (epochOpt.isPresent) {

Optional.of(epochOpt.getAsInt)

} else {

Optional.empty()

}

} else {

Optional.empty()

}

Some(new TimestampAndOffset(RecordBatch.NO_TIMESTAMP, earliestPendingUploadOffset, epochResult))

val epochResult = leaderEpochCache.flatMap(_.epochForOffset(earliestPendingUploadOffset).asScala)

.asJava.asInstanceOf[Optional[Integer]]

Some(new TimestampAndOffset(RecordBatch.NO_TIMESTAMP, earliestPendingUploadOffset, epochResult))

Done. Thanks for the suggestion.

There are other places in the same method can benefit from this refactor as well like line 1319 and 1298

Made the requested changes. Also changed for LATEST_TIMESTAMP

abhijeetk88 · 2024-08-11T09:07:14Z

Should we need to add an OffsetSpec on the client side to make the earliest_pending_upload_offset queryable from command line tools similar to 6e32448?

@kamalcph I can make it as part of a separate change. Will that be ok?

OmniaGM · 2024-08-13T15:25:30Z

        } else {
          Some(new TimestampAndOffset(RecordBatch.NO_TIMESTAMP, -1L, Optional.of(-1)))
        }
+      } else if (targetTimestamp == ListOffsetsRequest.EARLIEST_PENDING_UPLOAD_OFFSET_TIMESTAMP) {


Can we extract this into different method and call it from here especially that the logic here has too many if stataments ? I know the rest of fetchOffsetByTimestamp has been done in-place but the method has grown and it hard to follow.

kamalcph

Overall LGTM. Left one minor comment to address. Thanks for covering the patch with detailed unit tests!

abhijeetk88 · 2024-08-16T15:14:51Z

@kamalcph @OmniaGM addressed your comments. PTAL again.

kamalcph

LGTM, thanks for addressing the review comments!

Should we have to bump the ListOffsetRequest version for this change? PTAL.

cc @showuon @satishd

kamalcph · 2024-10-07T10:18:16Z

@abhijeetk88

Please rebase this PR with trunk.

github-actions · 2025-01-06T03:38:53Z

This PR is being marked as stale since it has not had any activity in 90 days. If you
would like to keep this PR alive, please leave a comment asking for a review. If the PR has
merge conflicts, update it with the latest from the base branch.

If you are having difficulty finding a reviewer, please reach out on the [mailing list](https://kafka.apache.org/contact).

If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed.

abhijeetk88 · 2025-01-12T13:01:57Z

I would like to revive this PR. @kamalcph @showuon will you be able to help with reviews. I can rebase the PR with trunk.

github-actions · 2025-04-13T04:11:47Z

This PR is being marked as stale since it has not had any activity in 90 days. If you
would like to keep this PR alive, please leave a comment asking for a review. If the PR has
merge conflicts, update it with the latest from the base branch.

If you are having difficulty finding a reviewer, please reach out on the [mailing list](https://kafka.apache.org/contact).

If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed.

github-actions · 2025-05-14T03:44:34Z

This PR has been closed since it has not had any activity in 120 days. If you feel like this
was a mistake, or you would like to continue working on it, please feel free to re-open the
PR and ask for a review.

kamalcph

Thanks for the patch! Please address the below comments:

ListOffsetsRequest version needs a bump as a new targetTimestamp value is added . Previous comment and
Also, could you add the EarliestPendingUploadSpec in OffsetSpec and cover with integration tests?

kamalcph · 2025-08-26T05:07:19Z

@abhijeetk88

Could you follow this #16781 PR and handle similarly for EarliestPendingUploadSpec? Thanks!

kamalcph

LGTM, thanks for the patch!

chia7712 · 2025-08-27T06:01:47Z

@abhijeetk88 Do you mind documenting these new APIs in upgrade.html?

chia7712 · 2025-08-27T06:13:44Z

+     * <br/>
+     * Note: When tiered storage is not enabled, we will return unknown offset.
+     */
+    public static OffsetSpec earliestPendingUpload() {


I mean this new public API

sure, will add.

…apache#16584) This is the first part of the implementation of [KIP-1023](https://cwiki.apache.org/confluence/display/KAFKA/KIP-1023%3A+Follower+fetch+from+tiered+offset) The purpose of this pull request is for the broker to start returning the correct offset when it receives a -6 as a timestamp in a ListOffsets API request. Added unit tests for the new timestamp. Reviewers: Kamal Chandraprakash <kamal.chandraprakash@gmail.com>

abhijeetk88 marked this pull request as draft July 12, 2024 17:18

abhijeetk88 force-pushed the KAFKA-17108 branch from 4e50e93 to 39ab781 Compare July 23, 2024 10:21

abhijeetk88 force-pushed the KAFKA-17108 branch 6 times, most recently from b4f63a6 to d89d53b Compare August 5, 2024 06:26

abhijeetk88 marked this pull request as ready for review August 5, 2024 06:30

kamalcph added the tiered-storage Related to the Tiered Storage feature label Aug 5, 2024

kamalcph self-requested a review August 5, 2024 07:55

kamalcph reviewed Aug 5, 2024

View reviewed changes

abhijeetk88 force-pushed the KAFKA-17108 branch 3 times, most recently from e40dd55 to 5684997 Compare August 8, 2024 15:17

OmniaGM reviewed Aug 13, 2024

View reviewed changes

kamalcph reviewed Aug 13, 2024

View reviewed changes

Comment thread clients/src/main/java/org/apache/kafka/common/requests/ListOffsetsRequest.java Outdated

abhijeetk88 force-pushed the KAFKA-17108 branch 2 times, most recently from bff7bf8 to 5e548a3 Compare August 16, 2024 15:11

abhijeetk88 requested review from OmniaGM and kamalcph August 16, 2024 15:15

kamalcph reviewed Aug 16, 2024

View reviewed changes

showuon self-assigned this Sep 6, 2024

github-actions Bot added the stale Stale PRs label Jan 6, 2025

github-actions Bot removed the stale Stale PRs label Jan 13, 2025

github-actions Bot added the stale Stale PRs label Apr 13, 2025

github-actions Bot added the closed-stale PRs that were closed due to inactivity label May 14, 2025

github-actions Bot closed this May 14, 2025

satishd reopened this Aug 18, 2025

github-actions Bot added triage PRs from the community core Kafka Broker clients and removed triage PRs from the community closed-stale PRs that were closed due to inactivity stale Stale PRs labels Aug 18, 2025

abhijeetk88 force-pushed the KAFKA-17108 branch from 5e548a3 to 2e030b6 Compare August 21, 2025 08:59

github-actions Bot added the storage Pull requests that target the storage module label Aug 21, 2025

abhijeetk88 requested a review from kamalcph August 21, 2025 09:05

abhijeetk88 force-pushed the KAFKA-17108 branch from 2e030b6 to 8644b22 Compare August 21, 2025 09:12

KAFKA-17108: Earliest pending upload offset

b9a0de9

abhijeetk88 force-pushed the KAFKA-17108 branch from 8644b22 to b9a0de9 Compare August 21, 2025 09:14

kamalcph reviewed Aug 24, 2025

View reviewed changes

Comment thread core/src/test/scala/unit/kafka/log/UnifiedLogTest.scala

Add OffsetSpec

5d4a7e6

github-actions Bot added the tools label Aug 25, 2025

Fix failing MetadataVersionTest

35d0911

Address review comments

5401eec

kamalcph approved these changes Aug 26, 2025

View reviewed changes

kamalcph changed the title ~~KAFKA-17108: Earliest pending upload offset~~ KAFKA-17108: Add EarliestPendingUpload offset spec in ListOffsets API Aug 26, 2025

kamalcph merged commit 8d93d10 into apache:trunk Aug 27, 2025
34 of 36 checks passed

chia7712 reviewed Aug 27, 2025

View reviewed changes

Uh oh!

Conversation

abhijeetk88 commented Jul 12, 2024 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kamalcph left a comment

Choose a reason for hiding this comment

Uh oh!

kamalcph Aug 5, 2024

Choose a reason for hiding this comment

Uh oh!

abhijeetk88 Aug 8, 2024

Choose a reason for hiding this comment

Uh oh!

OmniaGM Aug 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abhijeetk88 Aug 16, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

abhijeetk88 commented Aug 11, 2024

Uh oh!

OmniaGM Aug 13, 2024

Choose a reason for hiding this comment

Uh oh!

abhijeetk88 Aug 16, 2024

Choose a reason for hiding this comment

Uh oh!

kamalcph left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

abhijeetk88 commented Aug 16, 2024

Uh oh!

kamalcph left a comment

Choose a reason for hiding this comment

Uh oh!

kamalcph commented Oct 7, 2024

Uh oh!

github-actions Bot commented Jan 6, 2025

Uh oh!

abhijeetk88 commented Jan 12, 2025

Uh oh!

github-actions Bot commented Apr 13, 2025

Uh oh!

github-actions Bot commented May 14, 2025

Uh oh!

kamalcph left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kamalcph commented Aug 26, 2025

Uh oh!

kamalcph left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chia7712 commented Aug 27, 2025

Uh oh!

chia7712 Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

abhijeetk88 Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

abhijeetk88 commented Jul 12, 2024 •

edited by github-actions Bot

Loading

OmniaGM Aug 13, 2024 •

edited

Loading