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, validatingResults.maxTimestamp
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, validatingResults.maxTimestampMs
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, validatingResults.maxTimestampMs
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, validatingResults.maxTimestamp
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, validatingResults.maxTimestamp
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, validatingResults.maxTimestampMs
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 a
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: jira-unsubscr...@kafka.apache
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
```
https://issues.apache.
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
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 clo
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
```
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
--
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 e
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 automat
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 f
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
chia7712 commented on PR #15621:
URL: https://github.com/apache/kafka/pull/15621#issuecomment-2041650425
> There are quite a few test failures on
[kafka.server.ListOffsetsRequestTest.testResponseIncludesLeaderEpoch()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-15621/35/testRe
junrao commented on PR #15621:
URL: https://github.com/apache/kafka/pull/15621#issuecomment-2041649750
@chia7712 : There are quite a few test failures on
[kafka.server.ListOffsetsRequestTest.testResponseIncludesLeaderEpoch()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-15621/3
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 ==
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 == L
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
```
https://issues.apache.org/j
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 shou
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 r
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
UR
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
KafkaServerTestHarnes
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 the
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,
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 :(
--
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
KafkaServerTestHarnes
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 th
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 Whether
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
KafkaServerTestHarnes
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 Wheth
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 ==
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 offs
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 == L
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
Transact
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 ==
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 == L
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 ==
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 ==
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 == L
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 ==
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 == L
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 ==
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;
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 builtRecords
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;
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 Gi
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
validateMessagesAndAssignOffsetsComp
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;
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.
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
o
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=" +
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 Gi
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)
throws
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 lo
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 ent
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
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
KafkaServerTestHarn
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 o
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 ==
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 ==
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 ==
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 == L
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 `
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
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
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 t
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
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 need
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 whil
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 fo
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
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 whil
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
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)
- [ ] Ver
78 matches
Mail list logo
