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.

Sign up for GitHub

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

Jump to bottom

call Mod:handle_overload_info/2 for unknown msgs in vnode_proxy during o... #605

Merged
merged 2 commits into from Jun 20, 2014
Merged

call Mod:handle_overload_info/2 for unknown msgs in vnode_proxy during o... #605

merged 2 commits into from Jun 20, 2014

Conversation

andrewjstone
Copy link
Contributor

...verload

@andrewjstone andrewjstone mentioned this pull request Jun 19, 2014
Merged
@andrewjstone andrewjstone added this to the 2.0-RC milestone Jun 19, 2014
@andrewjstone
Copy link
Contributor Author

Part of a fix for basho/riak_kv#986

@kellymclaughlin kellymclaughlin self-assigned this Jun 19, 2014
@kellymclaughlin
Copy link
Contributor

Seeing the following failures running make test locally:

module 'riak_core_vnode_proxy'
  riak_core_vnode_proxy: overload_test_ (should not discard in normal operation)...[0.154 s] ok
  riak_core_vnode_proxy: overload_test_ (should discard during overflow)...*timed out*
  undefined
riak_core_vnode_proxy: overload_test_ (should tolerate slow vnodes)...*timed out*
undefined

develop ran all tests successfully, but I only tried once and also try extending timeouts in that test.

@kellymclaughlin
Copy link
Contributor

Those test failures definitely seem to be caused by this change. The test cases that are timing out result in a bunch of messages like hello being sent as the message and handled by handle_overload. Previously the ok return made the test happy, but now the function call is hanging it up.

@andrewjstone
Stub out fakemod:handle_overload_info/2 for test
c79268f 
In riak_core_vnode_proxy:overload_test_/0 we use fakemod as a vnode
module because we don't expect any callbacks from the proxy, even in
overload. This prior commit however, ensures that the vnode will get
called back with any messages that aren't handled directly via the
proxy. Previously those messages were dropped on the floor.
@andrewjstone
Copy link
Contributor Author

I just pushed up a fix for that error @kellymclaughlin. Thanks for pointing it out.

@kellymclaughlin
Copy link
Contributor

  • Eunit and eqc
  • Dialyzer
  • xref
  • code inspection

@kellymclaughlin
Copy link
Contributor

Looks good.

👍

borshop added a commit that referenced this pull request Jun 20, 2014
@borshop
Merge pull request #605 from basho/bugfix/handle-sc-vnode-overload
aea8ed3 
call Mod:handle_overload_info/2 for unknown msgs in vnode_proxy during o...

Reviewed-by: kellymclaughlin
@andrewjstone
Copy link
Contributor Author

@borshop merge

@borshop borshop merged commit c79268f into develop Jun 20, 2014
@andrewjstone andrewjstone mentioned this pull request Jun 20, 2014
Closed
@kellymclaughlin kellymclaughlin deleted the bugfix/handle-sc-vnode-overload branch June 20, 2014 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@kellymclaughlin kellymclaughlin

Labels
None yet
Projects
None yet
Milestone
2.0-RC
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@andrewjstone @kellymclaughlin @borshop