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

Producer/Consumer not respecting message/key null vs empty #36

Closed
mimaison opened this issue Sep 30, 2016 · 14 comments
Closed

Producer/Consumer not respecting message/key null vs empty #36

mimaison opened this issue Sep 30, 2016 · 14 comments

Comments

@mimaison
Copy link
Collaborator

When sending a message with an empty key (""), it is received as an undefined key by the Javascript consumer and a null key by the Java consumer. It should be received as an empty string.

Also currently, the producer doesn't allow sending messages with a null payload (can be done in Java). Similarly the consumer sees messages with a null payload (sent from Java) as empty.

@edoardocomar
Copy link
Collaborator

note that we tested the key received by the javascript consumer with the fix provided with issue #32

@webmakersteve
Copy link
Contributor

I'm a little confused with what you're saying. Are you saying that when producing an empty string key using the Java libraries, when consuming that message the key is null? If that is the case, I would like to try to be in line with how the java libs work pretty closely, and undefined makes sense in jsland where we have the ability to use it.

Producing a message with a null payload seems to be a separate issue. These null payloads come up as empty strings when consumed?

@edoardocomar
Copy link
Collaborator

@webmakersteve with the Java client, if you produce a message with null key or payload, then you will consume a message with a null key/payload. If you produce a message with an empty string as key or payload, you will consume a message with an empty string as key or payload.

In other words, the java client and the kafka wire protocol are able to distinguish between null and empty string.

we have noticed that the node-rdkafka producer will

  • not accept a null as payload
  • send a msg with a "" key as if it was null

and the consumer will turn a wire message with a null payoad into a javascript message with empty string as payload .

@webmakersteve
Copy link
Contributor

This requires a bit of refactoring, which may be happening soon in some performance tuning I am doing. Stay tuned! (pun intended 😛 )

@edoardocomar
Copy link
Collaborator

@webmakersteve thanks we can't wait 👍

@webmakersteve
Copy link
Contributor

#42 Is not in a mergeable state yet, but this does support sending an empty string a as a key to the producer as it does type checking on the variable rather than casting and then checking if it is empty.

Was able to do this by removing unnecessary intermediary classes.

It still does not accept null as a payload, but I can make it do that much more easily as an exceptional case. It always expects the parameter for message to be a buffer currently.

@webmakersteve
Copy link
Contributor

#42 merged. Feel free to use it by specifying the github URL for now. I want to ensure its stability before publishing it.

@mimaison
Copy link
Collaborator Author

I've gave this another look today.
For the Producer:

  • Payload: I was able to send a null, empty string and a string correctly
  • Key: an empty string is incorrectly sent as null

For the Consumer:

  • Payload: a null payload is received as an empty Buffer
  • Key: I was able to send a null, empty string and a string correctly

I used the Java 0.10.0.1 client for the verification.

@webmakersteve
Copy link
Contributor

How are you determining the empty string is incorrectly sent as null for the producer? Are you looking at the delivery reports?

If so, I probably just need to update the code there to follow the same pattern. Because, if the consumer is receiving a null key and empty string key correctly, it looks like the producer did its job properly.

I'll look into the other cases.

@mimaison
Copy link
Collaborator Author

I consumed the messages sent by the node-rdkafka producer using the Java client. Messages that had an empty string as the key were consumed as having null for the key. From Java to Java, if I send an empty string, I receive an empty string.

Same for the consumer, I sent messages using the Java client and consumed then using node-rdkafka consumer.

@webmakersteve
Copy link
Contributor

I believe commit de3d665 fixes the issue for the producer:

@webmakersteve
Copy link
Contributor

And... the next commit should fix the issue for the consumer. When you get a chance, if you could test it again using the master branch, it would be greatly appreciated :)

edoardocomar added a commit to edoardocomar/node-rdkafka that referenced this issue Jan 19, 2017
@mimaison
Copy link
Collaborator Author

Thanks !
As far as we can tell it's working correctly now. We were able to send and receive empty, null and normal keys and values.

We also manually tried interoperability with Java and everything was sent/received as expected !

@webmakersteve
Copy link
Contributor

Great to hear! This will be added in 0.7 which I will release early next week.

webmakersteve pushed a commit that referenced this issue Jan 19, 2017
* Test coverage for null or empty message value or key

This verifies the fix for #36

Developed with @mimaison

* Fixed missing clearInterval() calls

* Tweaked ordering
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

No branches or pull requests

3 participants