Skip to content

KAFKA-10407: Have KafkaLog4jAppender support linger.ms and batch.size#9189

Merged
huxihx merged 2 commits into
apache:trunkfrom
huxihx:KAFKA-10407
Aug 20, 2020
Merged

KAFKA-10407: Have KafkaLog4jAppender support linger.ms and batch.size#9189
huxihx merged 2 commits into
apache:trunkfrom
huxihx:KAFKA-10407

Conversation

@huxihx

@huxihx huxihx commented Aug 17, 2020

Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/KAFKA-10407

Currently, KafkaLog4jAppender does not support linger.ms or batch.size. In some situations, those two parameters are good to tune the performance.

More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Committer Checklist (excluded from commit message)

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

huxihx added 2 commits August 17, 2020 10:15
https://issues.apache.org/jira/browse/KAFKA-10407

Currently, KafkaLog4jAppender does not support `batch.size` or `linger.ms` which would otherwise be beneficial in some situations.
@huxihx
huxihx requested a review from omkreddy August 17, 2020 02:25
@huxihx huxihx changed the title Kafka 10407: Have KafkaLog4jAppender support linger.ms and batch.size KAFKA-10407: Have KafkaLog4jAppender support linger.ms and batch.size Aug 17, 2020
@dongjinleekr

Copy link
Copy Markdown
Contributor

Hi @huxihx,

It seems like this feature requires a KIP. Isn't it?

@huxihx

huxihx commented Aug 17, 2020

Copy link
Copy Markdown
Contributor Author

@dongjinleekr Thanks for the comments. I think it needs no KIP for this patch, since it does not break any compatibility. It's totally okay for you not to specify them when using KafkaLog4jAppender. Besides, deliveryTimeoutMs is added by #5425 without any KIP covered. Does it make sense?

@dongjinleekr

Copy link
Copy Markdown
Contributor

@huxihx

Besides, deliveryTimeoutMs is added by #5425 without any KIP covered.

Great! It perfectly makes sense.

@omkreddy omkreddy 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.

@huxihx Thanks for the PR. LGTM.

@huxihx

huxihx commented Aug 20, 2020

Copy link
Copy Markdown
Contributor Author

@omkreddy Thanks for the review, merging to trunk. @dongjinleekr Thanks for the comments again.

@huxihx
huxihx merged commit dc2b7d6 into apache:trunk Aug 20, 2020
@dongjinleekr

Copy link
Copy Markdown
Contributor

Congratulations, @huxihx! 😄

If you have some time, please have a look at the related issue: #7898

cc/ @omkreddy

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.

3 participants