hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Colin Patrick McCabe (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (HDFS-10175) add per-operation stats to FileSystem.Statistics
Date Mon, 25 Apr 2016 19:21:13 GMT

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

Colin Patrick McCabe edited comment on HDFS-10175 at 4/25/16 7:20 PM:
----------------------------------------------------------------------

Thanks for commenting, [~steve_l].  It's great to see work on s3 stats.  s3a has needed love
for a while.

Did you get a chance to look at my HDFS-10175.006.patch on this JIRA?  It seems to address
all of your concerns.  It provides a standard API that every FileSystem can implement (not
just s3, just HDFS, etc. etc.).  Once we adopt jdk8, we can easily implement this API using
{{java.util.concurrent.atomic.LongAdder}} if that proves to be more readable and/or efficient.

bq. Don't break any existing filesystem code by adding new params to existing methods, etc.

I agree.  My patch doesn't add new params to any existing methods.

bq. add the new code out of FileSystem

I agree.  That's why I separated {{StorageStatistics.java}} from {{FileSystem.java}}.  {{FileContext}}
should be able to use this API as well, simply by returning a {{StorageStatistics}} instance
just like {{FileSystem}} does.

bq. Use an int rather than an enum; lets filesystems add their own counters. I hereby reserve
0x200-0x255 for object store operations.

Hmm.  I'm not sure I follow.  My patch identifies counters by name (string), not by an int,
enum, or byte.  This is necessary because different storage backends will want to track different
things (s3a wants to track s3 PUTs, HDFS wants to track genstamp bump ops, etc. etc.).  We
should not try to create the "statistics type enum of doom" in some misguided attempt at space
optimization.  Consider the case of out-of-tree Filesystem implementations as well... they
are not going to add entries to some enum of doom in hadoop-common.

bq. public interface StatisticsSource {  Map<String, Long> snapshot(); }

I don't think an API that returns a map is the right approach for statistics.  That map could
get quite large.   We already know that people love adding just one more statistic to things
(and often for quite valid reasons).  It's very difficult to \-1 a patch just because it bloats
the statistics map more.  Once this API exists, the natural progression will be people adding
tons and tons of new entries to it.  We should be prepared for this and use an API that doesn't
choke if we have tons of stats.  We shouldn't have to materialize everything all the time--
an iterator approach is smarter because it can be O(1) in terms of memory, no matter how many
entries we have.

I also don't think we need snapshot consistency for stats.  It's a heavy burden for an implementation
to carry (it basically requires some kind of materialization into a map, and probably synchronization
to stop the world while the materialization is going on).  And there is no user demand for
it... the current FileSystem#Statistics interface doesn't have it, and nobody has asked for
it.

It seems like you are focusing on the ability to expose new stats to our metrics2 subsystem,
while this JIRA originally focused on adding metrics that MapReduce could read at the end
of a job.  I think these two use-cases can be covered by the same API.  We should try to hammer
that out (probably as a HADOOP JIRA rather than an HDFS JIRA, as well).

Do you think we should have a call about this or something?  I know some folks who might be
interested in testing the s3 metrics stuff, if there was a reasonable API to access it.


was (Author: cmccabe):
Thanks for commenting, [~steve_l].  It's great to see work on s3 stats.  s3a has needed love
for a while.

Did you get a chance to look at my HDFS-10175.006.patch on this JIRA?  It seems to address
all of your concerns.  It provides a standard API that every FileSystem can implement (not
just s3, just HDFS, etc. etc.).  Once we adopt jdk8, we can easily implement this API using
{{java.util.concurrent.atomic.LongAdder}} if that proves to be more readable and/or efficient.

bq. Don't break any existing filesystem code by adding new params to existing methods, etc.

I agree.  My patch doesn't add new params to any existing methods.

bq. add the new code out of FileSystem

I agree.  That's why I separated {{StorageStatistics.java}} from {{FileSystem.java}}.  {{FileContext}}
should be able to use this API as well, simply by returning a {{StorageStatistics}} instance
just like {{FileSystem}} does.

bq. Use an int rather than an enum; lets filesystems add their own counters. I hereby reserve
0x200-0x255 for object store operations.

Hmm.  I'm not sure I follow.  My patch identifies counters by name (string), not by an int,
enum, or byte.  This is necessary because different storage backends will want to track different
things (s3a wants to track s3 PUTs, HDFS wants to track genstamp bump ops, etc. etc.).  We
should not try to create the "statistics type enum of doom" in some misguided attempt at space
optimization.  Consider the case of out-of-tree Filesystem implementations as well... they
are not going to add entries to some enum of doom in hadoop-common.

bq. public interface StatisticsSource {  Map<String, Long> snapshot(); }

I don't think an API that returns a map is the right approach for statistics.  That map could
get quite large.   We already know that people love adding just one more statistic to things
(and often for quite valid reasons).  It's very difficult to -1 a patch just because it bloats
the statistics map more.  Once this API exists, the natural progression will be people adding
tons and tons of new entries to it.  We should be prepared for this and use an API that doesn't
choke if we have tons of stats.  We shouldn't have to materialize everything all the time--
an iterator approach is smarter because it can be O(1) in terms of memory, no matter how many
entries we have.

I also don't think we need snapshot consistency for stats.  It's a heavy burden for an implementation
to carry (it basically requires some kind of materialization into a map, and probably synchronization
to stop the world while the materialization is going on).  And there is no user demand for
it... the current FileSystem#Statistics interface doesn't have it, and nobody has asked for
it.

It seems like you are focusing on the ability to expose new stats to our metrics2 subsystem,
while this JIRA originally focused on adding metrics that MapReduce could read at the end
of a job.  I think these two use-cases can be covered by the same API.  We should try to hammer
that out (probably as a HADOOP JIRA rather than an HDFS JIRA, as well).

Do you think we should have a call about this or something?  I know some folks who might be
interested in testing the s3 metrics stuff, if there was a reasonable API to access it.

> add per-operation stats to FileSystem.Statistics
> ------------------------------------------------
>
>                 Key: HDFS-10175
>                 URL: https://issues.apache.org/jira/browse/HDFS-10175
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>          Components: hdfs-client
>            Reporter: Ram Venkatesh
>            Assignee: Mingliang Liu
>         Attachments: HDFS-10175.000.patch, HDFS-10175.001.patch, HDFS-10175.002.patch,
HDFS-10175.003.patch, HDFS-10175.004.patch, HDFS-10175.005.patch, HDFS-10175.006.patch, TestStatisticsOverhead.java
>
>
> Currently FileSystem.Statistics exposes the following statistics:
> BytesRead
> BytesWritten
> ReadOps
> LargeReadOps
> WriteOps
> These are in-turn exposed as job counters by MapReduce and other frameworks. There is
logic within DfsClient to map operations to these counters that can be confusing, for instance,
mkdirs counts as a writeOp.
> Proposed enhancement:
> Add a statistic for each DfsClient operation including create, append, createSymlink,
delete, exists, mkdirs, rename and expose them as new properties on the Statistics object.
The operation-specific counters can be used for analyzing the load imposed by a particular
job on HDFS. 
> For example, we can use them to identify jobs that end up creating a large number of
files.
> Once this information is available in the Statistics object, the app frameworks like
MapReduce can expose them as additional counters to be aggregated and recorded as part of
job summary.



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

Mime
View raw message