You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "Chris Douglas (JIRA)" <ji...@apache.org> on 2009/11/03 03:29:59 UTC

[jira] Updated: (HADOOP-6244) Improvements to FileContext metrics output formatting

     [ https://issues.apache.org/jira/browse/HADOOP-6244?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Chris Douglas updated HADOOP-6244:
----------------------------------

    Status: Open  (was: Patch Available)

While timestamps and distinguishing tags from metrics are both useful properties, changing the format of FileContext disrupts all the downstream consumers. Other metrics consumers (e.g. Chukwa) have defined different MetricsContexts to effect formatting changes, rather than modifying this class. That said, either (a) making the format configurable or (b) making FileContext more readily extensible would be welcome changes, since most alternative implementations end up copying FileContext. IIRC, there's a Log4j-based MetricsContext in Chukwa that may be a worthy, if heavyweight option for a configurable, file-based context.

Other, small notes:
* This seems unnecessary, as the lock is held while accessing {{updaters}}:
{noformat}
-  private void timerEvent() throws IOException {
+  private synchronized void timerEvent() throws IOException {
     if (isMonitoring) {
       Collection<Updater> myUpdaters;
       synchronized (this) {
{noformat}
Either the latter synchronized block needs to be removed (with the comment justifying it) or this should be reverted.
* The protected fields are part of FileContext's API and probably should not be made package-private.
* {{emitNowForTests}} is a regrettable method, but then again, so is use of {{Timer}}, and the metrics have been without any unit tests for too long. Still, this makes timerEvent a public API, despite the javadoc. Adding a call to {{timerEvent}} after {{stopTimer}} in {{stopMonitoring}} is tempting, but the caller of the latter method should probably not block if the metrics update does. Since it's only required for FileContext testing at the moment, would it be reasonable to let {{emitNowForTests}} be a package-private on FileContext?
* The unit test should use JUnit4 rather than JUnit3 semantics
* Adding a static class to read records emitted by FileContext would be great, but I think the notion highlights how the metrics package would be better served by adding a FileContext using a standard format, like JSON, or base it on Avro.

> Improvements to FileContext metrics output formatting
> -----------------------------------------------------
>
>                 Key: HADOOP-6244
>                 URL: https://issues.apache.org/jira/browse/HADOOP-6244
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: metrics
>    Affects Versions: 0.22.0
>            Reporter: Todd Lipcon
>            Assignee: Todd Lipcon
>             Fix For: 0.22.0
>
>         Attachments: hadoop-6244.txt
>
>
> The output of FileContext has two big issues: 1) it doesn't include a timestamp, 2) it doesn't differentiate between tags and metrics in formatting. This patch is to improve the output format to be more useful.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.