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,
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,
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,
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,
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,
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,
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
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:
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
```
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
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
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
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
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
junrao commented on PR #15621:
URL: https://github.com/apache/kafka/pull/15621#issuecomment-2041649750
@chia7712 : There are quite a few test failures on
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 ==
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
```
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
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
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
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
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
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
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
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
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
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
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
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 ==
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
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 ==
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 ==
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 ==
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
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
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
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
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
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)
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
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
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
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
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 ==
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
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
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
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
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
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)
- [ ]
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
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
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
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
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
###
83 matches
Mail list logo