basho / riak-dotnet-client

The Riak client for .NET
Apache License 2.0
76 stars 35 forks source link

Non-Blocking Asynchronous IO for the client #116

Open chrisleow opened 11 years ago

chrisleow commented 11 years ago

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:

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 commented 11 years ago

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/DistributedNonsense/CorrugatedIron/issues/116 .

OJ commented 11 years ago

@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 commented 11 years ago

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 commented 11 years ago

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/DistributedNonsense/CorrugatedIron/issues/116#issuecomment-17182495 .

chrisleow commented 11 years ago

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 commented 11 years ago

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!

peschkaj commented 11 years ago

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.

Entroper commented 10 years ago

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

peschkaj commented 10 years ago

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/DistributedNonsense/CorrugatedIron/issues/116#issuecomment-39282007 .

taliesins commented 10 years ago

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 commented 10 years ago

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.