You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/09/20 17:18:27 UTC
[GitHub] [kafka] guozhangwang opened a new pull request, #12667: MINOR: Improve logging with throwing OOOSE
guozhangwang opened a new pull request, #12667:
URL: https://github.com/apache/kafka/pull/12667
We need to have the full information on how the seq / epochs are inferred with logging.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] guozhangwang commented on pull request #12667: KAFKA-10635: Improve logging with throwing OOOSE
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on PR #12667:
URL: https://github.com/apache/kafka/pull/12667#issuecomment-1275079800
ping @nicktelford , have you been able to try out this patch and collect the improved logs?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on a diff in pull request #12667: KAFKA-10635: Improve logging with throwing OOOSE
Posted by GitBox <gi...@apache.org>.
showuon commented on code in PR #12667:
URL: https://github.com/apache/kafka/pull/12667#discussion_r975975334
##########
core/src/main/scala/kafka/log/ProducerStateManager.scala:
##########
@@ -233,10 +233,16 @@ private[log] class ProducerAppendInfo(val topicPartition: TopicPartition,
// If there is no current producer epoch (possibly because all producer records have been deleted due to
// retention or the DeleteRecords API) accept writes with any sequence number
- if (!(currentEntry.producerEpoch == RecordBatch.NO_PRODUCER_EPOCH || inSequence(currentLastSeq, appendFirstSeq))) {
+ if (currentEntry.producerEpoch != RecordBatch.NO_PRODUCER_EPOCH && !inSequence(currentLastSeq, appendFirstSeq)) {
throw new OutOfOrderSequenceException(s"Out of order sequence number for producer $producerId at " +
- s"offset $offset in partition $topicPartition: $appendFirstSeq (incoming seq. number), " +
- s"$currentLastSeq (current end sequence number)")
+ s"offset $offset in partition $topicPartition: producer request epoch $producerEpoch, " +
+ s"current entry epoch ${currentEntry.producerEpoch}, " +
+ s"updating entry epoch ${updatedEntry.producerEpoch}, " +
+ s"append first seq $appendFirstSeq, " +
+ s"updating entry last seq ${updatedEntry.lastSeq}, " +
+ s"updating batch empty ${updatedEntry.isEmpty}" +
Review Comment:
nit: updating entry last seq will be `-1` if `updatedEntry.isEmpty`. Is this line still necessary? Or you think it's possible this value might be changed by other thread?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] guozhangwang commented on a diff in pull request #12667: MINOR: Improve logging with throwing OOOSE
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on code in PR #12667:
URL: https://github.com/apache/kafka/pull/12667#discussion_r975630163
##########
core/src/main/scala/kafka/log/ProducerStateManager.scala:
##########
@@ -216,12 +216,12 @@ private[log] class ProducerAppendInfo(val topicPartition: TopicPartition,
private def checkSequence(producerEpoch: Short, appendFirstSeq: Int, offset: Long): Unit = {
if (producerEpoch != updatedEntry.producerEpoch) {
- if (appendFirstSeq != 0) {
- if (updatedEntry.producerEpoch != RecordBatch.NO_PRODUCER_EPOCH) {
- throw new OutOfOrderSequenceException(s"Invalid sequence number for new epoch of producer $producerId " +
- s"at offset $offset in partition $topicPartition: $producerEpoch (request epoch), $appendFirstSeq (seq. number), " +
- s"${updatedEntry.producerEpoch} (current producer epoch)")
- }
+ if (appendFirstSeq != 0 && updatedEntry.producerEpoch != RecordBatch.NO_PRODUCER_EPOCH) {
Review Comment:
Consolidating nested if-s
##########
core/src/main/scala/kafka/log/ProducerStateManager.scala:
##########
@@ -233,10 +233,16 @@ private[log] class ProducerAppendInfo(val topicPartition: TopicPartition,
// If there is no current producer epoch (possibly because all producer records have been deleted due to
// retention or the DeleteRecords API) accept writes with any sequence number
- if (!(currentEntry.producerEpoch == RecordBatch.NO_PRODUCER_EPOCH || inSequence(currentLastSeq, appendFirstSeq))) {
+ if (currentEntry.producerEpoch != RecordBatch.NO_PRODUCER_EPOCH && !inSequence(currentLastSeq, appendFirstSeq)) {
Review Comment:
Unfolding the conditions.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] nicktelford commented on pull request #12667: KAFKA-10635: Improve logging with throwing OOOSE
Posted by GitBox <gi...@apache.org>.
nicktelford commented on PR #12667:
URL: https://github.com/apache/kafka/pull/12667#issuecomment-1275111955
Hey @guozhangwang, we deployed this today, but we need to wait a couple of days for our app to recover before we can test a leadership election. I'm hoping I'll be able to do this towards the end of the week.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] divijvaidya commented on pull request #12667: KAFKA-10635: Improve logging with throwing OOOSE
Posted by "divijvaidya (via GitHub)" <gi...@apache.org>.
divijvaidya commented on PR #12667:
URL: https://github.com/apache/kafka/pull/12667#issuecomment-1594713861
@nicktelford ping
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org