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

MPUB protocol spec #140

Merged
merged 1 commit into from
Jan 23, 2013
Merged

MPUB protocol spec #140

merged 1 commit into from
Jan 23, 2013

Conversation

mreiferson
Copy link
Member

Looks like the protocol spec for MPUB does not match the implementation. I suspect the implementation is the up to date one, (so you can have a max request size). But I wanted to double check.

-Dustin

@mreiferson
Copy link
Member

@dustismo thanks...

I just reviewed the protocol spec - where are you finding it differ from the implementation in master?

there were some changes to MPUB since the 0.2.16 release, specifically making it atomic in #135... want to make sure I didn't miss something...

@dustismo
Copy link
Contributor Author

My reading of the spec is that a message should be of the form (to publish 3 messages to topic 'topic1'):

MPUB topic1 3\n
[messageSize1][message1]
[messageSize2][message2]
[messageSize3][message3]

The implementation appears to be:

MPUB topic1\n
[totalBodySize]
[messageSize1][message1]
[messageSize2][message2]
[messageSize3][message3]

@dustismo
Copy link
Contributor Author

Actually implementation looks like:

MPUB topic1\n
[totalMessages][totalBodySize]
[messageSize1][message1]
[messageSize2][message2]
[messageSize3][message3]

@dustismo
Copy link
Contributor Author

If it helps, I updated my local fork to conform to (my reading of) the spec. dustismo@26baf6e

I can clean it up and issue a pull req if that implementation is correct.

@mreiferson
Copy link
Member

hah, nice... excellent find...

Your last comment is the closest, the correct implementation will be the existing one in nsqd in master:

MPUB topic\n
[4-byte body size]
[4-byte num messages]
[4-byte message size][N-byte message]
[4-byte message size][N-byte message]
[4-byte message size][N-byte message]
...

When I introduced MPUB I probably implemented it for consistency with other commands with bodies (the body is always size prefixed) and forgot to update the docs.

I will update the docs and open a pull request against this issue.

@mreiferson
Copy link
Member

let me know if that reads well for you and I'll merge... thanks again

@dustismo
Copy link
Contributor Author

Great, thanks!

jehiah added a commit that referenced this pull request Jan 23, 2013
@jehiah jehiah merged commit e5fb3d0 into nsqio:master Jan 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants