aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maxim Khutornenko" <ma...@apache.org>
Subject Re: Review Request 25760: Performance improvements and instrumentation for snapshot
Date Thu, 18 Sep 2014 18:41:55 GMT


> On Sept. 18, 2014, 12:56 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java, line 152
> > <https://reviews.apache.org/r/25760/diff/1/?file=693090#file693090line152>
> >
> >     Technically, the outBytes may already be disposed at this point. Move return
inside of outer try()?
> 
> Kevin Sweeney wrote:
>     This is intentional since doing it outside the block means .close() will have been
called on the deflater output stream, flusing any buffers.
>     
>     outBytes won't have been disposed because it's still in scope. From http://docs.oracle.com/javase/7/docs/api/java/io/ByteArrayOutputStream.html#close()
>     
>     "Closing a ByteArrayOutputStream has no effect. The methods in this class can be
called after the stream has been closed without generating an IOException."

FWIW, your ByteArrayOutputStream is already flushed as closing the transport will also close
its inner output stream. BTW, you may want to move transport.close() into its own finally{}
to cover the exception case.


- Maxim


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


On Sept. 18, 2014, 6:13 p.m., Kevin Sweeney wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25760/
> -----------------------------------------------------------
> 
> (Updated Sept. 18, 2014, 6:13 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-722
>     https://issues.apache.org/jira/browse/AURORA-722
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> * Deflate snapshots using stream API
> * Make LogManager non-final
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java da4f0e5520f85782bbc246752cde29bd279be466

>   src/main/java/org/apache/aurora/scheduler/storage/log/Entries.java 1eca768a7daf6defd4bb35a5c46e9c0eab370d92

>   src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java 0b59043cf5f01c99d09168d7669f51b686d2e930

>   src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java 7aaab1e7debbfed65ac7c15154ec73fc9f2114af

> 
> Diff: https://reviews.apache.org/r/25760/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> The StreamManager @Timed annotations don't work yet since guice isn't used to instantiate
it. Ideally we'd use AssistedInject here, but that's a slightly larger change.
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>


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