Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

[transactions] Reduce spammy log during broker shutdown #1896

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Demogorgon314
Copy link
Member

Motivation

Reduce spammy log during broker shutdown

Documentation

Check the box below.

Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

@github-actions github-actions bot added the no-need-doc This pr does not need any document label Jun 8, 2023
@codecov
Copy link

codecov bot commented Jun 8, 2023

Codecov Report

Merging #1896 (cb8994f) into master (d246ebb) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

❗ Current head cb8994f differs from pull request most recent head f2823f4. Consider uploading reports for the commit f2823f4 to get more accurate results

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1896      +/-   ##
============================================
- Coverage     18.77%   18.77%   -0.01%     
  Complexity      743      743              
============================================
  Files           184      184              
  Lines         13289    13292       +3     
  Branches       1211     1213       +2     
============================================
  Hits           2495     2495              
- Misses        10613    10616       +3     
  Partials        181      181              
Impacted Files Coverage Δ
...r/transaction/TransactionMarkerChannelManager.java 0.00% <0.00%> (ø)
...ion/TransactionMarkerRequestCompletionHandler.java 0.00% <ø> (ø)
...native/pulsar/handlers/kop/utils/ByteBufUtils.java 0.00% <0.00%> (ø)

Comment on lines 333 to 339
if (throwable instanceof PulsarClientException.LookupException
|| throwable.getCause() instanceof PulsarClientException.LookupException) {
log.warn("Failed to find broker for topic partition {} - {}", topicPartition,
throwable + "");
} else {
log.warn("Failed to find broker for topic partition {}", topicPartition, throwable);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

How could it reduce the spammy logs? For the lookup exception, there are still warn logs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but when the cause is LookupException, it won't print the stack info.

Copy link
Collaborator

Choose a reason for hiding this comment

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

it won't print the stack info.

I think not. See line 335, it's still log.warn.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants