Remove AsyncApi

[Adrian Cole]

Hi, team.

Implicitly, what is now master (1.7) will have changes in structure and versioning as we enter the apache incubator.  

We have a chance to clean things up that have been haunting for some time before we issue the proposal.  I recommend we literally remove the AsyncApi of jclouds.  

Here's why:

  * maintaining mirror interfaces is extra work that serves zero benefit when using the sync interface.

  * now that google app engine supports executorservice, users can launch background tasks without relying on AsyncApi.

  * our transform functions do not take advantage of AsyncApi at http level.

  * very often, AsyncBlobStore is implemented by a combination of sync and Async methods and could as easily been done with only an executorservice and a sync http binding.

  * code complexity in core is greatly increased, needing to process and validate multiple similar interfaces.  Stack traces are longer and hairier.

  * if we remove the AsyncApi, we no longer require an executorservice at the rest level, making the context object safer.

There are probably many other reasons.

Work implies migrating the annotations from the AsyncApis to the normal ones, then deleting them.  It also implies going around and fixing AsyncBlobStore and possibly modifying ContextBuilder.buildView to accept an executorservice, similar to guava Futures calls.

I'm offering to help with this as it makes code review, participation, and troubleshooting much easier in the long term.

Thoughts?

-A

[Andrew Phillips]

> Thoughts?

What's the plan for methods that internally make use of async actions,
e.g. in BlobMap? Would those also be removed or be re-written to use
an executor?

Also, what is the suggested migration plan for users who are currently
calling the async APIs in their code? Do we have an idea on the impact
of the change for them? Or are we reasonably confident that this API
isn't really in use much?

Thanks!

ap

[Adrian Cole]

On Thursday, April 4, 2013, Andrew Phillips wrote:

Thoughts?

What's the plan for methods that internally make use of async actions, e.g. in BlobMap? Would those also be removed or be re-written to use an executor?

 

.  

 IMHO Only BlobStore should be retained.  The others in blobstore carry far more technical debt and pitfalls than they are worth.  Wrt impl details of ComputeService or BlobStore, yes these can be rewritten to use ExecutorService

Also, what is the suggested migration plan for users who are currently calling the async APIs in their code? Do we have an idea on the impact of the change for them? Or are we reasonably confident that this API isn't really in use much.

I dont think these are used much outside impl details of AsyncBlobStore.  Suggested migration path is to show examples of how to convert to executorservice and maybe mention a few libraries far better at managing a bunch of parallel ops such as storm trident, rxjava, hystrix, clojure, etc.

 

Thanks!

ap

--
You received this message because you are subscribed to the Google Groups "jclouds-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jclouds-dev...@googlegroups.com.
To post to this group, send email to jclou...@googlegroups.com.
Visit this group at http://groups.google.com/group/jclouds-dev?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

[Everett Toews]

We haven't been using the async stuff.

I'm all for removing it, especially if this means we can eventually eliminate all of the proxies too. Those are definitely a barrier to people contributing to jclouds.

Cheers,

Everett

[Adrian Cole]

Here's a bit of irc scrollback on the topic, edited as I removed some irrelevant conversations regarding representation models from it.

adrianc_if you look at impl you'll notice what I mean

adrianc_some async commands are just futures.immediateFuture(sync)

adrianc_but wrt Async at api level (ex. s3AsyncApi)

adrianc_this was made for nonblocking io, which we never completed in 4 years

adrianc_and is literally implemented as submit(callableFor(synccommand))

adrianc_which is something anyone can do, including asyncBlobStore

adrianc_AsyncBlobStore is also a joke due to things like container clear

adrianc_which in impl are usually a series of synchronous commands in a for loop

adrianc_so, main point is to stop all the pretending and instead just retain value-added code that is easy to work with and troubleshoot

adrianc_which will help with the problem we have where hardly anyone touched core code

adrianc_root cause being it is too complex

adrianc_and in the case of Async, complex for no real benefit

gaulwell we already agree clear container is an odd bird

gaulbut other things like remove blob are async

adrianc_not really

gaulsome of this immediateFuture stuff seems to be directories which I'm not sure who uses

adrianc_not in a useful way

adrianc_ex. removeBlob is implemented by a callable and a executorservice

adrianc_the calls take 100-200ms

adrianc_the actual time of blobstore are reading the streams

adrianc_which aren't started until after the future is derefed

adrianc_so it is a false sense of real concurrency value add

gaulwow I didn't realize that

adrianc_basically we are doing things that others could just as well do themselves without hte extra layer of proxies

adrianc_the future only breaks up the expect-continue bit

gaulwell that's terrible!

adrianc_meaning that we conveniently allow users to displace those 100-200ms

adrianc_which in some cases is far less than 100ms

adrianc_yeah exactly

adrianc_in other words we aren't adding "real value"

adrianc_and making futures that aren't cancellable

adrianc_and increasing stack depth, complexity, etc

adrianc_so summary of 4years in my opinion is cut the crap :P

adrianc_there was a dream of doing fancy things under the async api

adrianc_but since that never happened in 4 years, we should acknowledge and move on

adrianc_ ends rationale speal

gaulcan you capture this in an issue?  not guaranteeing I will work on it yet but I do want to understand this better.

adrianc_well there's the mail thread

adrianc_but which issue of the above, the rationale to deprecate and remove the asyncapi?

adrianc_gaul besides the lies the biggest issue with async is the sheer amount of work we do to support it.  ask anyone who's written or reviewed a provider :P

adrianc_etoews what's your take on axing Async?

adrianc_iocanel ^^

adrianc_do you guys ever use it?

adrianc_or rather spend tons of time writing it, like I do

adrianc_write-only asyncapi :P

adrianc_abayer do you use async?

adrianc_mattstep ^^

gaulI don't use any provider specific APIs today, although use both sync and async blobstore ones

gaulis the idea that the async blobstore would call into the sync provider specific API using the executor service?

adrianc_if we retain it, yes

adrianc_however by in large, AsyncBlobStore has many problems that make it a cure worse than the disease

gauloh, you propose dropping all async?  then callers have to wrap it themselves?

adrianc_regardless, I'm happy to convert AsyncBlobStore to lie in the same way, but without the provider asyncApi deps

adrianc_yep

adrianc_because then they can at least be aware that the futures aren't cancellable, are just calling a series of sync commands etc

adrianc_right now, many probably believe there's something fundamentally gained vs submit(callableFor(sync))

gaulwell I don't want anyone to lie in the endgame.  why can't asyncblobstore be as simple as submitting a future wrapping the sync interface?

adrianc_which is not true, and only causes us work

adrianc_gaul it can for you :)

adrianc_but otherwise I'd remove it

adrianc_ex. if you want it to lie, we'll keep it

adrianc_but the other stuff really should go

adrianc_in other words, the AsyncBlobStore is begign and we can remove that later

gauldisappointed that cancel lies to him

adrianc_the provider stuff is the real problem, that dramatically increases core complexity and time writing and reviewing providers, but without value

adrianc_I'd be happy to port AsyncBlobStore to lie less, if it meant I can remove the async legacy from core

adrianc_last thing I want is apache to just mean more of me doing complex work in core and no-one wanting to write providers

gaulwhy can't cancel raise an interruptedexception in the sync callee?

adrianc_if we can remove the complexity out of core, we have a chance at real growth

adrianc_it doesn't break the socket or nything

adrianc_do a test :)

adrianc_if you cancel a blob in progres

adrianc_look at the wire log

adrianc_you'll see it continues

adrianc_it only interrupts the caller

adrianc_same as TimeLimiter

gaulwhy doesn't it push all the way through to the socket?

gaulmissing something basic

adrianc_so the future is awaiting the response

adrianc_well think of it another way

adrianc_how would you use HttpUrlConnection and force close a socket?

adrianc_then, how would you do that in a callable such that if a future of the work in progress invokes that

adrianc_this is the essential point

adrianc_If you make a test that guarantees that a socket is interrupted outside jclouds, we could employ this strategy

adrianc_however, I'd make it a Futures2 thing vs requiring all provider api calls to randomly implement async

adrianc_for example, if you look at the size of the codebase, the parts relevant to blobstore async is a small fraction

adrianc_tiny really

adrianc_all the while apis pay price of heavy maintenance and complexity, and at the end of the day , it is for naught

adrianc_we rarely get email on the ML

adrianc_about "wow" futures work really well!

adrianc_on the other hand ... :P

adrianc_lets just say I could buy many beers if I had a nickel for every unitelligble stack trace, bug, or question about async

gaulhmm I see, we need to disrupt the socket somehow

gaulI guess this is really an issue with blocking sockets

gauloh

gaulcatches up with the beginning of the discussion

adrianc_even if we did disrupt, it would likely end up being a hook in Payload

adrianc_which doesn't require async

gaulwell you need it for the timeout case

adriancwhich timeout?  connect and sockettimeout breaks the socket

adriancthe only timeout would be the time the user spends reading the payload

abayeradrianc: a lot of the CloudStack stuff is async.

adriancwhich could be addressed by a hook in payload in a finally block

adriancabayer yeah so this is another reason to quit async :)

adriancin rest apis async has meaning

adriancit means that there is a job in progress

adriancetoews ^^

adriancnot literally this is a non-blocking http call

gaulwell I'm thinking about situations where we want to shoot down some futures due to external event and waiting the full socket timeout is prohibitive

abayerYeah, pretty sure CloudStack follows that model.

adriancgaul well this is impossible now

adriancregardless

gaulright

adriancmaybe good ole thread.stop :P

adriancit could be done with nonblocking I suppose, but yeah we never did htat

adriancand also the callback model for NB is different than a future

adriancif you look at async-http-client, you'll see what I mean

adriancso a "hard-stop" with blocking i/o is probably killing the vm unless someone knows a fancier way

adriancand if there's another way it would probably involve using a more advanced http impl

adrianclike apachehc or the like, I suppose

adriancat any rate, we would need to be able to stop sync commands as you'll notice AsyncBlobStore is implemented with them

adriancso if we could implement that feature, we could stop the sync commands, and then that could be wrapped in some closure of some JVM lang

adriancso, basically, if we clear the cruft of the async provider impls, and acknowledge they do more harm than good

adriancnot only do we have less complexity and stack trace, but also folks besides you and me might be willing to work on it

adriancright now the complexity level to support async at core is very high

adriancwhich is probably why no-one besides me works on it

adriancthe reason the complexity is high is due to supporting async native

adriancif we remove this, then before during or post apache, we'll likely have more help for the real issues such as "close the frigging socket, please"

adriancright now, it is pulling teeth to test *anything* async

adriancso it doesn't get tested

adriancand it stays complex, burdensome, and sad

adriancasync as in Future<Thing> :)

gaulsad too

gaullife is less magical than I thought it was

adriancso basically I'm lobbying to take the hit now, so that we can make a better impl of jclouds over time

adriancinstead of treading water

adriancremoving async would literally remove hundreds of useless classes from providers

adriancand make it straightforward to see things like what you are noticing now

adriancthat thread.interrupt doesn't kill a socket

adriancwhen the clutter is clear, then I really do thing we have a fighting chance at getting more miles of jclouds vs miles on a treadmill

adriancto help with transition, we could deprecate asyncblobstore for 2 versions

adriancwhile only the async provider apis for 1

adriancfirst phase unlocking asyncblobstore from the provider async so that it can outlast

adrianchere's a quick way to put things into perspective..

adrianchow many times have you seen <S, A> generic params tossed around

adrianccross that with the known individuals who know what the hell that's about

adriancmultiply number of commands in all jclouds apis by 2

adriancand half unit test coverage

adriancthis is the impact of the async api

gaul:/

adriancnow.. lets compare this to its value :)

adriancRestContext<S3Client, S3Client>.. whoops

adriancI meant RestContext<NovaApi, NovaAsyncApi> etc. in mailing lists

adriancall this stuff .. imagine it gone..

adriancand I said to myself... what a wonderful world...

adriancetoews with me?

adriancI'm in a difficult position as I know for a fact this stuff weighs down the community

adriancand largely bear the weight for it, outside those who write providers

adriancwhat would happen if we went to apache and someone needed a core code change?

adriancand for some reason I'm hit by a bus

gaulmuni buses are slow and easy to dodge ;)

adriancyet they still kill!

adriancthough me stopping alcohol increases my chances :)

etoewsbut if you're drunk enough you'll go limp and be less likely to be hurt. :P

etoewsjust got back from pho. catching up.

gauletoews: that's the spirit, double down!

adrianc resumes drinking so he could survive to maintain the async api

etoewsfor the record, we haven't really used the asyn api at all.

adriancetoews have you ever written a class for it and made a mistake because sync is mildly different than async?

etoewsin my workshops and presentations it's usually one bullet point on the end, under the heading "Advanced"

etoewsoh that's happened to me plenty.

adriancand then get some crazy error and figure out oh... Future<Set<Thing>> not Future<Iterable<Thing>>

adriancwhat if you woke up and realized you had all that time back

etoewsi'm pretty much conditioned now, when i see a stack trace 10 stories tall, to check my method sigs.

adrianchow far could you fly on those hourse?

adriancimagine a new ASF contributor

adriancdoes async make them want to help?

adriancimagine a senior dev who could help in core and wants to

etoewsam i to understand that if we removed async that we could get rid of the proxy?

adriancwould they enjoy the hall of mirrors there to allow async?

adriancyep

adriancbascially NovaApi.. that's it

adriancwe could even kill Context

etoewsthat alone makes me want to get rid of it.

adriancif we wanted to, and just make NovaApi closable

etoewshitting the proxy wall when you're debugging is where i've lost the most time.

adriancme too

adriancespecially trying to optimize the proxy wall

adrianctook me weeks

adriancand it still sucks

adriancI mean we still have a proxy, just not two :)

adriancsomething reads the annotations, but even that we could flex on

adriancex. for starters, just remove the requirement ot have mirror interfaces

adriancthen support concrete methods

adriancex. when somehting just returns void, no need to dance with annotations

adriancjust use httpclient.invoke(request)

etoewssgtm

adriancbut even before concrete, just clearing the path and halving the size of pull requests is very interesting to me for long-term success of jclouds

etoewsIMHO, the proxy stuff is a major barrier to learning how to contribute to jclouds.

etoewsyep

adriancyep exactly

adriancwhat I'm proposing is to cut ties

mattstepetoews : if you're not familiar with java.util.concurrent, you should probably read http://www.amazon.com/Java-Concurrency-Practice-Brian-Goetz/dp/0321349601 its probably the second best java book anyone has written, next to Bloch's effective java

adriancso that in incubation we can clear up and hopefully others will help make provider writing easy

adriancIt will be much easier regardless once async is nixed.  not that async is terrible, more that it doesn't help -> in the way

adriancmattstep +1 on that.. also good to re-read

mattstepmoving away from supporting async also lets us reduce the amount of generated proxies and such

etoewsmattstep: ya. it's been on my to read list for far too long.

etoewsreducing (eventually eliminating?) is my #1 reason for wanting to see async go away.

mattstepok, finally caught up

mattstepyeah, I tell people I work with if they want to avoid code review comments, read those two book and follow them religiously, then I really don't have much beyond "wtf does this do?" type comments

mattstepbooks even

mattstepadrianc : I think this is a brilliant idea, and I wholeheartedly support it.  Async apis in jclouds doesn't actually help anyone

mattstepit's a ton of extra code we have to maintain, and the prospect of sharing the load of what you do for the community today makes me defintely want it gone

gaulthis sounds like a win generally.  I suspect needing make some other accommodation for blobstore eventually since users tend to issue lots of small, parallel RPCs

mattstepalso, it would be awesome to have the flexibility when implementing the apis to be able to use interfaces, abstract classes, or concrete classes, and be able to only use the jersey stuff when it makes sense to

gauldoesn't seem like that applies to compute which is what most people use

mattstepand i think this is the only realistic way to get there

mattstepgaul : agreed

mattstepwell, i think there are some operations on compute that are like that, like checking the state of all the vms in a collection

mattstepbut it's not as big of a deal

gauladrianc: ok I understand the problem now.  I am happy to help with at least the blobstore side changes.

adrianccatching up on reading

gauland probably the interim fake asyncblobstore until I figure out how to provide a more meaningful workaround

mattstepcan actually help on blobstore now, and will :)

mattstepman, this channel is so much more interesting when adrianc is here, everyone else (including me) is so damn boring

adriancsorry got pulled away

mattstepj/k

adrianc:)

adrianccool sounds like we are on the same page.

adriancand yeah there are valuable reusable tasks we still want to make easy esp in blobstore and compute

adriancthis isn't against that, more making room to work on that

adriancso my request is for you guys to speak your piece on  https://groups.google.com/forum/?fromgroups=#!topic/jclouds-dev/iQXXZFeVq2Y

adriancI think tactically we can move pretty fast

adriancmeaning that in 1.6 we can add @Deprecated to the async apis and crufty things like BlobMap

mattstep^_^

adriancand carve in a way to build a rest connection that only returns the "formerly known as sync" api

adriancex. ContextBuilder....build(NovaApi.class);

adriancwhich makes for happy users and devs imho

mattstepagreed!

adriancI think the prep stuff is totally doable and I volunteer weekend time on this

adriancthen in master, we simply remove the async stuff

adriancand there's a valid and compatible transition path

adriancmake sense?

mattstepdoes to me, i'm happy to help as well

adriancif folks are interested, we could hack some together this weekend somewhere

adriancp.s. I'll start tasking out the todo

adriancthere are many things that can be done in parallel

[Adrian Cole]

This definitely removes some proxies, and is the first step towards supporting concrete implementations of ThingApi.

[Ignasi]

I really like how easy and elegant is right now to perform asynchronous operations in jclouds, but I also like the idea of keeping things as simple as possible. Code will be easier to read, maintain and debug, so +1.

Apart from what has been mentioned, there will be some strategies (at least in the abiquo and chef providers) that use the async apis via the "transformParallel" utility. I'm sure we'll find more providers using similar approaches, so we need to take that into account too.

I think we can not properly estimate how many people is using the async stuff (we use it in Abiquo) so we should take special care documenting the new way of working asynchronously, to make the migrations as easy as possible.

Ignasi

[Adrian Cole]

thanks for the support, Ignasi.

Within jclouds, I'm not sure the merit of continuing support or use of transformParallel.  Many times it works fine, but when it fails, it dies terribly and is scary in implementation.  It is definitely on my shit list :P

WRT migration strategies, there's a few routes I think.

 * instruct how to do what's going on now (return executor.submit(callableForGet(syncApi))).  For the rare cases where asyncapi is actually used for scatter-gather (typical in HATEOAS or delete in a for loop), this could be used and retains the same pros/cons of the current impl, adding the pro and con of visibility into exactly what's going on.

 * in the case where parallel isn't actually saving much or worse, causing issues, change to sync.

 * advise folks to craft their own parallel strategies, possibly looking at Brian Goetz book as a reference.

 * point outsiders who don't want to use java.util.concurrent or guavish stuff to libraries like rxjava (which has almost no deps) [1], hystrix (a few more deps, but tolerable)[2], heavier things like storm trident [3], or other languages to do parallel things.

 * (your idea here :) )

-A

[1] https://github.com/Netflix/RxJava/wiki

[2] https://github.com/Netflix/Hystrix/blob/master/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCollapser.java

[3] https://github.com/nathanmarz/storm/wiki/Trident-tutorial

[Andrew Phillips]

Would a couple of new examples in jclouds-examples on adding async
logic on top of the sync API help for the transition, in terms of
giving users a concrete piece of code to look at? Or are there too
many good ways of implementing that for us to want to make any
specific suggestions?

ap

[Everett Toews]

+1 for some examples

On Apr 5, 2013, at 7:08 AM, Andrew Phillips wrote:

> Would a couple of new examples in jclouds-examples on adding async logic on top of the sync API help for the transition, in terms of giving users a concrete piece of code to look at? Or are there too many good ways of implementing that for us to want to make any specific suggestions?
>
> ap
>

[Adrian Cole]

Here's my idea of the plan.

Proof is in the pudding.  Like Ignasi said, there are some places where we use async internally, including the dreadful and helpful awaitCompletion thing :)  I volunteer to take on transitioning the gnarly vcloud api off async for its compute adapter.  A pull request therein will have examples that actually work.

Between now and jclouds 1.6.0-rc.4, we can prep folks by doing above work and also the following steps:

1. Make it possible to get just the top-level api (ex. contextBuilder.buildApi(NovaApi.class)

2. Make the top-level apis implement close able, so Context is no longer needed, and folks can still close the implicit executor services.

3. Tag all Async stuff deprecated for removal in jclouds 1.7 (which will likely be released as Apache jclouds 2.0)

4. Add a new example project (possibly just revised from your lab code) that does some asyncy stuff like upload a bunch of blobs concurrently, just using normal j.u.concurrent + guava.

Any folks able to help with the above?

-A

On Fri, Apr 5, 2013 at 5:08 AM, Andrew Phillips <aphi...@qrmedia.com> wrote:

Would a couple of new examples in jclouds-examples on adding async logic on top of the sync API help for the transition, in terms of giving users a concrete piece of code to look at? Or are there too many good ways of implementing that for us to want to make any specific suggestions?

ap

--
You received this message because you are subscribed to the Google Groups "jclouds-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to jclouds-dev+unsubscribe@googlegroups.com.

[Ignasi]

I'll take care of abiquo and chef (no surprise :)), and help with other modules if needed.

To unsubscribe from this group and stop receiving emails from it, send an email to jclouds-dev...@googlegroups.com.

[Adrian Cole]

Thanks, Ignasi.

I'll start some of the groundwork tomorrow morning pacific time, continuing later that afternoon and most of Sunday.

Catch you online!

-A

[Andrew Phillips]

> 4. Add a new example project (possibly just revised from your lab
> code) that does some asyncy stuff like upload a bunch of blobs
> concurrently, just using normal j.u.concurrent + guava.
>
> Any folks able to help with the above?

I can try my hand on 4., with a bit of discussion as regards what we
think good patterns to "recommend" are...

Exercises 3 and 4 [1, 2] would seem like good candidates here.

ap

[1]
https://github.com/jclouds/jclouds-cloud-storage-workshop/blob/master/exercise3/src/main/java/org/jclouds/labs/blobstore/exercise3/FileUploaderB.java
[2]
https://github.com/jclouds/jclouds-cloud-storage-workshop/blob/master/exercise4/src/main/java/org/jclouds/labs/blobstore/exercise4/MyDropboxClientB.java

[Adrian Cole]

I spent the morning on labs stuff.  I'll start async this afternoon.

[Adrian Cole]

I got sidetracked today, and need to catch up on other things. I'll
pick whatever other folks haven't progressed by next weekend.

-A

[Adrian Cole]

Here's the task list.  Feel free to collaborate on it!

https://github.com/jclouds/jclouds/issues/1501

>>>>> an email to jclouds-dev+unsubscribe@googlegroups.com.

>>>>> To post to this group, send email to jclou...@googlegroups.com.
>>>>> Visit this group at http://groups.google.com/group/jclouds-dev?hl=en.
>>>>> For more options, visit https://groups.google.com/groups/opt_out.
>>>>>
>>>>>
>>>>
>>>> --
>>>> You received this message because you are subscribed to the Google
>>>> Groups "jclouds-dev" group.
>>>> To unsubscribe from this group and stop receiving emails from it, send

>>>> an email to jclouds-dev+unsubscribe@googlegroups.com.

>>>> To post to this group, send email to jclou...@googlegroups.com.
>>>> Visit this group at http://groups.google.com/group/jclouds-dev?hl=en.
>>>> For more options, visit https://groups.google.com/groups/opt_out.
>>>>
>>>>
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups
>>> "jclouds-dev" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an

>>> email to jclouds-dev+unsubscribe@googlegroups.com.

[Adrian Cole]

Here's the nail in the coffin pull request: https://github.com/jclouds/jclouds/pull/1511

Note this proves things work by converting route53 to sync-only.

Please review when you can so that I can help convert a couple other newbies.

-A

[Adrian Cole]

syncing to a theater near you is.. 1.6.0-rc.4

ContextBuilder.buildApi(YourApi.class)

https://github.com/jclouds/jclouds/issues/1524

Many thanks to Andrew(s), Matt, and Everett for all the help on pull reviews.

-a

[Everett Toews]

Many thanks to Adrian for the tireless effort.

Everett

To unsubscribe from this group and stop receiving emails from it, send an email to jclouds-dev...@googlegroups.com.