Skip to content

MINOR: Improve logging around index files. - #6385

Merged
hachikuji merged 7 commits into
apache:trunkfrom
bob-barrett:index-logging
Mar 19, 2019
Merged

MINOR: Improve logging around index files.#6385
hachikuji merged 7 commits into
apache:trunkfrom
bob-barrett:index-logging

Conversation

@bob-barrett

Copy link
Copy Markdown
Contributor

This patch adds additional DEBUG statements in AbstractIndex.scala, OffsetIndex.scala, and TimeIndex.scala. It also changes the logging on append from DEBUG to TRACE to make DEBUG logging less disruptive.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

This patch adds additional DEBUG statements in AbstractIndex.scala, OffsetIndex.scala, and TimeIndex.scala. It also changes the logging on append from DEBUG to TRACE to amake DEBUG logging less disruptive.

@hachikuji hachikuji left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, makes sense. Left a minor nitpick.

_entries = entries
mmap.position(_entries * entrySize)
_lastOffset = lastEntry.offset
debug(s"Truncated index ${file.getAbsolutePath} to $entries entries; position is now ${mmap.position()} and last offset is now ${_lastOffset}")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we're a little inconsistent on use of file.getName vs file.getAbsolutePath. Also, it would be nice to consistently favor the use of ${} substitutions over format.

@bob-barrett bob-barrett Mar 15, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went with file.getAbsolutePath in order to identify the partition, since we don't have a LogContext. Also replaced format with substitutions

@hachikuji hachikuji left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one minor comment.

lastOffset.foreach { offset =>
if (offset >= abortedTxn.lastOffset)
throw new IllegalArgumentException("The last offset of appended transactions must increase sequentially")
throw new IllegalArgumentException(s"The last offset of appended transactions must increase sequentially (${file.getAbsolutePath})")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the offsets above to this message?

@hachikuji
hachikuji merged commit 70ddd8a into apache:trunk Mar 19, 2019
hachikuji pushed a commit that referenced this pull request Mar 19, 2019
This patch adds additional DEBUG statements in AbstractIndex.scala, OffsetIndex.scala, and TimeIndex.scala. It also changes the logging on append from DEBUG to TRACE to make DEBUG logging less disruptive, and it ensures that exceptions raised from index classes include file/offset information.

Reviewers: Jason Gustafson <jason@confluent.io>
hachikuji pushed a commit that referenced this pull request Mar 19, 2019
This patch adds additional DEBUG statements in AbstractIndex.scala, OffsetIndex.scala, and TimeIndex.scala. It also changes the logging on append from DEBUG to TRACE to make DEBUG logging less disruptive, and it ensures that exceptions raised from index classes include file/offset information.

Reviewers: Jason Gustafson <jason@confluent.io>
hachikuji pushed a commit that referenced this pull request Mar 19, 2019
This patch adds additional DEBUG statements in AbstractIndex.scala, OffsetIndex.scala, and TimeIndex.scala. It also changes the logging on append from DEBUG to TRACE to make DEBUG logging less disruptive, and it ensures that exceptions raised from index classes include file/offset information.

Reviewers: Jason Gustafson <jason@confluent.io>
hachikuji pushed a commit that referenced this pull request Mar 19, 2019
This patch adds additional DEBUG statements in AbstractIndex.scala, OffsetIndex.scala, and TimeIndex.scala. It also changes the logging on append from DEBUG to TRACE to make DEBUG logging less disruptive, and it ensures that exceptions raised from index classes include file/offset information.

Reviewers: Jason Gustafson <jason@confluent.io>
jarekr pushed a commit to confluentinc/kafka that referenced this pull request Apr 18, 2019
* apache/trunk:
  MINOR: Improve logging around index files (apache#6385)
  KAFKA-7858: Automatically generate JoinGroup request/response
  KAFKA-8121; Shutdown ZK client expiry handler earlier during close (apache#6462)
  MINOR: Improve verification in flaky testPartitionReassignmentDuringDeleteTopic (apache#6460)
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
This patch adds additional DEBUG statements in AbstractIndex.scala, OffsetIndex.scala, and TimeIndex.scala. It also changes the logging on append from DEBUG to TRACE to make DEBUG logging less disruptive, and it ensures that exceptions raised from index classes include file/offset information.

Reviewers: Jason Gustafson <jason@confluent.io>
efeg added a commit to linkedin/kafka that referenced this pull request Dec 19, 2019
…cessary resource use during index operations. (#64)

TICKET =
LI_DESCRIPTION = PR apache#6385 has added several debug and trace statements in AbstractIndex.scala, OffsetIndex.scala, and TimeIndex.scala. However, not guarding these logging statements yields waste of resources during index operations. This patch adds log guarding to avoid this resource waste.
EXIT_CRITERIA = MANUAL [""]
gitlw pushed a commit to linkedin/kafka that referenced this pull request May 30, 2020
…cessary resource use during index operations. (#64)

TICKET =
LI_DESCRIPTION = PR apache#6385 has added several debug and trace statements in AbstractIndex.scala, OffsetIndex.scala, and TimeIndex.scala. However, not guarding these logging statements yields waste of resources during index operations. This patch adds log guarding to avoid this resource waste.
EXIT_CRITERIA = MANUAL [""]
gitlw pushed a commit to linkedin/kafka that referenced this pull request Jun 12, 2020
…cessary resource use during index operations. (#64)

TICKET =
LI_DESCRIPTION = PR apache#6385 has added several debug and trace statements in AbstractIndex.scala, OffsetIndex.scala, and TimeIndex.scala. However, not guarding these logging statements yields waste of resources during index operations. This patch adds log guarding to avoid this resource waste.
EXIT_CRITERIA = MANUAL [""]
gitlw pushed a commit to linkedin/kafka that referenced this pull request Jun 13, 2020
…cessary resource use during index operations. (#64)

TICKET =
LI_DESCRIPTION = PR apache#6385 has added several debug and trace statements in AbstractIndex.scala, OffsetIndex.scala, and TimeIndex.scala. However, not guarding these logging statements yields waste of resources during index operations. This patch adds log guarding to avoid this resource waste.
EXIT_CRITERIA = MANUAL [""]
lmr3796 pushed a commit to lmr3796/kafka that referenced this pull request Feb 9, 2022
…cessary resource use during index operations (linkedin#64)

TICKET =
LI_DESCRIPTION = PR apache#6385 has added several debug and trace statements in AbstractIndex.scala, OffsetIndex.scala, and TimeIndex.scala. However, not guarding these logging statements yields waste of resources during index operations. This patch adds log guarding to avoid this resource waste.
EXIT_CRITERIA = MANUAL [""]
lmr3796 pushed a commit to lmr3796/kafka that referenced this pull request Feb 9, 2022
…cessary resource use during index operations (linkedin#64)

TICKET =
LI_DESCRIPTION = PR apache#6385 has added several debug and trace statements in AbstractIndex.scala, OffsetIndex.scala, and TimeIndex.scala. However, not guarding these logging statements yields waste of resources during index operations. This patch adds log guarding to avoid this resource waste.
EXIT_CRITERIA = MANUAL [""]
lmr3796 pushed a commit to lmr3796/kafka that referenced this pull request Mar 25, 2022
…cessary resource use during index operations (linkedin#64)

TICKET =
LI_DESCRIPTION = PR apache#6385 has added several debug and trace statements in AbstractIndex.scala, OffsetIndex.scala, and TimeIndex.scala. However, not guarding these logging statements yields waste of resources during index operations. This patch adds log guarding to avoid this resource waste.
EXIT_CRITERIA = MANUAL [""]
lmr3796 pushed a commit to lmr3796/kafka that referenced this pull request Jun 2, 2022
…cessary resource use during index operations (linkedin#64)

TICKET =
LI_DESCRIPTION = PR apache#6385 has added several debug and trace statements in AbstractIndex.scala, OffsetIndex.scala, and TimeIndex.scala. However, not guarding these logging statements yields waste of resources during index operations. This patch adds log guarding to avoid this resource waste.
EXIT_CRITERIA = MANUAL [""]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants