KAFKA-19474: Move WARN log on log truncation below HWM #20106
Conversation
github-actions
bot
added
triage
|
CC: @hachikuji @junrao can you please take a look. This is related to https://github.com/apache/kafka/pull/5608/files#diff-693a5eaaaa4fe61a18af23d5b06ccc024261dda01558cbaacb7020e1911c6d55R264 |
There was a problem hiding this comment.
@gaurav-narula : Thanks for the PR. LGTM. Will wait for the CI to complete.
| @@ -166,12 +166,12 @@ class ReplicaFetcherThread(name: String, | |||
| val partition = replicaMgr.getPartitionOrException(tp) | |||
| val log = partition.localLogOrException | |||
|
|
|||
| partition.truncateTo(offsetTruncationState.offset, isFuture = false) | |||
|
|
|||
| if (offsetTruncationState.offset < log.highWatermark) | |||
There was a problem hiding this comment.
Perhaps this log could be moved into UnifiedLog#truncateTo. UnifiedLog#truncateTo already logs if the target offset is larger than or equal to end offset, so adding another log for this case seems to make sense. Additionally, ReplicaAlterLogDirsThread could issue the same warning if the log is moved to UnifiedLog#truncateTo
There was a problem hiding this comment.
Thanks for the feedback! I've moved the log inside UnifiedLog#truncateTo.
Although users might be able to disambiguate between the two cases using the thread name (ReplicaAlterLogDirsThread vs ReplicaFetcherThread), I felt it's perhaps better to disambiguate between a future/non-future replica in the log statement
Moves the check for `offsetTruncationState.offset < log.highWatermark` before truncating the log as truncation might update the HWM thereby leading to an incorrect condition evaluation.
gaurav-narula
force-pushed
the
KAFKA-19474
branch
from
e8f2fb8 to
dbaa0ec
Compare
|
@gaurav-narula could you please tweak the PR description according to the latest updates? |
|
@chia7712 Updated. PTAL again. |
#5608 introduced a regression where the check for
targetOffset < log.highWatermarkto emit a
WARNlog was made incorrectly after truncating the log.This change moves the check for
targetOffset < log.highWatermarktoUnifiedLog#truncateToand ensures we emit aWARNlog on truncationbelow the replica's HWM by both the
ReplicaFetcherThreadandReplicaAlterLogDirsThreadReviewers: Jun Rao junrao@gmail.com, Chia-Ping Tsai
chia7712@gmail.com