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

nsqd: touch handling #407

Merged
merged 2 commits into from
Jul 18, 2014
Merged

nsqd: touch handling #407

merged 2 commits into from
Jul 18, 2014

Conversation

jehiah
Copy link
Member

@jehiah jehiah commented Jul 17, 2014

when you TOUCH a message, the logic that checks bounds on the timestamp adds the msgtimeout twice when doing the check which it shouldn't. In addition it seems that the definition of touch "reset the msg timeout" would imply it's based on Now() just like it originally is, not that it extends the previous timeout.

cc: @mreiferson

@jehiah
Copy link
Member Author

jehiah commented Jul 17, 2014

also, seems like we should figure out a way that TOUCH uses the client timeout as modified by identify instead of the nsqd configured timeout.

@mreiferson
Copy link
Member

We should pass in the client's timeout where it is called in protocol_v2.go

Good catch...

@mreiferson mreiferson added the bug label Jul 17, 2014
@mreiferson
Copy link
Member

did the latter in mreiferson/nsq@5f0bcb2, can't push to your repo so cherry-pick that in

@mreiferson
Copy link
Member

👍

mreiferson added a commit that referenced this pull request Jul 18, 2014
@mreiferson mreiferson merged commit c8ddec6 into nsqio:master Jul 18, 2014
@jehiah jehiah deleted the touch_logic_407 branch February 6, 2015 19:02
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.

2 participants