kivra/giallo

Calling cowboy_req:body_qs multiple times on the same req

Closed this issue · 6 comments

Love the framework, I noticed that under load, calling this function https://github.com/kivra/giallo/blob/master/src/giallo.erl#L132 generates badmatch exceptions due to Cowboy not allowing calling cowboy_req:body_qs multiple times on the same Req. I believe body_qs can be called only once on each request because the data is streamed using transport:receive.

In simple test using command line curl, giallo:post_param can be called many times on the samerequest, but once I generated some load using JMeter 100 concurrent threads to POST data, then cowboy starts throwing timeout nondeterministically.

The way Axiom deals with it is https://github.com/tsujigiri/axiom/blob/master/src/axiom.erl#L152

I wish we could call body_qs many times on the same request, this way the body parameters would not need to be passed around.

Great find, I'll whip up a patch you can try out

Strange thing is I can't get this to work:
{ok, BodyQS1, Req1} = cowboy_req:body_qs(Req)
{ok, BodyQS2, Req2} = cowboy_req:body_qs(Req1)

The second time I get an empty list, I'll talk to essen to see if he has any insights into this. Otherwise this patch/branch should introduce changes returns the new Cowboy Req to be used for subsequent requests.

My understanding is that you are allowed/able to call cowboy_req:body_qs(Req) only once per request because they are streamed, once that happens, for the remainder of the life of that request, the result of the similar call will be undeterministic. We talked with essen that ideally another call to cowboy_req:body_qs(Req) should return some message saying that body_qs have been already been read ninenines/cowboy#502

IMHO, due to the goal of having high performance of the Cowboy server, it's just the way it is.

Ok, makes sense. Looks like the only option without resorting to gen_server and ets tables seem to set the
body back on the req buffer cowboy_req:set(buffer, Buffer). The Axiom code you linked to basically does the same thing as my patch?

Take a look at this branch, I set the body buffer back on the Req for subsequent requests;
https://github.com/kivra/giallo/tree/bt-0.3.0

Thank you for the patch. It is working great. Your code also shows how a csrf middleware can be implemented for cowboy.