[
https://issues.apache.org/jira/browse/HDFS-10175?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15256842#comment-15256842
]
Colin Patrick McCabe commented on HDFS-10175:
---------------------------------------------
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)
