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

make notify return error code on timeout (#9193) #2302

Closed
wants to merge 5 commits into from

Conversation

Projects
None yet
5 participants
@liewegas
Copy link
Member

commented Aug 21, 2014

No description provided.

@athanatos

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2014

Red on gitbuilder.

liewegas added some commits Aug 11, 2014

messages/MWatchNotify: include an error code in the message
Document the fields, while we are here.

Signed-off-by: Sage Weil <sage@redhat.com>
osd: include ETIMEDOUT in notify reply on timeout
If a notify operation times out (all watchers to not ACK in time), include
an ETIMEDOUT in the final error message back to the client, so that they
know about it.

Signed-off-by: Sage Weil <sage@redhat.com>
librados: rename watch/notify callback register functions
Make it clear these are for watch OR notify, not just watch.  I was
confused.

Signed-off-by: Sage Weil <sage@redhat.com>
librados: prefix msg handler with handle_
Signed-off-by: Sage Weil <sage@redhat.com>
librados: refactor watch/notify; return notify error code
Get rid of a level of intermediate classes with confusing names and put
the notify and notify finish logic in a single place so that it is easier
to follow and understand.

Pass the return value from the notify completion message to the caller.

Fixes: #9193
Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas force-pushed the wip-9193 branch from 0a749f9 to bf40cf1 Aug 29, 2014

@idryomov

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2014

Looks like this was merged a while ago?

@jdurgin

This comment has been minimized.

Copy link
Member

commented Sep 8, 2014

Yeah, looks like it was already merged.

@jdurgin jdurgin closed this Sep 8, 2014

@jdurgin jdurgin deleted the wip-9193 branch Sep 8, 2014

@liewegas liewegas restored the wip-9193 branch Sep 9, 2014

@dillaman

This comment has been minimized.

Copy link
Contributor

commented on src/messages/MWatchNotify.h in 7c7bf5f Sep 24, 2014

Do you need to bump the header version to 2?

This comment has been minimized.

Copy link
Member Author

replied Sep 25, 2014

The encoding version in the header is set to 2. This version is a bit goofy and I'm not sure it is used for anything useful. :/ Would need to check to see if anything actually looks at it before changing...

@liewegas liewegas deleted the wip-9193 branch Nov 23, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.