viktorsomogyi commented on code in PR #19137:
URL: https://github.com/apache/kafka/pull/19137#discussion_r1983354737
##########
core/src/main/scala/kafka/log/UnifiedLog.scala:
##########
@@ -1522,6 +1522,9 @@ class UnifiedLog(@volatile var logStartOffset: Long,
val startMs = time.milliseconds
def shouldDelete(segment: LogSegment, nextSegmentOpt: Option[LogSegment]):
Boolean = {
+ if (startMs < segment.largestTimestamp()) {
+ warn(s"Segment with base offset $segment contains future timestamp")
Review Comment:
Thinking about the bigger problem, I think that usually Kafka shouldn't
allow future log messages, only maybe +12 hours because of time zones.
Unfortunately we're quite late for making this breaking change in 4.0, so
there's that. Is someone has a different use case, it's better if they tweak
the configs to their use-case instead of allowing everything. Assuming that the
majority of use cases are when we don't allow future timestamps I think it's
fairer to log on warn than debug, I guess this is your argument too.
I'm more worried about the other way. If someone expects future timestamps,
they'll likely have a lot, so for them the log will be polluted with warn
messages. For them setting the logger to error isn't the best solution either
as they may miss the info level logs when needed for tracing a problem.
The best I can think of is leaving this on warn but defining a separate
logger just for this. It is a bit weird but it can be disabled separately if
someone has a use-case where future messages are normal and they're annoyed by
this.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
