Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-08 Thread via GitHub
lucasbru merged PR #15525: URL: https://github.com/apache/kafka/pull/15525 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail:

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-03 Thread via GitHub
lianetm commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1550191029 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -808,16 +808,55 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-03 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1550094782 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -808,16 +808,55 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-03 Thread via GitHub
lianetm commented on PR #15525: URL: https://github.com/apache/kafka/pull/15525#issuecomment-2034772501 Hey @philipnee, thanks for the updates, just one minor comment left above. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-03 Thread via GitHub
lianetm commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1549857291 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -808,16 +808,55 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
philipnee commented on PR #15525: URL: https://github.com/apache/kafka/pull/15525#issuecomment-2033505687 hi @lianetm - Much appreciate for the reviews. I think I've addressed your comments. LMK if there's anything more. cc @lucasbru -- This is an automated message from the Apache Git

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548891748 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -805,4 +805,73 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548887021 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerCommitTest.scala: ## @@ -304,6 +304,60 @@ class PlaintextConsumerCommitTest extends

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
lianetm commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548614429 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerCommitTest.scala: ## @@ -304,6 +304,60 @@ class PlaintextConsumerCommitTest extends AbstractConsumerTest

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
lianetm commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548603219 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetAndTimestampInternal.java: ## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548599057 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetAndTimestampInternal.java: ## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
lianetm commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548596033 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetAndTimestampInternal.java: ## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548593720 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -805,4 +805,73 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548593720 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -805,4 +805,73 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
lianetm commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548593248 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java: ## @@ -1141,21 +1146,29 @@ private Map beginningOrEndOffset(Collection

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
lianetm commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548591307 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetAndTimestampInternal.java: ## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548585054 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetFetcherUtils.java: ## @@ -240,20 +241,48 @@ Map getOffsetResetTimestamp() {

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548581658 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetAndTimestampInternal.java: ## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
lianetm commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548580872 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -805,4 +805,73 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
lianetm commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548576809 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -805,4 +805,73 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548574194 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java: ## @@ -1141,21 +1146,29 @@ private Map beginningOrEndOffset(Collection

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
lianetm commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548571273 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetFetcherUtils.java: ## @@ -240,20 +241,48 @@ Map getOffsetResetTimestamp() { return

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
lianetm commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548554563 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetAndTimestampInternal.java: ## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
lianetm commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548547539 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java: ## @@ -1141,21 +1146,29 @@ private Map beginningOrEndOffset(Collection

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-04-02 Thread via GitHub
lianetm commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1548547539 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java: ## @@ -1141,21 +1146,29 @@ private Map beginningOrEndOffset(Collection

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-28 Thread via GitHub
philipnee commented on PR #15525: URL: https://github.com/apache/kafka/pull/15525#issuecomment-2026690869 @lucasbru - Thanks for taking time reviewing this PR. This PR is ready for another pass. -- This is an automated message from the Apache Git Service. To respond to the message,

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-28 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1544067091 ## clients/src/test/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumerTest.java: ## @@ -946,7 +956,7 @@ public void

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-28 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1544046037 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java: ## @@ -1141,21 +1141,27 @@ private Map beginningOrEndOffset(Collection

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-28 Thread via GitHub
philipnee commented on PR #15525: URL: https://github.com/apache/kafka/pull/15525#issuecomment-2025522451 hi @lucasbru - Let me address Lianets comment in this PR and have a separated PR for the behavior inconsistency as it does require some changes to the unit test -- This is an

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-28 Thread via GitHub
lucasbru commented on PR #15525: URL: https://github.com/apache/kafka/pull/15525#issuecomment-2024835245 @philipnee Okay, thanks for creating the ticket. Not sure if it's blocker priority though. If it's a quick thing, you could address it in this PR. Are you going to implement

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-27 Thread via GitHub
philipnee commented on PR #15525: URL: https://github.com/apache/kafka/pull/15525#issuecomment-2023293507 @lucasbru - If I'm not mistaken, the current implementation for both beginningOrEndOffsets and OffsetsForTimes both need to send out a request upon getting ZERO duration. Seems like

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-27 Thread via GitHub
lucasbru commented on PR #15525: URL: https://github.com/apache/kafka/pull/15525#issuecomment-2022475971 > @lucasbru - Thanks again for reviewing the PR. Sorry about the misinterpretation on short circuting logic so here I updated the beginningOrEndOffsets API. It seems like the right

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-27 Thread via GitHub
lucasbru commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1540879732 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ListOffsetsEvent.java: ## @@ -25,22 +25,15 @@ import java.util.Map; /** - * Event for

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-26 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1540114887 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ListOffsetsEvent.java: ## @@ -25,22 +25,15 @@ import java.util.Map; /** - * Event for

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-26 Thread via GitHub
lianetm commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1539933946 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ListOffsetsEvent.java: ## @@ -25,22 +25,15 @@ import java.util.Map; /** - * Event for

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-26 Thread via GitHub
philipnee commented on PR #15525: URL: https://github.com/apache/kafka/pull/15525#issuecomment-2020963772 @lucasbru - Thanks again for reviewing the PR. Sorry about the misinterpretation on short circuting logic so here I updated the beginningOrEndOffsets API. It seems like the right

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-26 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1539524047 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java: ## @@ -1141,21 +1141,27 @@ private Map beginningOrEndOffset(Collection

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-26 Thread via GitHub
lucasbru commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1538901757 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java: ## @@ -1141,21 +1141,27 @@ private Map beginningOrEndOffset(Collection

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-26 Thread via GitHub
lucasbru commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1538863396 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java: ## @@ -1141,21 +1141,27 @@ private Map beginningOrEndOffset(Collection

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-25 Thread via GitHub
philipnee commented on PR #15525: URL: https://github.com/apache/kafka/pull/15525#issuecomment-2018886221 Hey @lucasbru - Thanks for taking the time to review this PR. Let me know if there's anything to add to the PR. -- This is an automated message from the Apache Git Service. To

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-25 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1538195311 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetsRequestManager.java: ## @@ -423,11 +432,11 @@ private CompletableFuture

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-25 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1537804695 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/MembershipManagerImpl.java: ## @@ -411,8 +411,8 @@ public int memberEpoch() { public void

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-25 Thread via GitHub
lucasbru commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1537350070 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ListOffsetsEvent.java: ## @@ -32,8 +32,7 @@ * {@link OffsetAndTimestamp} found (offset

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-25 Thread via GitHub
philipnee commented on PR #15525: URL: https://github.com/apache/kafka/pull/15525#issuecomment-2017309149 Hey @lucasbru - Would it be possible to ask you to review this PR? Thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-21 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1535039670 ## clients/src/test/java/org/apache/kafka/clients/consumer/internals/OffsetsRequestManagerTest.java: ## @@ -480,11 +475,16 @@ public void

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-21 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1535038917 ## clients/src/test/java/org/apache/kafka/clients/consumer/internals/OffsetsRequestManagerTest.java: ## @@ -131,13 +131,11 @@ public void

Re: [PR] KAFKA-16156: beginningOrEndOffsets does not need to build an OffsetAndTimestamps object upon completion [kafka]

2024-03-21 Thread via GitHub
philipnee commented on code in PR #15525: URL: https://github.com/apache/kafka/pull/15525#discussion_r1535038917 ## clients/src/test/java/org/apache/kafka/clients/consumer/internals/OffsetsRequestManagerTest.java: ## @@ -131,13 +131,11 @@ public void