async per op #42

Closed
edwardcapriolo opened this Issue Jan 2, 2013 · 35 comments

Comments

Projects
None yet
5 participants
Collaborator

edwardcapriolo commented Jan 2, 2013

There is a case that since we are async we can deliver the result of each op as they are generated rather then blocking until the entire request is done. (This is a mid-future type issue)

Collaborator

edwardcapriolo commented Feb 8, 2013

In the grand scheme of things IntraVert is async. Having async per op is not required at this phase. This might be more appropriate if we decide to farm out events to multiple IV servers but for now close

zznate reopened this Feb 9, 2013

Owner

zznate commented Feb 9, 2013

Marking this as re-opened and enhancement. We should keep it up for discussion.

Keep in mind that pretty much everything below StorageProxy is async. SP marshalls the waits based on the CL and ring health. Therefore correctly addressing this will require overriding some of storageProxy's methods to do correctly.

To see the power of this, as a thought exercise lets look at the case of SP#mutate. Think about the ramifications of sending back the List of AbstractWriteResponseHandler verbs as the argument to a vert.x MutationHandler or similar. This would push the async power of cassandra directly into the vert.x handler flow.

This is sort of the pipe dream i've had all along, but doing this correctly needs to be slow and iterative or else we will end up with a hot mess of difficult to maintain code that is more or less copy-paste from SP. I don't want that.

Collaborator

jsanda commented Feb 18, 2013

I have been working on a prototype over the weekend that I would love for you guys to check out. I pushed my changes to a branch at https://github.com/zznate/intravert-ug/tree/async-requests. Here is a high level overview of what I did. Previously, when IntraHandlerJson received an http request, the operations specified in the body of the request were executed synchronously in the event loop. Now when an http request is received, the only thing that is done is to send a message on the event bus to a handler that will process the operations specified in the request body. A separate message is sent on the event bus to OperationsRequestHandler for each operation. Once all of the operations have been executed, the operation results are written out in the http response, ending the http request.

You will see that my changes make heavy use of the classes in the org.vertx.java.core.json package. In master we convert requests and responses to IntraReq and IntraRes respectively. I stick with using JSON for the most part; consequently, there are lots of places where string constants are used to read/write properties for the request and/or response. A nice enhancement would be to basically introduce versions of IntraReq and IntraRes that extend JsonObject.

I have only added support for a handful of operations so far. From the outset one of my primary objectives was to preserve behavior. The implementation for each operation that I have ported is unchanged except for dealing with JsonObject. I have added tests in RawJsonITest to test my changes. Since one of my objectives is to preserve behavior, I added a system property that can be set to determine whether to process requests using the new async handlers I added or the existing impl in master. By default RawJsonITest will run tests with the behavior as it exists in master with requests being process synchronously. Run RawJsonITest with the system property async-requests-enabled set to true and the new async handlers will be used instead. The tests that I have enabled at the moment pass in both cases which means so far I am meeting my goal of preserving existing behavior.

There are lots of other things that are ripe for discussion with respect to my changes, but I will hold off until everyone has had a chance to review what I have done.

Collaborator

edwardcapriolo commented Feb 18, 2013

Ok so I read that one of the new features is vertx where the bus workers can a local in process thread pool. So if we are doing a request by making each part async there are major things we have to deal with.

  1. Currently the requests are ordered impliictly, req1depends on req0. Doing each step async changes the load profile and now we need to explicitly
  2. performance. There is an IPerfTest class in the project. what is the cpu /memory cost of all these json object conversions? We need a multistep request that we can run locally and determine what the overall performance profile looks like.

I will review the part more deeply.

Collaborator

edwardcapriolo commented Feb 18, 2013

Did another pass on the code. I do like it. One thing it gives us is that it allows us to solve this issue of the growing IntraOp in a vertx friendly way.

  1. I do not think it makes a ton of sense to hand the IntraReq to the event bus. Some ops depend on other operations. I do think it makes sense to have the operations on their own even bus.

  2. State currently we are counting on a "concurrent" state to deliver us ID's

I am 90% sure IntraVert back end should look more like your branch then it looks like its current version. My biggest blocker with this is we are trying to hit a 1.0 version and while it seems like we can make this change it may backlog on on things like adding counters, types like timeuuid and other items we need to have a 1.0.

But lets try to iperftest the throughput. WE get big wins from going async and from using the handler interface (we can farm out requests), but I think at this point we would be better off defining an InvtraVert 2.0 doc and backlogging drastic changes, while we put the finishing touches on 1.0.

Collaborator

jsanda commented Feb 18, 2013

The order of operations is preserved. The way OperationsHandler works is
that it goes through each operation making a call to the operation handler
via the event bus. OperationsHandler specifies itself as the reply callback
so that it knows when to invoke the next operation. I also realize that
there may times where we want some set of operations to be executed
synchronously or even allow for some operations to be executed in parallel.
I think these are things that could be easily supported in several ways.
For instance, if we always want Operation X and Y to executed synchronously
the operation handlers could provide meta data possibly to express this.
And maybe the client includes info in the request that specifies some set
of the operations can be executed in parallel and we can handle that as
well.

I am not sure the cost of the doing the object conversions. I do think this
is something we should look at closely. Even if this approach for async
request handling does not bear out, I presume that as Intravert evolves, we
will start leveraging the event bus in one way or another. Other than
primitive types JsonObject and Buffer are the two supported message types.
This is why I think it might be worth considering having classes like
IntraReq and IntraRes extend JsonObject.

Maybe this could go into master post 1.0. I could keep it up to date with
master, merging and running tests regularly. I will start looking at some
of the perf tests.

On Mon, Feb 18, 2013 at 9:34 AM, edwardcapriolo notifications@github.comwrote:

Did another pass on the code. I do like it. One thing it gives us is that
it allows us to solve this issue of the growing IntraOp in a vertx friendly
way.

  1. I do not think it makes a ton of sense to hand the IntraReq to the
    event bus. Some ops depend on other operations. I do think it makes sense
    to have the operations on their own even bus.

  2. State currently we are counting on a "concurrent" state to deliver us
    ID's

I am 90% sure IntraVert back end should look more like your branch then it
looks like its current version. My biggest blocker with this is we are
trying to hit a 1.0 version and while it seems like we can make this change
it may backlog on on things like adding counters, types like timeuuid and
other items we need to have a 1.0.

But lets try to iperftest the throughput. WE get big wins from going async
and from using the handler interface (we can farm out requests), but I
think at this point we would be better off defining an InvtraVert 2.0 doc
and backlogging drastic changes, while we put the finishing touches on 1.0.


Reply to this email directly or view it on GitHubhttps://github.com/zznate/intravert-ug/issues/42#issuecomment-13724411.

  • John
Collaborator

edwardcapriolo commented Feb 18, 2013

Originally async-per-op meant: From OP:

There is a case that since we are async we can deliver the result of each op as they are generated rather then blocking until the entire request is done. (This is a mid-future type issue)

So the concept was that we can return the results op per op. This is not always valid actually (since a service processor could change or clear a result) but mostly it is valid.

If we do async makes more sense to me if we can order the operations in the request explicitly on each other. Then there is a big win, like:
a
b requires a
c requires a
d requrires b

Technically we can start doing D when B is done. Things like that. This is why I say we are better off looking at this in the 2.0 time frame, technically doing it may not be difficult but the dependency language and how the tasks gets executed is something we should layout for 2.0. For know , what you are doing is fine as a way to prove out the concept, which I am cool with.

Owner

zznate commented Feb 18, 2013

I was in the process of messing around with splitting out operations as well. I like the handler-per approach, but it does get us immediately into a bunch of hairy issues as you both mention above.

That said, I think it is the way we need to go - farming out ops to handlers lets us (eventually) get to a much greater modularity where we could do things like 'the Co-processor Handlers are in there own module so they can be updated independantly', etc. I.e. more ability to leverage the vert.x-y ness of the platform in a good way.

Collaborator

jsanda commented Feb 18, 2013

I really like the idea of streaming results to the client as they become
available, for those cases where it makes sense like you pointed out. And I
agree that figuring out ordering operations and expressing dependencies
will be rather involved. I think my next steps in the async-requests branch
will be to,

  • look at the perf tests
  • implement more operation handlers
  • look to minimize object conversions

On Mon, Feb 18, 2013 at 10:23 AM, edwardcapriolo
notifications@github.comwrote:

Originally async-per-op meant: From OP:

There is a case that since we are async we can deliver the result of each
op as they are generated rather then blocking until the entire request is
done. (This is a mid-future type issue)

So the concept was that we can return the results op per op. This is not
always valid actually (since a service processor could change or clear a
result) but mostly it is valid.

If we do async makes more sense to me if we can order the operations in
the request explicitly on each other. Then there is a big win, like:
a
b requires a
c requires a
d requrires b

Technically we can start doing D when B is done. Things like that. This is
why I say we are better off looking at this in the 2.0 time frame,
technically doing it may not be difficult but the dependency language and
how the tasks gets executed is something we should layout for 2.0. For know
, what you are doing is fine as a way to prove out the concept, which I am
cool with.


Reply to this email directly or view it on GitHubhttps://github.com/zznate/intravert-ug/issues/42#issuecomment-13726601.

  • John
Owner

zznate commented Feb 18, 2013

Thinking about this for another min, I'd actually like to move forward with this approach at the potential deterement to other issues (counters, types etc) as those are well-defined problems that don't necessarily depend on the underlying architecture. We can 'fix 'em later' in short, and focus on a solid integration.

Also, I think it's time to call in some air support on the JSON thing - Tatu is a good dude and is always happy to jump in when a project has good Jackson questions. Where should we have him look first? Best way to deal with IntraReq/Res?

Collaborator

jsanda commented Feb 18, 2013

There are a few things at play here. The body of the http request/response
is json. Then we have Jackson to map to/from IntraReq/Res. And then for
sending/receiving stuff on the event bus we have JsonObject. I'd love any
feedback/insight on how best to marry all them.

On Mon, Feb 18, 2013 at 10:48 AM, Nate McCall notifications@github.comwrote:

Thinking about this for another min, I'd actually like to move forward
with this approach at the potential deterement to other issues (counters,
types etc) as those are well-defined problems that don't necessarily depend
on the underlying architecture. We can 'fix 'em later' in short, and focus
on a solid integration.

Also, I think it's time to call in some air support on the JSON thing -
Tatu is a good dude and is always happy to jump in when a project has good
Jackson questions. Where should we have him look first? Best way to deal
with IntraReq/Res?


Reply to this email directly or view it on GitHubhttps://github.com/zznate/intravert-ug/issues/42#issuecomment-13727837.

  • John
Collaborator

edwardcapriolo commented Feb 18, 2013

Ok if we want to back off 1.0 for this....

Nate what is that link you were showing me where the EventBus can be done in local threads now. Is that something new in the new vertx? That would seem to be more efficient and we should look at that.

Right now the defacto standard is operations produce List we need to look at vertx's json object and possibly just doing our own serialization. We need something very efficient that can be moved between modules.

Remember c* has its own memory "issues" and if we introduce something very heavy weight it is going to be a deal breaker for many.

I am going to start a wiki page so we can track 2.0 stuff and put up our ideas and notes feel free to edit.

Owner

zznate commented Feb 18, 2013

@edwardcapriolo yes, latest vert.x threading is in master (marked as 2.0).

Owner

zznate commented Feb 18, 2013

@cowtowncoder would love your $0.02 on the best JSON serialization integration to:

  • minimize CPU/garbage within general cassandra churn
  • play nice with vert.x wire format (simple wrapper around ByteBuffer)
  • all this from the perspective of minimizing latency from async event handlers both over the wire (via event bus) and back out to client (via HTTP)

Ok, I'll starts with some random thoughts...

  1. Jackson does not do ByteBuffers, since access is generally slower than from plain byte arrays. Same is true for other json libs I am aware of. So currently one would copy things between byte arrays and `ByteBuffer's. Not a big deal, just something to be aware of.
  2. ... however, writing a JsonParser (or JsonGenerator) that operates on ByteBuffers would not be difficult, just some grunt work to create a mostly-cut-n-paste implementation.
  3. For streaming processing, in async context, easiest way is to do framing by using length-indicator, buffer up to each message, parse using blocking parser (since all content is buffered there will not be any blocking). Writing non-blocking parser is quite a bit of work: I wrote Aalto for XML, which is fully async Stax parser (with extension to feed content), but way fewer people really really use non-blocking than who think they would want to.
  4. Performance-wise, Smile format is a nice binary JSON variant (Jackson module at https://github.com/FasterXML/jackson-dataformat-smile) -- 100% JSON compatible at logical model level, faster to read and write (some binary formats are only faster to read), auto-detectable. So for internal system integration it works really nicely. I use it for storage system I am working on at work (which is mucg like Voldemort in many ways).

Without having much context I don't know how helpful above is, but what I can say is that I rarely see JSON decoding show up as significant CPU usage component. And since it's CPU bound, it should be relatively "nice" latency component, not very variable. Some JSON libs have higher garbage production rates; Jackson is reasonably lean especially as it recycles all of its internal buffers and handlers.

Collaborator

jsanda commented Feb 19, 2013

If we want to consider a binary format, what about http://msgpack.org?

kutchar commented Feb 19, 2013

+1 on JSON with SMILE encoding. I tried msgpack and I hit a few issues with it, mainly with typing (http://jira.msgpack.org/browse/MSGPACK-66). I've been using SMILE happily for a year now. Also, msgpack java impl is not the best java code I've seen vs Jackson code which is at a much higher quality. The nice plus with SMILE with Jackson is that you can easily interchange it with regular JSON, which is nice when debugging. The downside is that I'm not really aware of SMILE implementation in any other languages, so not sure if supporting clients in different languages is on the roadmap. Although it should be possible to allow other clients simply use JSON.

Here's a good presentation on the topic from eBay: http://qconsf.com/dl/qcon-sanfran-2011/slides/SastryMalladi_DealingWithPerformanceChallengesOptimizedSerializationTechniques.pdf

Collaborator

edwardcapriolo commented Feb 19, 2013

Smile is already one of the transports

Wrt Smile -- lack of non-Java implementations is something I would love to resolve.
But there is this project:

https://github.com/pierre/libsmile

which produces relatively complete C library, and its author said he has Ruby bindings for it as well.
Project could definitely use help (I helped a bit with test data and so on); if so, it should be viable building block.
Another thing that should be easy enough to do is C# version; I am not aware of one but conversion from Java would seem simple. Ideally tho it would expose "Dotnet-friendly" interface of some ind.

Finally, if it's of interest, I did some preliminary work on writing non-blocking (async) parser for Smile. While it is not complete, it is something that could be completed relatively easily, probably with less work than non-blocking JSON parser.

Collaborator

edwardcapriolo commented Feb 19, 2013

I am not sure we need a non-blocking parser. Our requests and responses are very small. I think the biggest question here is that VertX has it's own JSONObject classes. Yet we are using jackson to serialize req/res objects.

It might make more sense to use VertX's Json object all the way though, but then we lose jacksons free object mapping.

Understood, if length-prefix (or other kinds of external framing) is used, then non-blocking parsing isn't of much benefit. Plus data binding of non-blocking readers does not exist either.

Use of internal JSON nodes is often additional extra overhead without tons of benefits, at least if using Map/List based storage, which uses 2x-3x memory of equivalent POJOs (and ~50% more processing time). It is possible to do better, if set of keys is limited, or if random key access is not needed (i.e. can just do sequences of key value pairs).

If custom representation is wanted it should still be easy enough to add serializers/deserializers to simplify use of Jackson. There exists "json org" module (https://github.com/FasterXML/jackson-datatype-json-org), for example, which allows use of Jackson parser, generator, for reading and writing node objects from reference implementation (http://json.org/java). This would work equally well for JSON and Smile; as well as (if useful) other types Jackson supports via modules (bson, csv, xml, yaml even avro)

Collaborator

edwardcapriolo commented Feb 19, 2013

So I did some benchmarking, https://github.com/zznate/intravert-ug/blob/master/src/test/java/org/usergrid/vx/experimental/PerfTestingITest.java . and as you would expect, we can do like 1000000000000000000000 mapper calls in a second but surprisingly network sockets are slower, and cassandra even slower yet :)

Mostly I am worried about young gen server side. In the current design we are doing the entire request sync, if we make this fundamental shift to go async for ever Op in the request we are going to be creating many more short lived json objects. Ths is the choice we need to understand.

Yup. :)

One more thing that may or may not be helpful: since serialization as bytes (whether as byte[] or ByteBuffer) is typically more compact than source data / POJO, and low maintenance (single object), it is often good to try to serialize things as early as possible. With JSON (and Smile) one can just concatenate such content iff entities are part of root-level sequence. This can help reduce GC pressure. And if such queuing wasn't already simple or efficient, I wrote https://github.com/cowtowncoder/low-gc-membuffers for off-heap queuing (of stuff sent from front-end hosts to log aggregation thingy).

So it may not be bad idea to do serialization / deserialization in smaller units where possible. This can both reduce latency and reduce life-time of many intermediate objects.

Collaborator

jsanda commented Feb 20, 2013

Last night I ran the perf tests on the async-requests branch. The good news
is that running with the async handlers nothing broke. Things are a bit
slower though. Running the big perf test that does 2500 ops takes about 7.1
sec on my box when using the sync request processing. When using async
request processing, it was taking around 9.2, 9.3 sec. After talking with
@edrwardcapriolo about the local thread pools coming in vertx, I am eager
to run against a version that has enhancement to see if there is any
appreciable difference. For now though I am going to avoid anything other
than minor refactoring aorund the request/response objects in the branch. I
want to continue focusing on implementing async handlers for the
operations, making sure existing tests pass, and increasing test coverage.

On Tue, Feb 19, 2013 at 4:01 PM, Tatu Saloranta notifications@github.comwrote:

Yup. :)

One more thing that may or may not be helpful: since serialization as
bytes (whether as byte[] or ByteBuffer) is typically more compact than
source data / POJO, and low maintenance (single object), it is often good
to try to serialize things as early as possible. With JSON (and Smile) one
can just concatenate such content iff entities are part of root-level
sequence. This can help reduce GC pressure. And if such queuing wasn't
already simple or efficient, I wrote
https://github.com/cowtowncoder/low-gc-membuffers for off-heap queuing
(of stuff sent from front-end hosts to log aggregation thingy).

So it may not be bad idea to do serialization / deserialization in smaller
units where possible. This can both reduce latency and reduce life-time of
many intermediate objects.


Reply to this email directly or view it on GitHubhttps://github.com/zznate/intravert-ug/issues/42#issuecomment-13798995.

  • John
Collaborator

edwardcapriolo commented Feb 21, 2013

I am ok with the extra time. Those tests are not indicative of real load anyway. I think we get a lot our of your refactor. Namely I am getting tired or working on that ever growing java files. This chance splits up the code and will make the development process more sane. John when your ready let us know we can review the branch for a day and then commit it in.

Collaborator

jsanda commented Feb 21, 2013

I need to merge changes from master. Was going to do it tonight but hit
merge conflicts that I will wait to resolve until tomorrow. I want to
add/update a few more tests and then should be good to go. I hope to be
ready for review on Friday.

On Wed, Feb 20, 2013 at 10:48 PM, edwardcapriolo
notifications@github.comwrote:

I am ok with the extra time. Those tests are not indicative of real load
anyway. I think we get a lot our of your refactor. Namely I am getting
tired or working on that ever growing java files. This chance splits up the
code and will make the development process more sane. John when your ready
let us know we can review the branch for a day and then commit it in.


Reply to this email directly or view it on GitHubhttps://github.com/zznate/intravert-ug/issues/42#issuecomment-13871598.

  • John
Collaborator

edwardcapriolo commented Feb 21, 2013

Ok let's avoid changes to master then until this gets merged in

On Thursday, February 21, 2013, jsanda notifications@github.com wrote:

I need to merge changes from master. Was going to do it tonight but hit
merge conflicts that I will wait to resolve until tomorrow. I want to
add/update a few more tests and then should be good to go. I hope to be
ready for review on Friday.

On Wed, Feb 20, 2013 at 10:48 PM, edwardcapriolo
notifications@github.comwrote:

I am ok with the extra time. Those tests are not indicative of real load
anyway. I think we get a lot our of your refactor. Namely I am getting
tired or working on that ever growing java files. This chance splits up
the
code and will make the development process more sane. John when your
ready
let us know we can review the branch for a day and then commit it in.


Reply to this email directly or view it on GitHub<
https://github.com/zznate/intravert-ug/issues/42#issuecomment-13871598>.

  • John


Reply to this email directly or view it on GitHub.

Collaborator

edwardcapriolo commented Feb 22, 2013

So I was thinking of this.

req.add( Operations.setKeyspaceOp("myks") ); //0
req.add( Operations.setColumnFamilyOp("mycf") ); //1

implicit 1 depends on 0

req.add( Operations.explicitDependency(true) );
req.add( Operations.setKeyspaceOp("myks")
.set("id","a")
.set("depends", "" );
req.add( Operations.setColumnFamilyOp("mycf")
.set("id", "b")
.set("depends","a") );

explicit b depends on a
algorithm find all nodes that depends on "".

These ALL are scheduled for execution async and removed from the op list. When an object returns from async. Thread schedules for async anything that depends on the id of the completed.

The other thing I was thinking was a feature called "EARLY RETURNS" Essentially why not return the results from ops as they are finishing rather then waiting till the entire request is done.

I think the code should not be one way or the other. We should be able to support all permutations

ops in order/ early returns
ops in order / full request

ops async with dependencies/ early returns
ops async with deps / full request

What do you think?

Owner

zznate commented Feb 22, 2013

I do like this idea - implicit reference to a specific operation. (I brought this up a while ago and I think you poo-poo'ed it).

I agree with this approach facilitating distribution (locally or remote via evenbus maybe).

As for early returns - how would this work? Streaming back in order of completion? What about to register webhooks or notification via websocket?

Collaborator

edwardcapriolo commented Feb 22, 2013

As for early returns - how would this work? Streaming back in order of completion? What about to register webhooks or notification via websocket?

I was thinking a request calls reply N times before calling end. Since we are doing all the ops async, there we just reply as done. This does not fit into the mold of the current ICClient.sendBlocking() we would need some other client interface I guess.

Collaborator

jsanda commented Feb 22, 2013

I like the idea as well. In some situations though I can see where it
probably makes sense to impose an order. For example, it wouldn't make
sense to execute a slice and a filter in parallel where the filter is
supposed to be applied to the slice results. And there might be other cases
we can reasonably assume certain operations can be executed in parallel.

I'm not sure about webhooks or websockets, but with streaming we need to be
care not to have results overwrite one another. One approach might be to
treat writing the result of an operation back to the client as an atomic
operation at least with respect to other operation. So if we have results A
and B to write back in the response, either all of A or none if it is
written before B and vice versa.

On Thu, Feb 21, 2013 at 8:43 PM, Nate McCall notifications@github.comwrote:

I do like this idea - implicit reference to a specific operation. (I
brought this up a while ago and I think you poo-poo'ed it).

I agree with this approach facilitating distribution (locally or remote
via evenbus maybe).

As for early returns - how would this work? Streaming back in order of
completion? What about to register webhooks or notification via websocket?


Reply to this email directly or view it on GitHubhttps://github.com/zznate/intravert-ug/issues/42#issuecomment-13924209.

  • John
Collaborator

edwardcapriolo commented Feb 22, 2013

I am pretty sure vertx handles the cases multiplexing for us.

Collaborator

jsanda commented Feb 23, 2013

Yesterday I merged master into the async-requests branch. I ran tests after
resolving some conflicts and everything looked good except for
IntraServiceITest.scannerTest, but that is failing for me in master as
well. At this point I am ready for review so we can look to merge into
master. There are still several operation handlers that need to be
implemented but that can be done post merge. And the request/response
mapping stuff needs to get sorted out. Those are the two biggest things
that I can think of.

On Thu, Feb 21, 2013 at 9:00 PM, edwardcapriolo notifications@github.comwrote:

I am pretty sure vertx handles the cases multiplexing for us.


Reply to this email directly or view it on GitHubhttps://github.com/zznate/intravert-ug/issues/42#issuecomment-13924711.

  • John
Owner

zznate commented Feb 24, 2013

Currently in transit out to PDX - I'll look at this tonight (if Ed doesnt beat me to it). Thanks a bunch!

I'm happy to help fill in the missing handlers as well. Let's create another issue(s) for the remaining ones - one per if you think they will be tricky - primarily so we can parallelize work and not butt heads. Or if you get time to just bang 'em out, go for it.

zznate closed this May 13, 2014

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