aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Zameer Manji <zma...@apache.org>
Subject Re: Review Request 51513: Add support for ETags in the Aurora API.
Date Tue, 30 Aug 2016 22:04:46 GMT


> On Aug. 29, 2016, 6:32 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java, lines
139-142
> > <https://reviews.apache.org/r/51513/diff/1/?file=1488357#file1488357line139>
> >
> >     Am I reading this correctly: you're buffering the entire response in memory
so we can calculate the hash for the etag? Do you envision this causing memory pressure for
larger responses (especially given that the goal here is to optimize for the case where responses
are large enough that sending them on the wire and processing them on the client side is causing
a perf problem)?
> >     
> >     At the very least, perhaps we should only buffer the response if the `If-None-Match`
header is set?
> 
> Zameer Manji wrote:
>     If the `If-None-Match` header is not set, we need to compute the `ETag` header for
the response anyways so the client could send it in a future request.
>     If it is set, we need to compute it to compare against the value of the `If-None-Match`
to determine if we should send a 304 or not.
>     
>     I understand your concern with buffering responses. I have no information right now
if this will change memory allocation patterns or not, but I doubt it will cause a huge impact.
We don't even `flush` the output stream of the response here so nothing says we aren't already
buffering the response.
>     
>     If the memory performance is a big concern, I could wrap the outputstream of the
response and have that compute the hash as bytes are given.
> 
> Joshua Cohen wrote:
>     If we can stream the bytes into the hash calculation rather than buffering, that
would be ideal.

Actually, after further inspection I have determined we need the buffering. If I wrap the
response output stream and compute the hashcode at the same time and later determine that
the hashcode matches the `If-None-Match` header, it's impossible to send an HTTP 304 with
an empty body.

I think this code should stand as is.


- Zameer


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51513/#review147255
-----------------------------------------------------------


On Aug. 29, 2016, 6:12 p.m., Zameer Manji wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51513/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2016, 6:12 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1757
>     https://issues.apache.org/jira/browse/AURORA-1757
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This patch enhances the Aurora API by producing an `ETag` header for each API
> request. It also consumes etag values in API requests via the `If-None-Match`
> header and produces a HTTP 304 if the request would produce a body with the same
> etag value.
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md 1819eaa20cf5014228643a1e120316d646cc2824 
>   src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java 1634cb88ac09c778c5bb277ca902f4ca35dd6c9d

>   src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java 0a3ff05586c87e0ab2cc20470e99b5dd609f7039

> 
> Diff: https://reviews.apache.org/r/51513/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message