Avro
  1. Avro
  2. AVRO-275

Histogram class to keep RPC timing stats

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3.0
    • Component/s: java
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Class to keep a histogram of RPC timinigs for AVRO-270.

      1. AVRO-275.patch.txt
        11 kB
        Philip Zeyliger
      2. AVRO-275.patch.txt
        11 kB
        Philip Zeyliger

        Issue Links

          Activity

          Hide
          Philip Zeyliger added a comment -

          Attaching a patch.

          I went for a non-adaptive approach: you have to specify the formula (a "segmenter" implementation: I don't much like the name) by which to bucket your values ahead of time, as well as the number of buckets. I wrote one implementation of this formula (using a TreeMap), and linear and exponential versions make sense too.

          This may be more convoluted than necessary: I support generic buckets, not just numbers.

          I looked around a bit on whether I should have borrowed other Apache-licensed code. http://code.google.com/p/hist4j/ has an adaptive approach. Apache commons math has summary statistics code.

          Show
          Philip Zeyliger added a comment - Attaching a patch. I went for a non-adaptive approach: you have to specify the formula (a "segmenter" implementation: I don't much like the name) by which to bucket your values ahead of time, as well as the number of buckets. I wrote one implementation of this formula (using a TreeMap), and linear and exponential versions make sense too. This may be more convoluted than necessary: I support generic buckets, not just numbers. I looked around a bit on whether I should have borrowed other Apache-licensed code. http://code.google.com/p/hist4j/ has an adaptive approach. Apache commons math has summary statistics code.
          Hide
          Doug Cutting added a comment -

          This looks fine, but, again, I don't think this makes much sense in Avro indepdendent of AVRO-270: I'd prefer to commit this, AVRO-273 and AVRO-275 all in a single commit. I also question whether this needs to be public. It's nice to have the timer and the histogram class pluggable, but until someone wants to plug in different implementations, these just increase the number of public APIs we have to maintain back-compatibly. So my vote might be to make these package-private for now.

          Show
          Doug Cutting added a comment - This looks fine, but, again, I don't think this makes much sense in Avro indepdendent of AVRO-270 : I'd prefer to commit this, AVRO-273 and AVRO-275 all in a single commit. I also question whether this needs to be public. It's nice to have the timer and the histogram class pluggable, but until someone wants to plug in different implementations, these just increase the number of public APIs we have to maintain back-compatibly. So my vote might be to make these package-private for now.
          Hide
          Philip Zeyliger added a comment -

          Attaching a new patch where the new non-test classes are package-private.

          As far as committing AVRO-273, AVRO-275, AVRO-279 together, that would be splendid. They're designed so that this one and AVRO-273 could be done independently (though would have no utility), so that the reviews are easier.

          Show
          Philip Zeyliger added a comment - Attaching a new patch where the new non-test classes are package-private. As far as committing AVRO-273 , AVRO-275 , AVRO-279 together, that would be splendid. They're designed so that this one and AVRO-273 could be done independently (though would have no utility), so that the reviews are easier.
          Hide
          Doug Cutting added a comment -

          I just committed this. Thanks, Philip!

          Show
          Doug Cutting added a comment - I just committed this. Thanks, Philip!

            People

            • Assignee:
              Philip Zeyliger
              Reporter:
              Philip Zeyliger
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development