Mailing-List: contact hdfs-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: hdfs-issues@hadoop.apache.org
Date: Mon, 25 Apr 2016 19:21:13 +0000 (UTC)
From: "Colin Patrick McCabe (JIRA)" <jira@apache.org>
To: hdfs-issues@hadoop.apache.org
Message-ID: <JIRA.12951000.1458162347000.26741.1461612073271@Atlassian.JIRA>
In-Reply-To: <JIRA.12951000.1458162347000@Atlassian.JIRA>
References: <JIRA.12951000.1458162347000@Atlassian.JIRA>
 <JIRA.12951000.1458162347692@arcas>
Subject: [jira] [Commented] (HDFS-10175) add per-operation stats to
 FileSystem.Statistics
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ 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)