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

Allow message payload to be NULL. #200

Closed
wants to merge 1 commit into from

Conversation

secretmike
Copy link
Contributor

@secretmike secretmike commented Feb 16, 2015

NULL message payloads are used with compaction topics introduced in 0.8.1
which use the NULL payload to indicate the corresponding key should be deleted.

(see Log Compaction for more details)

The Kafka protocol guide also notes that the Value (payload) may be NULL.

This commit makes these changes:

  • prevents NULL payloads from being copied when RD_KAFKA_MSG_F_COPY is set.
  • prevents payload bytes being copied into rkbuf when NULL.
  • sets rkm_len to -1 when payload is NULL (just like key).
  • Allows NULL payloads to be received in a fetch.

This is my first change to this code so I'm sure there are things missed. If you can point me in the right direction I'd be happy make any changes, or if you'd rather clean things yourself that works for me too :)

Thanks!

@edenhill
Copy link
Contributor

Thanks for the PR, this is a good fix!

NULL message payloads are used with compaction topics introduced in 0.8.1
which use the NULL payload to indicate the corresponding key should be deleted.

The Kafka protocol guide also notes that the Value (payload) may be NULL.
https://cwiki.apache.org/confluence/display/KAFKA/A+Guide+To+The+Kafka+Protocol#AGuideToTheKafkaProtocol-Messagesets

This commit makes these changes:

- prevents NULL payloads from being copied when RD_KAFKA_MSG_F_COPY is set.
- prevents payload bytes being copied into rkbuf when NULL.
- sets rkm_len to -1 when payload is NULL (just like key).
- Allows NULL payloads to be received in a fetch.
@edenhill
Copy link
Contributor

Null message support is now in master.
Thanks for your contribution!

@edenhill edenhill closed this Mar 20, 2015
@secretmike
Copy link
Contributor Author

Thank you!

@secretmike secretmike deleted the null_payloads branch March 20, 2015 20:03
@edenhill
Copy link
Contributor

Can you verify this works as intended for you?

@secretmike
Copy link
Contributor Author

I'll be trying it out, but probably not until next week.

On Fri, Mar 20, 2015 at 5:04 PM, Magnus Edenhill notifications@github.com
wrote:

Can you verify this works as intended for you?


Reply to this email directly or view it on GitHub
#200 (comment).

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

Successfully merging this pull request may close these issues.

2 participants