Skip to content

[MINOR] Fix documentation of parameter "block.on.buffer.full"#954

Closed
mjsax wants to merge 1 commit into
apache:trunkfrom
mjsax:hotfix-docu
Closed

[MINOR] Fix documentation of parameter "block.on.buffer.full"#954
mjsax wants to merge 1 commit into
apache:trunkfrom
mjsax:hotfix-docu

Conversation

@mjsax
Copy link
Copy Markdown
Member

@mjsax mjsax commented Feb 23, 2016

default value is "false" and not "true"

See: https://stackoverflow.com/questions/35578519/kafka-block-on-buffer-full-default-value
and

.define(BLOCK_ON_BUFFER_FULL_CONFIG, Type.BOOLEAN, false, Importance.LOW, BLOCK_ON_BUFFER_FULL_DOC)

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.

The javadoc for this config says:

This config will be removed in a future release. Also, the {@link #METADATA_FETCH_TIMEOUT_CONFIG} is no longer honored when this property is set to true.

It may be worth including that in the actual text. Going forward, MAX_BLOCK_MS_CONFIG should be used instead.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Feb 23, 2016

@ijuma thanks for the review. I just updated this PR. I did not squash both commits on purpose (if you want me to squash them, just let me know.)

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Feb 23, 2016

LGTM cc @gwenshap

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.

Could use the parameter variable here. METADATA_FETCH_TIMEOUT_CONFIG

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Feb 23, 2016

Thanks for your comments @granthenke Unfortunately, I get an compile error I do not understand... (I commented your comment...)

Btw: I had a look through the whole file, and there are some more thing we could improve.

  1. use BATCH_SIZE_CONFIG instead of "batch.size" in LINGER_MS_DOC
  2. use LINGER_MS_CONFIG instead of "linger.ms" in LINGER_MS_DOC
  3. replace {@link KafkaProducer#send()} with <code>KafkaProducer.send()<code> in MAX_BLOCK_MS_DOC (JavaDoc markup does not work here...)
  4. replace {@link KafkaProducer#partitionsFor} with <code>KafkaProducer.partitionsFor()</code> in MAX_BLOCK_MS_DOC (JavaDoc markup does not work here...)

(There is also "block.on.buffer.full" in BUFFER_MEMORY_DOC which should actually be BLOCK_ON_BUFFER_FULL_CONFIG; however, this is deprecated and changing it gives the same error as above -- what does make sense it this case.)

What do you think about this? Should I include some of it in this PR?

…lt value is "false" and not "true"

- extended documentation according to reviewer comment.
- minor improvements of other parmeters:
  * use variables instead of hard coded parameter names
  * replace JavaDoc with proper HTML markup
@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Feb 24, 2016

@ijuma @granthenke Just updated this.

@ewencp
Copy link
Copy Markdown
Contributor

ewencp commented Mar 9, 2016

LGTM, thanks @mjsax!

@asfgit asfgit closed this in 6893091 Mar 9, 2016
@mjsax mjsax deleted the hotfix-docu branch April 2, 2016 12:22
efeg pushed a commit to efeg/kafka that referenced this pull request Jan 29, 2020
@mjsax mjsax changed the title [MINOR] Fixed documenation of parameter "block.on.buffer.full" [MINOR] Fix documentation of parameter "block.on.buffer.full" Jul 8, 2020
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.

4 participants