Skip to content

KAFKA-7532: Fix controller log for list of shutting down brokers#5831

Merged
ijuma merged 1 commit intoapache:trunkfrom
stanislavkozlovski:KAFKA-7532
Oct 26, 2018
Merged

KAFKA-7532: Fix controller log for list of shutting down brokers#5831
ijuma merged 1 commit intoapache:trunkfrom
stanislavkozlovski:KAFKA-7532

Conversation

@stanislavkozlovski
Copy link
Copy Markdown
Contributor

@stanislavkozlovski stanislavkozlovski commented Oct 23, 2018

As reported in KAFKA-7532 - this line prints out

[2018-10-23 12:19:59,977] INFO [Controller id=0] Removed ArrayBuffer() from list of shutting down brokers. (kafka.controller.KafkaController)

This change seems to be present in all versions even as back as 0.9.0. I think it might be worth it to merge to 1.0 (pre-2.0 uses "".format())

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

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Oct 24, 2018

Retest this please.

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Oct 24, 2018

Will merge to trunk. This is a cosmetic fix so I don't think we need to backport it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should probably only log this if the list is not empty.

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Oct 24, 2018

If we avoid the log when the seq is empty, then maybe it's worth cherry-picking to 2.1 and 2.0 since it provides more value that way.

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

@ijuma ijuma merged commit eda4a29 into apache:trunk Oct 26, 2018
ijuma pushed a commit that referenced this pull request Oct 26, 2018
This line prints out (when empty):

```
[2018-10-23 12:19:59,977] INFO [Controller id=0] Removed ArrayBuffer() from list of shutting down brokers. (kafka.controller.KafkaController)
```

Use `mkString` to eliminate `ArrayBuffer` and only log if not empty.

Reviewers: Ismael Juma <ismael@juma.me.uk>
ijuma pushed a commit that referenced this pull request Oct 26, 2018
This line prints out (when empty):

```
[2018-10-23 12:19:59,977] INFO [Controller id=0] Removed ArrayBuffer() from list of shutting down brokers. (kafka.controller.KafkaController)
```

Use `mkString` to eliminate `ArrayBuffer` and only log if not empty.

Reviewers: Ismael Juma <ismael@juma.me.uk>
@ijuma
Copy link
Copy Markdown
Member

ijuma commented Oct 26, 2018

Merged to trunk, 2.1 and 2.0.

pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
…e#5831)

This line prints out (when empty):

```
[2018-10-23 12:19:59,977] INFO [Controller id=0] Removed ArrayBuffer() from list of shutting down brokers. (kafka.controller.KafkaController)
```

Use `mkString` to eliminate `ArrayBuffer` and only log if not empty.

Reviewers: Ismael Juma <ismael@juma.me.uk>
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