Re: Concurrency in invoker http client
BTW, trying prefixAndTail, this doesn’t work for me
e.g.
response.entity.dataBytes
.prefixAndTail(1)
.runWith(Sink.head)
.map({
case (Seq(r), _) =>
Right(ContainerResponse(response.status.intValue, r.utf8String,
Some(contentLength.B, maxResponse)))
})
Does not result with r.size==1, rather r.size == response.entity.size
I will try to fiddle with it some more(and ignore the tail)
> On Jul 13, 2018, at 5:29 PM, Tyson Norris wrote:
>
> Thanks
>
> Currently HttpUtils does return (almost) the same response when it is
> truncated.. so that the truncated version can later be included in the
> ErrorResponse…
>
> I would be OK with changing the error message to not include ANY of the
> response, since either way it is an error message.
> Then the error message would ONLY be:
> "The action produced a response that exceeded the allowed length:
> ${length.toBytes} > ${maxLength.toBytes} bytes.”
> Instead of
> "The action produced a response that exceeded the allowed length:
> ${length.toBytes} > ${maxLength.toBytes} bytes. The truncated response was:
> $trunk"
>
> WDYT?
>
>
>> On Jul 13, 2018, at 5:21 PM, Markus Thoemmes
>> wrote:
>>
>> Hi Tyson,
>>
>> Chetan solved a similar problem in "inlineOrAttach" in
>> "AttachmentSupport.scala". He did this by continuously running a stream
>> through "prefixAndTail", where he'd pick just one element and then run the
>> stream to check whether he already crossed the limit.
>>
>> This case now is a lot more performance critical, so I propose we add a
>> "prefixAndTailWeighted" stage, which runs similar to "prefixAndTail" but
>> weighs the inputs by their siuze so you can define how much you actually
>> want to consume in bytes. You can then run the "tail" stream to an ignore
>> Sink. Further, I'd propose to implement a "quick path" (like there is on in
>> the current HttpUtils), which checks the "Content-Length" field of the
>> response and just consumes the whole stream into a string if it's safe to do
>> so to only use this special method on the truncation path.
>>
>> As a more general, existential question: Do we even need the truncation
>> path? Could we just deny the response straight away if the user's action
>> returns a bigger value than allowed?
>>
>> Hope this helps
>>
>> Cheers,
>> Markus
>>
>
Re: Concurrency in invoker http client
Thanks
Currently HttpUtils does return (almost) the same response when it is
truncated.. so that the truncated version can later be included in the
ErrorResponse…
I would be OK with changing the error message to not include ANY of the
response, since either way it is an error message.
Then the error message would ONLY be:
"The action produced a response that exceeded the allowed length:
${length.toBytes} > ${maxLength.toBytes} bytes.”
Instead of
"The action produced a response that exceeded the allowed length:
${length.toBytes} > ${maxLength.toBytes} bytes. The truncated response was:
$trunk"
WDYT?
> On Jul 13, 2018, at 5:21 PM, Markus Thoemmes
> wrote:
>
> Hi Tyson,
>
> Chetan solved a similar problem in "inlineOrAttach" in
> "AttachmentSupport.scala". He did this by continuously running a stream
> through "prefixAndTail", where he'd pick just one element and then run the
> stream to check whether he already crossed the limit.
>
> This case now is a lot more performance critical, so I propose we add a
> "prefixAndTailWeighted" stage, which runs similar to "prefixAndTail" but
> weighs the inputs by their siuze so you can define how much you actually want
> to consume in bytes. You can then run the "tail" stream to an ignore Sink.
> Further, I'd propose to implement a "quick path" (like there is on in the
> current HttpUtils), which checks the "Content-Length" field of the response
> and just consumes the whole stream into a string if it's safe to do so to
> only use this special method on the truncation path.
>
> As a more general, existential question: Do we even need the truncation path?
> Could we just deny the response straight away if the user's action returns a
> bigger value than allowed?
>
> Hope this helps
>
> Cheers,
> Markus
>
Re: Concurrency in invoker http client
Hi Tyson, Chetan solved a similar problem in "inlineOrAttach" in "AttachmentSupport.scala". He did this by continuously running a stream through "prefixAndTail", where he'd pick just one element and then run the stream to check whether he already crossed the limit. This case now is a lot more performance critical, so I propose we add a "prefixAndTailWeighted" stage, which runs similar to "prefixAndTail" but weighs the inputs by their siuze so you can define how much you actually want to consume in bytes. You can then run the "tail" stream to an ignore Sink. Further, I'd propose to implement a "quick path" (like there is on in the current HttpUtils), which checks the "Content-Length" field of the response and just consumes the whole stream into a string if it's safe to do so to only use this special method on the truncation path. As a more general, existential question: Do we even need the truncation path? Could we just deny the response straight away if the user's action returns a bigger value than allowed? Hope this helps Cheers, Markus
Re: Concurrency in invoker http client
Getting back to this, I’m close, but not sure how to get akka http client to truncate a response? There is response.entity.withSizeLimit() - but this throws an exception, not really what I want. I think what I want is a variant of the HttpEntity.Limitable GraphStage that just stops emitting characters, but I’m not sure how exactly to do this? Thanks Tyson > On Jun 25, 2018, at 10:32 AM, Rodric Rabbah wrote: > > the retires are only on failure to establish a connection - no other > retries should be happening iirc. > > -r > > On Mon, Jun 25, 2018 at 1:29 PM Tyson Norris > wrote: > >> Thanks Markus - one other question: >> >> Assuming retry is the current missing piece to using PoolingRestClient (or >> akka http directly), I’m also wondering if “retry” is the proper approach >> here? >> It may be worthwhile to initiate a port connection (with its own >> timeout/retry behavior) before the /init so that we can distinguish between >> “container startup is slow” and “bad behavior in action container after >> startup”? >> >> Also, I’m wondering if there are cases where rampant retry causes >> unintended side affects, etc - this could be worse with concurrency >> enabled, but I don’t know if this should be considered a real problem. >> >> FWIW We avoid this (http request to container that is not yet listening) >> in mesos by not returning the container till the mesos health check passes >> (which currently just check the port connection), so this would be a >> similar setup at the invoker layer. >> >> Thanks >> Tyson >> >>> On Jun 25, 2018, at 10:08 AM, Markus Thoemmes < >> markus.thoem...@de.ibm.com> wrote: >>> >>> Hi Tyson, >>> >>> Ha, I was thinking about moving back to akka the other day. A few >> comments: >>> >>> 1. Travis build environments have 1.5 CPU cores which might explain the >> "strange" behavior you get from the apache client? Maybe it adjusts its >> thread pool based on the number of cores available? >>> 2. To implement retries based on akka-http, have a look at what we used >> to use for invoker communication: >> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fincubator-openwhisk%2Fcommit%2F31946029cad740a00c6e6f367637a1bcfea5dd18%23diff-5c6f165d3e8395b6fe915ef0d24e5d1f=02%7C01%7Ctnorris%40adobe.com%7Ca07af0966c50438a270908d5dabe4b3a%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C1%7C636655433199601572=ZHjbf0ukNQaEFq2i58f5hxMt3zRa3JCHdHR0MRAn8Uo%3D=0 >> (NewHttpUtils.scala to be precise). >>> 3. I guess you're going to create a new PoolingRestClient per container? >> I could imagine it is problematic if new containers come and go with a >> global connection pool. Just something to be aware of. >>> >>> Oh, another thing to be aware of: We **used** to have akka-http there >> and never tried again after the revert. We're certainly on a much newer >> version now but we had issues of indefinitly hanging connections when we >> first implemented it. Running some high-load scenarios before pushing this >> into master will be needed. >>> >>> I don't want to put you off the task though, give it a shot, I'd love to >> have this back :). Thanks for attacking! >>> >>> Cheers, >>> -m >>> >> >>
Re: Let's maintain and test our Swagger spec
I've submitted a PR for the Swagger spec changes and validations at https://github.com/apache/incubator-openwhisk/pull/3878 For this first iteration, I just focused on making the Swagger spec match the behavior of our existing tests. As Rodric pointed out, that may not exactly match the REST interface, but it at least gets us closer. As a future second iteration, I'd like to investigate generating and maintaining a new Java OpenWhisk client from the spec (see https://github.com/apache/incubator-openwhisk/issues/2466). Using the spec to generate and test a functioning Java client should help find and fix more spec issues and pave the way for potentially moving other hand-generated client libraries over to being at least partially generated from the API spec. Ben On Fri, Jul 6, 2018 at 12:55 AM, Rodric Rabbah wrote: > Ben, you might want to take note of this PR https://github.com/apache/ > incubator-openwhisk/pull/3840 > which removes a number of tests suites (redundant with unit tests). Also > I've found that the WskRestOperations implementation in some places fixes > behaviors to match the CLI instead of strictly implementing the > narrower/strict REST interface. This may mean you have to edit tests (or > remove some that are only valid for clients). > > -r > > On Thu, Jul 5, 2018 at 9:20 PM, Ben Browning wrote: > >> After several failed attempts, I have an approach that I believe will >> work well enough for our needs. I pushed some in-progress code and >> swagger spec fixes to a branch on a fork just so the general approach >> can be shared - >> https://github.com/projectodd/incubator-openwhisk/commit/5ad >> b6be27188dcdc5b9519b57c11802a431b3e9c >> >> This approach uses the swagger-request-validator Java library. I'm >> wiring that up to validate all requests and responses from the >> WskRest* tests transparently, which you can see in >> https://github.com/projectodd/incubator-openwhisk/commit/5ad >> b6be27188dcdc5b9519b57c11802a431b3e9c#diff-5e514c6c3c27b9210 >> d85a08d4ed3ed35 >> (with some cruft and debug output that I'll clean up before submitting >> a PR) >> >> This means that almost all of those tests are now failing due to >> failed swagger spec validations. For now I'm going to concentrate on >> making the spec match the reality and try to get all the existing >> tests green with the validation in place and submit a PR. After that, >> we can patch up any known but uncaught holes by increasing the test >> coverage. >> >> Ben >> >> >> >> On Thu, Jun 28, 2018 at 11:23 PM, Carlos Santana >> wrote: >> > Thanks Ben for the quick update on this task >> > >> > -cs >> > >> > On Thu, Jun 28, 2018 at 5:15 PM Ben Browning >> wrote: >> > >> >> After doing some preliminary poking at this, I believe we'll want to >> >> use either a tool like https://github.com/google/oatts to generate a >> >> test suite from our Swagger spec OR use swagger-codegen to generate a >> >> Scala client from our Swagger spec and try to plug that into the >> >> existing WskRest tests. >> >> >> >> Using the oatts tool doesn't really fit well with the test setup in >> >> the existing incubator-openwhisk repo (where the API spec lives) >> >> because that generates Node.js tests. >> >> >> >> So, I'm leaning towards the second option, which is wiring in >> >> generation of a Scala client into the gradle build and having the >> >> current WskRest test client use this generated Scala client for >> >> testing instead of directly invoking URLs. >> >> >> >> However, last time I played with Scala code generated from Swagger >> >> specs it wasn't that usable. So, a bit more experimentation will >> >> validate whether this option is viable or if other alternatives need >> >> to be considered. I already have a handful of bugs in the API spec >> >> that need to be fixed but I'm waiting to fix and push those until I >> >> can get some kind of testing wired up to reproduce the bugs and verify >> >> the fix. >> >> >> >> Ben >> >> >> >> >> >> On Thu, Jun 21, 2018 at 3:04 PM, Carlos Santana >> >> wrote: >> >> > Thanks Ben for looking into this, having a good API doc/spec and >> matching >> >> > tests is very need it. >> >> > >> >> > +1 >> >> > >> >> > -cs >> >> > >> >> > On Thu, Jun 21, 2018 at 2:25 PM Ben Browning >> >> wrote: >> >> > >> >> >> Our Swagger spec >> >> >> ( >> >> >> >> >> https://github.com/apache/incubator-openwhisk/blob/92a64c291 >> 156a2cd3d6b304babc2a193a46d0699/core/controller/src/main/ >> resources/apiv1swagger.json >> >> >> ) >> >> >> is incomplete and doesn't accurately reflect the actual Controller >> >> >> API. It's manually updated without a full test suite which means it's >> >> >> easy for changes in the API to happen without the spec getting >> >> >> updated. >> >> >> >> >> >> An accurate Swagger spec will not only better document the OpenWhisk >> >> >> API but also allow autogenerated clients in multiple languages to >> >> >> supplement or eventually replace some of the existing client >> >> >> implementations we
Proposal on a future architecture of OpenWhisk
Hello OpenWhiskers, I just published a proposal on a potential future architecture for OpenWhisk that aligns deployments with and without an underlying container orchestrator like Mesos or Kubernetes. It also incooperates some of the proposals that are already out there and tries to give a holistic view of where we want OpenWhisk to go to in the near future. It's designed to keep the APIs stable but is very invasive in its changes under the hood. This proposal is the outcome of a lot of discussions with fellow colleagues and community members. It is based on experience with the problems the current architecture has. Moreover it aims to remove friction with the deployment topologies on top of a container orchestrator. Feedback is very very very welcome! The proposal has some gaps and generally does not go into much detail implementationwise. I'd love to see all those gaps filled by the community! Find the proposal here: https://cwiki.apache.org/confluence/display/OPENWHISK/OpenWhisk+future+architecture Cheers, Markus
