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

Non-Blocking Asynchronous IO for the client #116

Open
chrisleow opened this issue Apr 26, 2013 · 11 comments
Open

Non-Blocking Asynchronous IO for the client #116

chrisleow opened this issue Apr 26, 2013 · 11 comments

Comments

@chrisleow
Copy link

Am using Riak and CorrugatedIron for a new project, had a dig around in the code base and have discovered that the API is using blocking IO. I would like to change this into non-blocking IO by using the .NET 4.0 TPL library.

This would involve the following:

  • RiakPbcSocket needs to be non-blocking and asynchronous
  • IRiakConnection needs to have all return types as Task<.....>
  • IRiakConnection needs to have all return types as Task<.....>
  • IRiakEndPoint and RiakExternalLoadBalancer need to implement async functions.

The end result is that then RiakClient becomes a small wrapper of RiakAsyncClient, as opposed to RiakAsyncClient being a wrapper around RiakClient, and CorrugatedIron is then a fully non-blocking Riak client that can handle 1000's of simultaneous requests (if your cluster is that big).

All without changing the API.

I don't need this functionality right away, but may well fork / pull / something (open-source committer newbie, not sure what the right term is) in a few weeks to a month, maybe longer.

Is this something that people want to see? Have I left anything out? Are there any other plans to do this that I'm not aware of? Any other thoughts?

@peschkaj
Copy link
Contributor

There aren't any concrete plans to add this, but it's something that we've
been considering. Time has been what's stopping us, more than anything
else. If you've got something in the works, feel free to get going on it.
I'm not sure what our availability looks like in the short term. We'll keep
updating this issue if we start working on something, just so you get
notified.


Jeremiah Peschka - Founder, Brent Ozar Unlimited
MCITP: SQL Server 2008, MVP
Cloudera Certified Developer for Apache Hadoop

On Fri, Apr 26, 2013 at 7:04 AM, chrisleow notifications@github.com wrote:

Am using Riak and CorrugatedIron for a new project, had a dig around in
the code base and have discovered that the API is using blocking IO. I
would like to change this into non-blocking IO by using the .NET 4.0 TPL
library.

This would involve the following:

  • RiakPbcSocket needs to be non-blocking and asynchronous
  • IRiakConnection needs to have all return types as Task<.....>
  • IRiakConnection needs to have all return types as Task<.....>
  • IRiakEndPoint and RiakExternalLoadBalancer need to implement async
    functions.

The end result is that then RiakClient becomes a small wrapper of
RiakAsyncClient, as opposed to RiakAsyncClient being a wrapper around
RiakClient, and CorrugatedIron is then a fully non-blocking Riak client
that can handle 1000's of simultaneous requests (if your cluster is that
big).

All without changing the API.

I don't need this functionality right away, but may well fork / pull /
something (open-source committer newbie, not sure what the right term is)
in a few weeks to a month, maybe longer.

Is this something that people want to see? Have I left anything out? Are
there any other plans to do this that I'm not aware of? Any other thoughts?


Reply to this email directly or view it on GitHubhttps://github.com//issues/116
.

@OJ
Copy link
Contributor

OJ commented Apr 28, 2013

@chrisleow Just to back up what @peschkaj has said, this is something that we've talked about for a while but haven't yet go around to implementing. It's definitely something that we want to do though and your point about inverting the Async <-> Sync interface dependency is spot on.

You're not the first to mention it too. A few others have reached out recently saying that this would be "nice to have". I think it might be time to move this to the top of the list.

Thanks for opening the discussion!

@chrisleow
Copy link
Author

Hi guys, decided to stop being a first-time caller, long-time listener in open-source, and made a start on converting the code. I've done most of the donkey work, so there's a few little bits I don't quite understand, and none of it's tested, but I've basically converted the bulk of it to asynchronous code.

I think it'll be quite easy to move the code over in two phases. It's still blocking code under the bonnet (IRiakConnection wraps the old blocking implementation), but if the new stuff can be tested and signed off to make sure all the drudge works OK, and then all you need to do is make RiakPbcSocket asynchronous, and your completely non-blocking!

@peschkaj
Copy link
Contributor

I saw the pull request come it. That's fantastic. I'll take a look at it
and run it through the paces to see how it fares. If nothing fails, we can
build a performance harness to help demonstrate the difference and help us
monitor changes over time.

Thanks a lot for contributing this.

I'll review and comment as time allows.


Jeremiah Peschka - Founder, Brent Ozar Unlimited
MCITP: SQL Server 2008, MVP
Cloudera Certified Developer for Apache Hadoop

On Mon, Apr 29, 2013 at 10:49 AM, chrisleow notifications@github.comwrote:

Hi guys, decided to stop being a first-time caller, long-time listener in
open-source, and made a start on converting the code. I've done most of the
donkey work, so there's a few little bits I don't quite understand, and
none of it's tested, but I've basically converted the bulk of it to
asynchronous code.

I think it'll be quite easy to move the code over in two phases. It's
still blocking code under the bonnet (IRiakConnection wraps the old
blocking implementation), but if the new stuff can be tested and signed off
to make sure all the drudge works OK, and then all you need to do is make
RiakPbcSocket asynchronous, and your completely non-blocking!


Reply to this email directly or view it on GitHubhttps://github.com//issues/116#issuecomment-17182495
.

@chrisleow
Copy link
Author

Hi Jeremiah,

This is the result of an all-day bender, so it almost certainly will fail! But shouldn't be too hard to debug, I've mostly done the grunt work, so it'll just be a few things out of place, and it should then work.

I haven't really changed to much, just wrapped all of your code in the TPL. Probably should have tidied it up a bit more before committing, but I guess you live and learn!

@chrisleow
Copy link
Author

Right, finished it all off, so batching is now implemented both synchronously and asynchronously, the socket, and connection are all fully asynchronous. I've done some basic testing of the important parts locally with a single node, but can't run the full integration test suite. Hopefully can get this tested in your test harness @peschkaj!

@ghost ghost assigned OJ Jul 19, 2013
@peschkaj
Copy link
Contributor

Pushing out to a later milestone so we can make sure the SocketAsyncEventArgs support in Mono 3.2 has a chance to get put through the paces before forcing a Mono version bump.

peschkaj referenced this issue in amirhalatzi/CorrugatedIron Sep 1, 2013
To accomodate for rapid fail scenarios, added ConnectTimeout
configuration value
@Entroper
Copy link

Entroper commented Apr 2, 2014

Is this still being worked on / in the queue to be worked on?

@peschkaj
Copy link
Contributor

peschkaj commented Apr 2, 2014

It's not in process. Our initial thought was to use SocketAsyncEventArgs,
but that's not currently supported on Mono -
https://bugzilla.xamarin.com/show_bug.cgi?id=12211


Jeremiah Peschka - Managing Director, Brent Ozar Unlimited
MCSE: Data Platform, MVP
Cloudera Certified Developer for Apache Hadoop

On Tue, Apr 1, 2014 at 7:05 PM, Entroper notifications@github.com wrote:

Is this still being worked on / in the queue to be worked on?

Reply to this email directly or view it on GitHubhttps://github.com//issues/116#issuecomment-39282007
.

@taliesins
Copy link

Hi guys

I have a fork where I have implemented:

I would really love to see this get into the official client. Performance and stability seems a lot better.

Regards
Taliesin

@peschkaj
Copy link
Contributor

As in #153 - could you send over a pull request? That makes it much easier to test your code and changes in isolation, as opposed to wiping commit history via glorious copypasta.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants