[ 
https://issues.apache.org/jira/browse/HADOOP-13028?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15274739#comment-15274739
 ] 

Chris Nauroth commented on HADOOP-13028:
----------------------------------------

bq. {{S3AInputStream#closed}}: it seems like this should be an 
{{AtomicBoolean}}. Otherwise two threads could both enter this code block, 
right?

I think this is OK.  The whole {{close}} method is {{synchronized}}, so we 
won't have two threads concurrently doing the actual close.  Almost all other 
accesses of {{closed}} are within {{synchronized}} methods too.  It's marked 
{{volatile}} to help with one unsynchronized access from {{readFully}}, calling 
into {{checkNotClosed}}.  That's only a read, not an update, so {{volatile}} is 
sufficient.

I don't object to using {{AtomicBoolean}}, but I don't think it's necessary.

bq. Is it really necessary to put statistics information into the toString 
methods of the streams?

I'd like to keep this.  It's very convenient for logging.  
{{TestS3AInputStreamPerformance}} uses it for both logging output and detailed 
assertion messages.  It's poor practice to rely on a Java object's {{toString}} 
output as a stable, parseable format.  This is something that I'd like to see 
clarified in our compatibility documentation.

I don't have a strong opinion on the naming questions.

> add low level counter metrics for S3A; use in read performance tests
> --------------------------------------------------------------------
>
>                 Key: HADOOP-13028
>                 URL: https://issues.apache.org/jira/browse/HADOOP-13028
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: fs/s3, metrics
>    Affects Versions: 2.8.0
>            Reporter: Steve Loughran
>            Assignee: Steve Loughran
>         Attachments: HADOOP-13028-001.patch, HADOOP-13028-002.patch, 
> HADOOP-13028-004.patch, HADOOP-13028-005.patch, HADOOP-13028-006.patch, 
> HADOOP-13028-007.patch, HADOOP-13028-008.patch, HADOOP-13028-009.patch, 
> HADOOP-13028-branch-2-008.patch, HADOOP-13028-branch-2-009.patch, 
> HADOOP-13028-branch-2-010.patch, 
> org.apache.hadoop.fs.s3a.scale.TestS3AInputStreamPerformance-output.txt, 
> org.apache.hadoop.fs.s3a.scale.TestS3AInputStreamPerformance-output.txt
>
>
> against S3 (and other object stores), opening connections can be expensive, 
> closing connections may be expensive (a sign of a regression). 
> S3A FS and individual input streams should have counters of the # of 
> open/close/failure+reconnect operations, timers of how long things take. This 
> can be used downstream to measure efficiency of the code (how often 
> connections are being made), connection reliability, etc.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to