Commit Graph

37 Commits

Author SHA1 Message Date
Tolga Ceylan
afeb8e6f6a fn: json excess data check should ignore whitespace (#830)
* fn: json excess data check should ignore whitespace

* fn: adjustments and test case
2018-03-09 11:59:30 -08:00
Tolga Ceylan
7677aad450 fn: I/O related improvements (#809)
*) I/O protocol parse issues should shutdown the container as the container
goes to inconsistent state between calls. (eg. next call may receive previous
calls left overs.)
*) Move ghost read/write code into io_utils in common.
*) Clean unused error from docker Wait()
*) We can catch one case in JSON, if there's remaining unparsed data in
decoder buffer, we can shut the container
*) stdout/stderr when container is not handling a request are now blocked if freezer is also enabled.
*) if a fatal err is set for slot, we do not requeue it and proceed to shutdown
*) added a test function for a few cases with freezer strict behavior
2018-03-07 15:09:24 -08:00
Reed Allman
206aa3c203 opentracing -> opencensus (#802)
* update vendor directory, add go.opencensus.io

* update imports

* oops

* s/opentracing/opencensus/ & remove prometheus / zipkin stuff & remove old stats

* the dep train rides again

* fix gin build

* deps from last guy

* start in on the agent metrics

* she builds

* remove tags for now, cardinality error is fussing. subscribe instead of register

* update to patched version of opencensus to proceed for now TODO switch to a release

* meh

fix imports

* println debug the bad boys

* lace it with the tags

* update deps again

* fix all inconsistent cardinality errors

* add our own logger

* fix init

* fix oom measure

* remove bugged removal code

* fix s3 measures

* fix prom handler nil
2018-03-05 09:35:28 -08:00
Reed Allman
a56d204450 fix up response headers (#788)
* fix up response headers

* stops defaulting to application/json. this was something awful, go stdlib has
a func to detect content type. sadly, it doesn't contain json, but we can do a
pretty good job by checking for an opening '{'... there are other fish in the
sea, and now we handle them nicely instead of saying it's a json [when it's
not]. a test confirms this, there should be no breakage for any routes
returning a json blob that were relying on us defaulting to this format
(granted that they start with a '{').
* buffers output now to a buffer for all protocol types (default is no longer
left out in the cold). use a little response writer so that we can still let
users write headers from their functions. this is useful for content type
detection instead of having to do it in multiple places.
* plumbs the little content type bit into fn-test-util just so we can test it,
we don't want to put this in the fdk since it's redundant.

I am totally in favor of getting rid of content type from the top level json
blurb. it's redundant, at best, and can have confusing behaviors if a user
uses both the headers and the content_type field (we override with the latter,
now). it's client protocol specific to http to a certain degree, other
protocols may use this concept but have their own way to set it (like http
does in headers..). I realize that it mostly exists because it's somewhat gross
to have to index a list from the headers in certain languages more than
others, but with the ^ behavior, is it really worth it?

closes #782

* reset idle timeouts back

* move json prefix to stack / next to use
2018-02-27 10:30:33 -08:00
Travis Reeder
575e1d3d0c Removes "type" from json format. Was pointless. (#783) 2018-02-20 12:04:08 -08:00
Reed Allman
04ae223a5d fixup json,http protocols (#772)
* http now buffers the entire request body from the container before copying
it to the response writer (and sets content length). this is a level of sad i
don't feel comfortable talking about but it is what it is.
* json protocol was buffering the entire body so there wasn't any reason for
us to try to write this directly to the container stdin manually, we needed to
add a bufio.Writer around it anyway it was making too many write(fd) syscalls
with the way it was. this is just easier overall and has the same performance
as http now in my tests, whereas previously this was 50% slower [than http].
* add buffer pool for http & json to share/use. json doesn't create a new
buffer every stinkin request. we need to plumb down content length so that we
can properly size the buffer for json, have to add header size and everything
together but it's probably faster than malloc(); punting on properly sizing.
* json now sets content type to the length of the body from the returned json
blurb from the container

this does not handle imposing a maximum size of the response returned from a
container, which we need to add, but this has been open for some time
(specifically, on json). we can impose this by wrapping the pipes, but there's
some discussion to be had for json specifically we won't be able to just cut
off the output stream and use that (http we can do this). anyway, filing a
ticket...

closes #326 :(((((((
2018-02-14 14:06:36 -08:00
Reed Allman
9cbe4ea536 add pprof endpoints, additional spans (#770)
i would split this commit in two if i were a good dev.

the pprof stuff is really useful and this only samples when called. this is
pretty standard go service stuff. expvar is cool, too.

the additional spannos have turned up some interesting tid bits... gonna slide
em in
2018-02-13 20:01:41 -08:00
Reed Allman
97194b3d8b return bad function http resp error (#728)
* return bad function http resp error

this was being thrown into the fn server logs but it's relatively easy to get
this to crop up if a function user forgets that they left a `println` laying
around that gets written to stdout, it garbles the http (or json, in its case)
output and they just see 'internal server error'. for certain clients i could
see that we really do want to keep this as 'internal server error' but for
things like e.g. docker image not authorized we're showing that in the
response, so this seems apt.

json likely needs the same treatment, will file a bug.

as always, my error messages are rarely helpful enough, help me please :)

closes #355

* add formatting directive

* fix up http error

* output bad jasons to user

closes #729

woo
2018-02-12 17:51:45 -08:00
Reed Allman
3b261fc144 pipe swapparoo each slot (#721)
* pipe swapparoo each slot

previously, we made a pair of pipes for stdin and stdout for each container,
and then handed them out to each call (slot) to use. this meant that multiple
calls could have a handle on the same stdin pipe and stdout pipe to read/write
to/from from fn's perspective and could mix input/output and get garbage. this
also meant that each was blocked on the previous' reads.

now we make a new pipe every time we get a slot, and swap it out with the
previous ones. calls are no longer blocked from fn's perspective, and we don't
have to worry about timing out dispatch for any hot format. there is still the
issue that if a function does not finish reading the input from the previous
task, from its perspective, and reads the next call's it can error out the
second call. with fn deadline we provide the necessary tools to skirt this,
but without some additional coordination am not sure this is a closable hole
with our current protocols since terminating a previous calls input requires
some protocol specific bytes to go in (json in particular is tricky). anyway,
from fn's side fixing pipes was definitely a hole, but this client hole is
still hanging out. there was an attempt to send an io.EOF but the issue is
that will shut down docker's read on the stdin pipe (and the container). poop.

this adds a test for this behavior, and makes sure 2 containers don't get
launched.

this also closes the response writer header race a little, but not entirely, I
think there's still a chance that we read a full response from a function and
get a timeout while we're changing the headers. I guess we need a thread safe
header bucket, otherwise we have to rely on timings (racy). thinking on it.

* fix stats mu race
2018-01-31 17:25:24 -08:00
Dario Domizioli
e753732bd8 Hot protocols improvements (for 662) (#724)
* Improve deadline handling in streaming protocols

* Move special headers handling down to the protocols

* Adding function format documentation for JSON changes

* Add tests for request url and method in JSON protocol

* Fix  protocol missing fn-specific info

* Fix import

* Add panic for something that should never happen
2018-01-31 12:26:43 +00:00
Reed Allman
20089c4e83 make headers quasi-consistent (#660)
possible breakages:

* `FN_HEADER` on cold are no longer `s/-/_/` -- this is so that cold functions
can rebuild the headers as they were when they came in on the request (fdks,
specifically), there's no guarantee that a reversal `s/_/-/` is the original
header on the request.
* app and route config no longer `s/-/_/` -- it seemed really weird to rewrite
the users config vars on these. should just pass them exactly as is to env.
* headers no longer contain the environment vars (previously, base config; app
config, route config, `FN_PATH`, etc.), these are still available in the
environment.

this gets rid of a lot of the code around headers, specifically the stuff that
shoved everything into headers when constructing a call to begin with. now we
just store the headers separately and add a few things, like FN_CALL_ID to
them, and build a separate 'config' now to store on the call. I thought
'config' was more aptly named, 'env' was confusing, though now 'config' is
exactly what 'base_vars' was, which is only the things being put into the env.
we weren't storing this field in the db, this doesn't break unless there are
messages in a queue from another version, anyway, don't think we're there and
don't expect any breakage for anybody with field name changes.

this makes the configuration stuff pretty straight forward, there's just two
separate buckets of things, and cold just needs to mash them together into the
env, and otherwise hot containers just need to put 'config' in the env, and then
hot format can shove 'headers' in however they'd like. this seems better than
my last idea about making this easier but worse (RIP).

this means:

* headers no longer contain all vars, the set of base vars can only be found
in the environment.
* headers is only the headers from request + call_id, deadline, method, url
* for cold, we simply add the headers to the environment, prepending
`FN_HEADER_` to them, BUT NOT upper casing or `s/-/_/`
* fixes issue where async hot functions would end up with `Fn_header_`
prefixed headers
* removes idea of 'base' vars and 'env'. this was a strange concept. now we just have
'config' which was base vars, and headers, which was base_env+headers; i.e.
they are disjoint now.
* casing for all headers will lean to be `My-Header` style, which should help
with consistency. notable exceptions for cold only are FN_CALL_ID, FN_METHOD,
and FN_REQUEST_URL -- this is simply to avoid breakage, in either hot format
they appear as `Fn_call_id` still.
* removes FN_PARAM stuff
* updated doc with behavior

weird things left:

`Fn_call_id` e.g. isn't a correctly formatted http header, it should likely be
`Fn-Call-Id` but I wanted to live to fight another day on this one, it would
add some breakage.

examples to be posted of each format below

closes #329
2018-01-09 10:08:30 -08:00
Travis Reeder
96cfc9f5c1 Update json (#463)
* wip

* wip

* Added more fields to JSON and added blank line between objects.

* Update tests.

* wip

* Updated to represent recent discussions.

* Fixed up the json test

* More docs

* Changed from blank line to bracket, newline, open bracket.

* Blank line added back, easier for delimiting.
2017-11-16 09:59:13 -08:00
Denis Makogon
ce25adfddb JSON protocol updating (#426)
* JSON protocol updating

 this patch adds HTTP query string into payload (see more TODOs in code)
 adds one more test to verify query

* Fixing FMT
2017-10-12 23:10:21 +03:00
Denis Makogon
22b5140f56 Do not expect function to set response code 2017-10-07 03:07:21 +03:00
Denis Makogon
e4684096f7 Fmt and docs 2017-10-07 02:59:08 +03:00
Denis Makogon
6141344e5f Error before sending json object if something bad happend with reading a request body 2017-10-07 02:33:43 +03:00
Denis Makogon
6682de4768 Addressing comments 2017-10-07 02:28:56 +03:00
Denis Makogon
181ccf54b4 Addressing more comments + tests 2017-10-07 02:11:49 +03:00
Denis Makogon
9f3bfa1005 Read request body and see if it's not empty then decide whether write it or not 2017-10-07 01:24:43 +03:00
Denis Makogon
b4b5302a44 Addressing certain comments from last review 2017-10-07 01:20:53 +03:00
Denis Makogon
de7b4e6067 Returning error instead of writing it to a response writer 2017-10-07 00:52:01 +03:00
Denis Makogon
7dd9b5a4cd We still can write JSON request object in parts
except just copying content from request body to STDIN
  we need to write encoded data,
  so we're using STDIN JSON stream encoder.
2017-10-07 00:43:09 +03:00
Denis Makogon
588d9e523b Do not forget to close request body 2017-10-07 00:43:09 +03:00
Denis Makogon
c2ee67fb21 Revisiting request body processing 2017-10-07 00:43:09 +03:00
Denis Makogon
1f589d641e Let function write headers to a response 2017-10-07 00:43:09 +03:00
Denis Makogon
caf1488dd9 Make Dispatch cleaner 2017-10-07 00:43:09 +03:00
Denis Makogon
2250e1d08c Get rid of content-length-based copying 2017-10-07 00:43:08 +03:00
Denis Makogon
1cdd241920 Trying to avoid buffers and write directly to pipe
this change makes Dispatch write request body and
 http headers directly to pipe one by one
 in case of non-empty request body,
 if not - write headers and close finalize JSON
2017-10-07 00:43:08 +03:00
Denis Makogon
955b294bc6 Trying to avoid any buffering 2017-10-07 00:43:08 +03:00
Denis Makogon
da9629d8dc Use STDIN as writer for encoding func's JSON input data instead of buffering 2017-10-07 00:43:08 +03:00
Denis Makogon
0316cd90a1 Dismiss redundant function 2017-10-07 00:43:08 +03:00
Denis Makogon
3fb040f293 Addressing comments
What's new?
  - unmarshal JSON response only in case of HTTP response writer
2017-10-07 00:43:07 +03:00
Denis Makogon
783490dc79 Addressing certain comments
What's new?
 - better error handling
 - still need to decode JSON from function because we need status code and body
 - prevent request body to be a problem by deferring its close
 - moving examples around: putting http and json samples into one folder
2017-10-07 00:43:07 +03:00
Denis Makogon
1882845a61 Respond with any error that happends during JSON dispatching 2017-10-07 00:43:07 +03:00
Denis Makogon
3da9ad4328 Using io.LimitReader as the way to control size of request body with respect to content length 2017-10-07 00:43:07 +03:00
Denis Makogon
ecaa5eefbf Cleaning up code
Getting rid of request url, call id, method: all of them are
 redundant and available through env
2017-10-07 00:43:07 +03:00
amykang2020
b6b9b55ca9 apply/make Travis's json-format branch prototype to work with latest restructured master; added StatusCode to JSONOutput server-function contract 2017-10-07 00:43:07 +03:00