Skip to content

MINOR: FetchSessionHandler unnecessarily generates strings for DEBUG logging at INFO level#7394

Merged
ijuma merged 1 commit intoapache:trunkfrom
lbradstreet:trace-debug-fetch-session-elide
Sep 29, 2019
Merged

MINOR: FetchSessionHandler unnecessarily generates strings for DEBUG logging at INFO level#7394
ijuma merged 1 commit intoapache:trunkfrom
lbradstreet:trace-debug-fetch-session-elide

Conversation

@lbradstreet
Copy link
Copy Markdown
Contributor

@lbradstreet lbradstreet commented Sep 26, 2019

Profiling while benchmarking shows FetchSessionHandler unnecessary calls to responseDataToLogString when logging was set to INFO level. This leads to 1.47% of the JVM CPU time going to responseDataToLogString. This PR fixes this issue.

image

@lbradstreet
Copy link
Copy Markdown
Contributor Author

retest this please

1 similar comment
@lbradstreet
Copy link
Copy Markdown
Contributor Author

retest this please

Copy link
Copy Markdown
Member

@ijuma ijuma left a comment

Choose a reason for hiding this comment

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

LGTM

@lbradstreet
Copy link
Copy Markdown
Contributor Author

retest this please

2 similar comments
@lbradstreet
Copy link
Copy Markdown
Contributor Author

retest this please

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Sep 28, 2019

retest this please

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Sep 29, 2019

Scala 2.12 and 2.13 builds passed, 2.11 failed due to a Jenkins issue:

hudson.plugins.git.GitException: Could not init /home/jenkins/jenkins-slave/workspace/kafka-pr-jdk8-scala2.11@2
20:43:48 at org.jenkinsci.plugins.gitclient.CliGitAPIImpl$5.execute(CliGitAPIImpl.java:81

Given that the change doesn't affect Scala code, this is ready to be merged.

@ijuma ijuma merged commit 1dc5063 into apache:trunk Sep 29, 2019
ijuma added a commit to confluentinc/kafka that referenced this pull request Sep 29, 2019
Conflicts:
* .gitignore: addition of clients/src/generated-test was near
local additions for support-metrics.
* checkstyle/suppressions.xml: upstream refactoring of exclusions
for generator were near the local changes for support-metrics.
* gradle.properties: scala version bump caused a minor conflict
due to the kafka version change locally.
gradle/dependencies.gradle: bcpkix version bump was near avro
additions in the local version.

* apache-github/trunk: (49 commits)
  KAFKA-8471: Replace control requests/responses with automated protocol (apache#7353)
  MINOR: Don't generate unnecessary strings for debug logging in FetchSessionHandler (apache#7394)
  MINOR:fixed typo and removed outdated varilable name (apache#7402)
  KAFKA-8934: Create version file during build for Streams (apache#7397)
  KAFKA-8319: Make KafkaStreamsTest a non-integration test class (apache#7382)
  KAFKA-6883: Add toUpperCase support to sasl.kerberos.principal.to.local rule (KIP-309)
  KAFKA-8907; Return topic configs in CreateTopics response (KIP-525) (apache#7380)
  MINOR: Address review comments for KIP-504 authorizer changes (apache#7379)
  MINOR: add versioning to request and response headers (apache#7372)
  KAFKA-7273: Extend Connect Converter to support headers (apache#6362)
  MINOR: improve the Kafka RPC code generator (apache#7340)
  MINOR: Improve the org.apache.kafka.common.protocol code (apache#7344)
  KAFKA-8880: Docs on upgrade-guide (apache#7385)
  KAFKA-8179: do not suspend standby tasks during rebalance (apache#7321)
  KAFKA-8580: Compute RocksDB metrics (apache#7263)
  KAFKA-8880: Add overloaded function of Consumer.committed (apache#7304)
  HOTFIX: fix Kafka Streams upgrade note for broker backward compatibility (apache#7363)
  KAFKA-8848; Update system tests to use new AclAuthorizer (apache#7374)
  MINOR: remove unnecessary null check (apache#7299)
  KAFKA-6958: Overload methods for group and windowed stream to allow to name operation name using the new Named class (apache#6413)
  ...
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