Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize the debug log that affects performance, and unify the style #13498

Merged

Conversation

liudezhi2098
Copy link
Contributor

Master Issue: #13497

Motivation

Optimize the debug log that affects performance, and unify the style

Modifications

add isDebugEnabled method

 if (log.isDebugEnabled()) {
    log.debug();
}

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: ( no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

  • no-need-doc

@github-actions
Copy link

@liudezhi2098:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@@ -105,7 +105,9 @@ public void add(T event, long ts, Record<?> record) {
public void add(Event<T> windowEvent) {
// watermark events are not added to the queue.
if (windowEvent.isWatermark()) {
log.debug(String.format("Got watermark event with ts %d", windowEvent.getTimestamp()));
if (log.isDebugEnabled()) {
log.debug(String.format("Got watermark event with ts %d", windowEvent.getTimestamp()));
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to use String.format, just
log.debug("Got watermark event with ts {}", windowEvent.getTimestamp());

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes , I will delete

log.debug(String.format("invoking windowLifecycleListener onActivation, [%d] events in "
+ "window.", events.size()));
if (log.isDebugEnabled()) {
log.debug(String.format("invoking windowLifecycleListener onActivation, [%d] events in "
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

@@ -216,7 +220,9 @@ private void track(Event<T> windowEvent) {
lock.unlock();
}
eventsSinceLastExpiry.set(0);
log.debug(String.format("[%d] events expired from window.", eventsToExpire.size()));
if (log.isDebugEnabled()) {
log.debug(String.format("[%d] events expired from window.", eventsToExpire.size()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

@liudezhi2098
Copy link
Contributor Author

/pulsarbot run-failure-checks

1 similar comment
@liudezhi2098
Copy link
Contributor Author

/pulsarbot run-failure-checks

@Anonymitaet Anonymitaet added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Dec 27, 2021
@codelipenghui codelipenghui added this to the 2.10.0 milestone Dec 27, 2021
@codelipenghui codelipenghui merged commit fb4e2c8 into apache:master Dec 27, 2021
codelipenghui pushed a commit that referenced this pull request Dec 30, 2021
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.9.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants