showuon commented on a change in pull request #11936:
URL: https://github.com/apache/kafka/pull/11936#discussion_r833881280
##########
File path: core/src/test/scala/kafka/tools/GetOffsetShellTest.scala
##########
@@ -166,6 +166,13 @@ class GetOffsetShellTest extends KafkaServerTestHarness
with Logging {
)
}
+ @Test
+ def testNoOffsetIfTimestampGreaterThanLatestRecord(): Unit = {
Review comment:
Checking the code, it looks like the unknown_offset could mean the error
state or, like your test did, the timestamp is not found. Could we also add a
test to verify if the request has some error response, we should get empty
offset, too (due to the unknown_offset response). Ex: the topic doesn't exist.
##########
File path: core/src/main/scala/kafka/tools/GetOffsetShell.scala
##########
@@ -135,7 +135,11 @@ object GetOffsetShell {
val partitionOffsets = partitionInfos.flatMap { tp =>
try {
val partitionInfo = listOffsetsResult.partitionResult(tp).get
- Some((tp, partitionInfo.offset))
+ if (partitionInfo.offset != ListOffsetsResponse.UNKNOWN_OFFSET) {
+ Some((tp, partitionInfo.offset))
+ } else {
+ None
Review comment:
Could we output something to user that like the consumer group command
did:
https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala#L680-L681
ex: "Warn: the offset for Partition $tp is unknown." WDYT?
--
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.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
