Flume
  1. Flume
  2. FLUME-1940

Log a snapshot of Flume metrics on shutdown

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: v1.3.1
    • Fix Version/s: v1.4.0
    • Component/s: None
    • Labels:

      Description

      It would be great to log a JMX snapshot to the flume.log at Flume shutdown time.

      1. FLUME-1940.patch
        3 kB
        Israel Ekpo
      2. FLUME-1940.2.patch
        4 kB
        Israel Ekpo
      3. FLUME-1940.3.patch
        6 kB
        Israel Ekpo
      4. FLUME-1940.20130424.patch
        7 kB
        Israel Ekpo

        Issue Links

          Activity

          Mike Percy created issue -
          Mike Percy made changes -
          Field Original Value New Value
          Link This issue relates to FLUME-1957 [ FLUME-1957 ]
          Hide
          Israel Ekpo added a comment -

          I have created a patch for this.

          Will be attaching the patch shortly.

          Show
          Israel Ekpo added a comment - I have created a patch for this. Will be attaching the patch shortly.
          Israel Ekpo made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Assignee Israel Ekpo [ iekpo ]
          Israel Ekpo made changes -
          Attachment FLUME-1940.patch [ 12575466 ]
          Hide
          Mike Percy added a comment -

          Israel, very nice! This is a great start.

          Some suggestions:
          1. It would be great to add this generic logging functionality to MonitoredCounterGroup
          2. Do the same as you did with ChannelCounter to SourceCounter and SinkCounter and just call the above impl from stop()
          3. It would be great to sort the metric key names before printing. As an example:

          List<String> keys = new ArrayList<String>(getCounterMap().keySet());
          Collections.sort(keys);
          for (String key : keys) { ... }
          

          4. Nit: minor indentation issue in the new MonitoredCounterGroup.getName() method (but you may not need to expose that if the impl is there)
          5. It would be great to also post this patch on https://reviews.apache.org/ per https://cwiki.apache.org/confluence/display/FLUME/How+to+Contribute#HowtoContribute-ProvidingPatches - use group Flume - and cross-link them. See other reviews at https://reviews.apache.org/groups/Flume/ as an example

          Thanks!

          Show
          Mike Percy added a comment - Israel, very nice! This is a great start. Some suggestions: 1. It would be great to add this generic logging functionality to MonitoredCounterGroup 2. Do the same as you did with ChannelCounter to SourceCounter and SinkCounter and just call the above impl from stop() 3. It would be great to sort the metric key names before printing. As an example: List<String> keys = new ArrayList<String>(getCounterMap().keySet()); Collections.sort(keys); for (String key : keys) { ... } 4. Nit: minor indentation issue in the new MonitoredCounterGroup.getName() method (but you may not need to expose that if the impl is there) 5. It would be great to also post this patch on https://reviews.apache.org/ per https://cwiki.apache.org/confluence/display/FLUME/How+to+Contribute#HowtoContribute-ProvidingPatches - use group Flume - and cross-link them. See other reviews at https://reviews.apache.org/groups/Flume/ as an example Thanks!
          Hide
          Mike Percy added a comment -

          Hi Israel, any update on this one?

          Thanks,
          Mike

          Show
          Mike Percy added a comment - Hi Israel, any update on this one? Thanks, Mike
          Hide
          Israel Ekpo added a comment -

          I started working on them last weekend.

          I should have a patch ready for review within the next 3 days.

          I think this issue will supersede FLUME-1957 when it is resolved and closed.

          Show
          Israel Ekpo added a comment - I started working on them last weekend. I should have a patch ready for review within the next 3 days. I think this issue will supersede FLUME-1957 when it is resolved and closed.
          Hide
          Mike Percy added a comment -

          Sounds great!

          Show
          Mike Percy added a comment - Sounds great!
          Hide
          Israel Ekpo added a comment -

          I plan to finish this either tonight or tomorrow night.

          I will put up the patch for review.

          Show
          Israel Ekpo added a comment - I plan to finish this either tonight or tomorrow night. I will put up the patch for review.
          Hide
          Israel Ekpo added a comment -

          This is the updated patch.

          Show
          Israel Ekpo added a comment - This is the updated patch.
          Israel Ekpo made changes -
          Attachment FLUME-1940.2.patch [ 12578157 ]
          Hide
          Israel Ekpo added a comment -

          The review is available here

          https://reviews.apache.org/r/10416/

          Show
          Israel Ekpo added a comment - The review is available here https://reviews.apache.org/r/10416/
          Israel Ekpo made changes -
          Due Date 20/Apr/13
          Mike Percy made changes -
          Remote Link This issue links to "Review Board (Web Link)" [ 12150 ]
          Hide
          Mike Percy added a comment -

          Temporarily cancelling patch available due to RB feedback

          Show
          Mike Percy added a comment - Temporarily cancelling patch available due to RB feedback
          Mike Percy made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Mike Percy made changes -
          Link This issue is duplicated by FLUME-1957 [ FLUME-1957 ]
          Mike Percy made changes -
          Link This issue relates to FLUME-1957 [ FLUME-1957 ]
          Israel Ekpo made changes -
          Attachment FLUME-1940.3.patch [ 12579971 ]
          Israel Ekpo made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          Israel Ekpo added a comment -

          The updated patch has been submitted for review

          Show
          Israel Ekpo added a comment - The updated patch has been submitted for review
          Israel Ekpo made changes -
          Status In Progress [ 3 ] Patch Available [ 10002 ]
          Israel Ekpo made changes -
          Attachment FLUME-1940.20130424.patch [ 12580267 ]
          Hide
          Mike Percy added a comment -

          +1

          Show
          Mike Percy added a comment - +1
          Hide
          Mike Percy added a comment -

          Thanks for the patch Israel! Pushed to trunk and flume-1.4 branches.

          Show
          Mike Percy added a comment - Thanks for the patch Israel! Pushed to trunk and flume-1.4 branches.
          Mike Percy made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Hudson added a comment -

          Integrated in flume-trunk #399 (See https://builds.apache.org/job/flume-trunk/399/)
          FLUME-1940. Log a snapshot of Flume metrics on shutdown. (Revision 215d75eb15362f34cd3246107cbea7ace247af10)

          Result = UNSTABLE
          mpercy : http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=215d75eb15362f34cd3246107cbea7ace247af10
          Files :

          • flume-ng-core/src/main/java/org/apache/flume/instrumentation/MonitoredCounterGroup.java
          Show
          Hudson added a comment - Integrated in flume-trunk #399 (See https://builds.apache.org/job/flume-trunk/399/ ) FLUME-1940 . Log a snapshot of Flume metrics on shutdown. (Revision 215d75eb15362f34cd3246107cbea7ace247af10) Result = UNSTABLE mpercy : http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=215d75eb15362f34cd3246107cbea7ace247af10 Files : flume-ng-core/src/main/java/org/apache/flume/instrumentation/MonitoredCounterGroup.java
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Patch Available Patch Available
          14d 9h 23m 1 Israel Ekpo 26/Mar/13 04:24
          Patch Available Patch Available Open Open
          22d 16h 15m 1 Mike Percy 17/Apr/13 21:40
          Open Open In Progress In Progress
          5d 7h 39m 1 Israel Ekpo 23/Apr/13 05:20
          In Progress In Progress Patch Available Patch Available
          1d 6h 57m 1 Israel Ekpo 24/Apr/13 12:17
          Patch Available Patch Available Resolved Resolved
          1d 21h 36m 1 Mike Percy 26/Apr/13 09:54

            People

            • Assignee:
              Israel Ekpo
              Reporter:
              Mike Percy
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Due:
                Created:
                Updated:
                Resolved:

                Development