Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-05-08 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1594849626 ## core/src/test/scala/unit/kafka/log/LogValidatorTest.scala: ## @@ -404,9 +414,7 @@ class LogValidatorTest { assertEquals(now + 1,

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-05-08 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1594333143 ## core/src/test/scala/unit/kafka/log/LogValidatorTest.scala: ## @@ -404,9 +414,7 @@ class LogValidatorTest { assertEquals(now + 1,

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-05-08 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1594333143 ## core/src/test/scala/unit/kafka/log/LogValidatorTest.scala: ## @@ -404,9 +414,7 @@ class LogValidatorTest { assertEquals(now + 1,

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-05-07 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1593260374 ## core/src/test/scala/unit/kafka/log/LogValidatorTest.scala: ## @@ -404,9 +414,7 @@ class LogValidatorTest { assertEquals(now + 1,

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-05-07 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1593241011 ## core/src/test/scala/unit/kafka/log/LogValidatorTest.scala: ## @@ -404,9 +414,7 @@ class LogValidatorTest { assertEquals(now + 1,

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-05-07 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1592859538 ## core/src/test/scala/unit/kafka/log/LogValidatorTest.scala: ## @@ -404,9 +414,7 @@ class LogValidatorTest { assertEquals(now + 1,

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-09 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2046489133 @junrao @showuon thanks for all your reviews and helps! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-09 Thread via GitHub
chia7712 merged PR #15621: URL: https://github.com/apache/kafka/pull/15621 -- 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-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-09 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2046457414 ``` Build / JDK 21 and Scala 2.13 / testInvalidPasswordSaslScram() – org.apache.kafka.common.security.authenticator.SaslAuthenticatorFailureNoDelayTest ```

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-09 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2045917889 > In the last run, it seem that JDK 21 and Scala 2.13 didn't complete. Could you trigger another build? Typically, this could be done by closing the PR, waiting for 20 secs and opening

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-09 Thread via GitHub
junrao commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2045841631 @chia7712 : Thanks for triaging the failed tests. In the last run, it seem that JDK 21 and Scala 2.13 didn't complete. Could you trigger another build? Typically, this could be done by

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-09 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2045819511 ``` Build / JDK 11 and Scala 2.13 / testLowMaxFetchSizeForRequestAndPartition(String, String).quorum=kraft+kip848.groupProtocol=consumer – kafka.api.PlaintextConsumerFetchTest

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-09 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2045781013 ``` Build / JDK 11 and Scala 2.13 / testLowMaxFetchSizeForRequestAndPartition(String, String).quorum=kraft+kip848.groupProtocol=consumer – kafka.api.PlaintextConsumerFetchTest --

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-09 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2044846640 > Thanks for the updated PR. The code looks good to me. There were 50 failed tests. Is any of them related to the PR? If not, have they all been tracked? there are many timeout

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-08 Thread via GitHub
junrao commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2043792883 @chia7712 : Thanks for the updated PR. The code looks good to me. There were 50 failed tests. Is any of them related to the PR? If not, have they all been tracked? -- This is an

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-08 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2043274125 @junrao thanks for reviews. both comments get addressed in https://github.com/apache/kafka/pull/15621/commits/581242c1fa6c005bf91a7ced96932774c2c02cd9 -- This is an automated message

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-08 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1556068446 ## core/src/test/scala/integration/kafka/admin/ListOffsetsIntegrationTest.scala: ## @@ -56,11 +60,33 @@ class ListOffsetsIntegrationTest extends KafkaServerTestHarness

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-07 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2041650425 > There are quite a few test failures on

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-07 Thread via GitHub
junrao commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2041649750 @chia7712 : There are quite a few test failures on

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-06 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1554638802 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,15 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-06 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1554155328 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,15 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp ==

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-06 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2041118676 ``` Build / JDK 21 and Scala 2.13 / testReplicateSourceDefault() – org.apache.kafka.connect.mirror.integration.MirrorConnectorsIntegrationBaseTest ```

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-05 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2040270805 > Regarding the previous failed tests, one possibility is that the data on the server passed the retention time and is garbage collected. The default retention time is 7 days, which

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-05 Thread via GitHub
junrao commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2040231361 @chia7712 : Thanks for the updated PR. Regarding the previous failed tests, one possibility is that the data on the server passed the retention time and is garbage collected. The default

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-04 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2038916109 previous failed tests are gone. rebase to trigger QA again -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-04 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1552358023 ## core/src/test/scala/integration/kafka/admin/ListOffsetsIntegrationTest.scala: ## @@ -189,14 +215,56 @@ class ListOffsetsIntegrationTest extends

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-04 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2038064405 @junrao thanks for reviews. I have removed the useless log and revise the test. let us see what happens. -- This is an automated message from the Apache Git Service. To respond to

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-04 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1552264764 ## storage/src/main/java/org/apache/kafka/storage/internals/log/LogSegment.java: ## @@ -483,12 +484,13 @@ public int recover(ProducerStateManager producerStateManager,

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-04 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2037697246 > (kafka.admin.ListOffsetsIntegrationTest.testThreeCompressedRecordsInSeparateBatch(String).quorum=kraft) failed with the following. I'm trying to reproduce it on my local :(

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-04 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1552022713 ## core/src/test/scala/integration/kafka/admin/ListOffsetsIntegrationTest.scala: ## @@ -189,14 +220,49 @@ class ListOffsetsIntegrationTest extends

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-04 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2037601997 merge trunk to trigger QA again. Also, the error seems happen due to unchanged leader. will check it later -- This is an automated message from the Apache Git Service. To respond to

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-03 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1550393556 ## clients/src/main/java/org/apache/kafka/common/record/RecordBatch.java: ## @@ -245,4 +246,23 @@ public interface RecordBatch extends Iterable { * @return

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-03 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1550271336 ## core/src/test/scala/integration/kafka/admin/ListOffsetsIntegrationTest.scala: ## @@ -56,11 +60,38 @@ class ListOffsetsIntegrationTest extends

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-03 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1550270774 ## clients/src/main/java/org/apache/kafka/common/record/RecordBatch.java: ## @@ -245,4 +246,23 @@ public interface RecordBatch extends Iterable { * @return

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-03 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1550270383 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,15 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-03 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2035323125 > There were 69 test failures and quite a few of them related to ListOffset There is another PR (https://github.com/apache/kafka/pull/15489) encounters same error that listing

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-03 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1550055646 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,15 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp ==

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-03 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2034230452 the failed tests pass on my local. ``` ./gradlew cleanTest :streams:test --tests SlidingWindowedKStreamIntegrationTest.shouldRestoreAfterJoinRestart :storage:test --tests

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-02 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1548765182 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,17 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-02 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1548687661 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,17 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp ==

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-02 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1548654826 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,17 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-02 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1548644261 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,17 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-02 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1548633965 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,17 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp ==

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-02 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1548530489 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,16 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-02 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1548487542 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,16 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp ==

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-02 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1548438900 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,16 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-02 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1548427028 ## clients/src/main/java/org/apache/kafka/common/record/MemoryRecordsBuilder.java: ## @@ -240,25 +240,54 @@ public MemoryRecords build() { return builtRecords;

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-02 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1548390558 ## clients/src/main/java/org/apache/kafka/common/record/MemoryRecordsBuilder.java: ## @@ -240,25 +240,54 @@ public MemoryRecords build() { return

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-02 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r154830 ## clients/src/main/java/org/apache/kafka/common/record/MemoryRecordsBuilder.java: ## @@ -240,25 +240,40 @@ public MemoryRecords build() { return builtRecords;

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-02 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2032614122 @junrao thanks for additional reviews. I have addressed them except https://github.com/apache/kafka/pull/15621#discussion_r1548230600 -- This is an automated message from the Apache

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-02 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1548258165 ## storage/src/main/java/org/apache/kafka/storage/internals/log/LogValidator.java: ## @@ -434,7 +442,8 @@ public ValidationResult

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-02 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1548211287 ## clients/src/main/java/org/apache/kafka/common/record/MemoryRecordsBuilder.java: ## @@ -240,25 +240,40 @@ public MemoryRecords build() { return builtRecords;

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-01 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2030856554 @junrao thanks for reviews. I have addressed all comments, and add new test `testLogAppendTimeNonCompressedV0` to cover v0 -- This is an automated message from the Apache Git Service.

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-01 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1546994725 ## storage/src/main/java/org/apache/kafka/storage/internals/log/LogValidator.java: ## @@ -275,7 +278,7 @@ public ValidationResult assignOffsetsNonCompressed(LongRef

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-01 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1546966401 ## storage/src/main/java/org/apache/kafka/storage/internals/log/LogAppendInfo.java: ## @@ -259,7 +259,7 @@ public String toString() { ", lastOffset=" +

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-01 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2030454105 @junrao thanks for all your reviews and patience. all comments are addressed -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-01 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1546692495 ## storage/src/main/java/org/apache/kafka/storage/internals/log/LogSegment.java: ## @@ -232,17 +233,17 @@ private boolean canConvertToRelativeOffset(long offset)

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-31 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2029213022 rebase code and apply Luke's patch from https://github.com/chia7712/kafka/pull/3/files -- This is an automated message from the Apache Git Service. To respond to the message, please

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-29 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2027904152 > I think both are important. First, it's important to be able to derive the same thing consistently from the leader and the follower log. This affects things like the time indexing

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-29 Thread via GitHub
junrao commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2027526301 > Do we need to revert all of them? the paths we had fixed works well now. > > 1. It seems to me adding comments for both "recover" and "follower" cases can remind readers that this

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-29 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1544283639 ## core/src/test/scala/integration/kafka/admin/ListOffsetsIntegrationTest.scala: ## @@ -189,14 +190,47 @@ class ListOffsetsIntegrationTest extends

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
chia7712 commented on PR #15621: URL: https://github.com/apache/kafka/pull/15621#issuecomment-2026641230 > (1) revert all offsetForMaxTimestamp to shallowOffsetMaxTimestamp (2) change/revert the implementation to set shallowOffsetMaxTimestamp accordingly. Do we need to revert all

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
chia7712 commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1544046500 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,16 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
showuon commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1543916425 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,16 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp ==

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
showuon commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1543906755 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,16 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp ==

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
junrao commented on code in PR #15621: URL: https://github.com/apache/kafka/pull/15621#discussion_r1543754297 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1337,13 +1337,16 @@ class UnifiedLog(@volatile var logStartOffset: Long, } else if (targetTimestamp ==

[PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
chia7712 opened a new pull request, #15621: URL: https://github.com/apache/kafka/pull/15621 We do iterate the records to find the `offsetOfMaxTimestamp` instead of returning the cached one when handling `ListOffsetsRequest.MAX_TIMESTAMP`, since it is hard to align all paths to get correct

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
chia7712 commented on PR #15618: URL: https://github.com/apache/kafka/pull/15618#issuecomment-2026013673 > I am just saying that we only need to fix this in trunk since the implementation was never correct in any previous branches, thus not a regression. got it. will open another PR

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
chia7712 closed pull request #15618: KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… URL: https://github.com/apache/kafka/pull/15618 -- 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

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
junrao commented on PR #15618: URL: https://github.com/apache/kafka/pull/15618#issuecomment-2026000277 > I am not sure I understand this. All we need for this solution (or workaround) is the "max timestamp" of a segment, since we always iterate the batches (from the segment having the max

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
junrao commented on code in PR #15618: URL: https://github.com/apache/kafka/pull/15618#discussion_r1543560539 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1320,10 +1320,8 @@ class UnifiedLog(@volatile var logStartOffset: Long, // constant time access while

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
chia7712 commented on PR #15618: URL: https://github.com/apache/kafka/pull/15618#issuecomment-2025943523 > Since the follower only maintains offsetForMaxTimestamp at the batch level, the listMaxTimestamp API was never implemented correctly. I am not sure I understand this. All we

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
chia7712 commented on code in PR #15618: URL: https://github.com/apache/kafka/pull/15618#discussion_r1543505409 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1320,10 +1320,8 @@ class UnifiedLog(@volatile var logStartOffset: Long, // constant time access

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
junrao commented on PR #15618: URL: https://github.com/apache/kafka/pull/15618#issuecomment-2025853565 @chia7712: Since the follower only maintains offsetForMaxTimestamp at the batch level, the listMaxTimestamp API was never implemented correctly. So, technically, there was no regression

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
junrao commented on code in PR #15618: URL: https://github.com/apache/kafka/pull/15618#discussion_r1543430656 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1320,10 +1320,8 @@ class UnifiedLog(@volatile var logStartOffset: Long, // constant time access while

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
chia7712 commented on code in PR #15618: URL: https://github.com/apache/kafka/pull/15618#discussion_r1543390343 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1320,10 +1320,8 @@ class UnifiedLog(@volatile var logStartOffset: Long, // constant time access

Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
junrao commented on code in PR #15618: URL: https://github.com/apache/kafka/pull/15618#discussion_r1543349555 ## core/src/main/scala/kafka/log/UnifiedLog.scala: ## @@ -1320,10 +1320,8 @@ class UnifiedLog(@volatile var logStartOffset: Long, // constant time access while

[PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-03-28 Thread via GitHub
chia7712 opened a new pull request, #15618: URL: https://github.com/apache/kafka/pull/15618 - add default implementation to Batch to return offset of max timestamp - copy ListOffsetsIntegrationTest from trunk branch ### Committer Checklist (excluded from commit message) - [ ]

Re: [PR] KAFKA-16310: ListOffsets doesn't report the offset with maxTimestamp anymore [kafka]

2024-03-04 Thread via GitHub
KevinZTW commented on PR #15461: URL: https://github.com/apache/kafka/pull/15461#issuecomment-1976256564 @showuon much thanks for helping me look into this! Base on the information on the Jira ticket, It seems that there are other developers already found the root cause and going to submit

Re: [PR] KAFKA-16310: ListOffsets doesn't report the offset with maxTimestamp anymore [kafka]

2024-03-04 Thread via GitHub
KevinZTW closed pull request #15461: KAFKA-16310: ListOffsets doesn't report the offset with maxTimestamp anymore URL: https://github.com/apache/kafka/pull/15461 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL

Re: [PR] KAFKA-16310: ListOffsets doesn't report the offset with maxTimestamp anymore [kafka]

2024-03-04 Thread via GitHub
showuon commented on PR #15461: URL: https://github.com/apache/kafka/pull/15461#issuecomment-1976165637 This is not an easy one. I need more time to dig into it. Thanks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use

Re: [PR] KAFKA-16310: ListOffsets doesn't report the offset with maxTimestamp anymore [kafka]

2024-03-03 Thread via GitHub
showuon commented on PR #15461: URL: https://github.com/apache/kafka/pull/15461#issuecomment-1975543018 Will check it today. Thanks. -- 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

[PR] KAFKA-16310: ListOffsets doesn't report the offset with maxTimestamp anymore [kafka]

2024-03-03 Thread via GitHub
KevinZTW opened a new pull request, #15461: URL: https://github.com/apache/kafka/pull/15461 When add the message with timestamp through producer, get offset with max timestamp didn't get the expected offset jira ticket: https://issues.apache.org/jira/browse/KAFKA-16310 ###