You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Israel Ekpo <is...@aicer.org> on 2013/04/11 06:44:38 UTC

Review Request: FLUME-1940, FLUME-1957 - Logging Snapshots of Flume Metrics on Shutdown

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10416/
-----------------------------------------------------------

Review request for Flume.


Description
-------

Logging Metrics from the stop() method for the following components: ChannelCounter, ChannelProcessorCounter, SinkCounter, SinkProcessorCounter, SourceCounter


This addresses bugs FLUME-1940 and FLUME-1957.
    https://issues.apache.org/jira/browse/FLUME-1940
    https://issues.apache.org/jira/browse/FLUME-1957


Diffs
-----

  flume-ng-core/src/main/java/org/apache/flume/instrumentation/MonitoredCounterGroup.java 502fe9e 

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


Testing
-------


Thanks,

Israel Ekpo


Re: Review Request: FLUME-1940, FLUME-1957 - Logging Snapshots of Flume Metrics on Shutdown

Posted by Israel Ekpo <is...@aicer.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10416/#review19250
-----------------------------------------------------------


I think instead of adding the start and stop times for the components in the map I will just dump them separately and then proceed with sorting the orignal keys in the map only.

I will also add a check to make sure the value exists as you recommended.

Thanks for the feedback.

- Israel Ekpo


On April 11, 2013, 4:44 a.m., Israel Ekpo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10416/
> -----------------------------------------------------------
> 
> (Updated April 11, 2013, 4:44 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> Logging Metrics from the stop() method for the following components: ChannelCounter, ChannelProcessorCounter, SinkCounter, SinkProcessorCounter, SourceCounter
> 
> 
> This addresses bugs FLUME-1940 and FLUME-1957.
>     https://issues.apache.org/jira/browse/FLUME-1940
>     https://issues.apache.org/jira/browse/FLUME-1957
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/instrumentation/MonitoredCounterGroup.java 502fe9e 
> 
> Diff: https://reviews.apache.org/r/10416/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Israel Ekpo
> 
>


Re: Review Request: FLUME-1940, FLUME-1957 - Logging Snapshots of Flume Metrics on Shutdown

Posted by Mike Percy <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10416/#review19757
-----------------------------------------------------------

Ship it!


+1, I tested this and it works great!

- Mike Percy


On April 24, 2013, 11:16 a.m., Israel Ekpo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10416/
> -----------------------------------------------------------
> 
> (Updated April 24, 2013, 11:16 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> Logging Metrics from the stop() method for the following components: ChannelCounter, ChannelProcessorCounter, SinkCounter, SinkProcessorCounter, SourceCounter
> 
> 
> This addresses bugs FLUME-1940 and FLUME-1957.
>     https://issues.apache.org/jira/browse/FLUME-1940
>     https://issues.apache.org/jira/browse/FLUME-1957
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/instrumentation/MonitoredCounterGroup.java 502fe9e 
> 
> Diff: https://reviews.apache.org/r/10416/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Israel Ekpo
> 
>


Re: Review Request: FLUME-1940, FLUME-1957 - Logging Snapshots of Flume Metrics on Shutdown

Posted by Israel Ekpo <is...@aicer.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10416/
-----------------------------------------------------------

(Updated April 24, 2013, 11:16 a.m.)


Review request for Flume.


Changes
-------

Attaching changes made based on feedback from code review comments.

I also realized that the original map was not modified and this was why the error was being thrown.

In this new patch the stop and start times are printed directly and the remaining keys are printed as is from the map.


Description
-------

Logging Metrics from the stop() method for the following components: ChannelCounter, ChannelProcessorCounter, SinkCounter, SinkProcessorCounter, SourceCounter


This addresses bugs FLUME-1940 and FLUME-1957.
    https://issues.apache.org/jira/browse/FLUME-1940
    https://issues.apache.org/jira/browse/FLUME-1957


Diffs (updated)
-----

  flume-ng-core/src/main/java/org/apache/flume/instrumentation/MonitoredCounterGroup.java 502fe9e 

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


Testing
-------


Thanks,

Israel Ekpo


Re: Review Request: FLUME-1940, FLUME-1957 - Logging Snapshots of Flume Metrics on Shutdown

Posted by Israel Ekpo <is...@aicer.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10416/
-----------------------------------------------------------

(Updated April 23, 2013, 4:21 a.m.)


Review request for Flume.


Changes
-------

Made changes from feedback in code reviews.


Description
-------

Logging Metrics from the stop() method for the following components: ChannelCounter, ChannelProcessorCounter, SinkCounter, SinkProcessorCounter, SourceCounter


This addresses bugs FLUME-1940 and FLUME-1957.
    https://issues.apache.org/jira/browse/FLUME-1940
    https://issues.apache.org/jira/browse/FLUME-1957


Diffs (updated)
-----

  flume-ng-core/src/main/java/org/apache/flume/instrumentation/MonitoredCounterGroup.java 502fe9e 

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


Testing
-------


Thanks,

Israel Ekpo


Re: Review Request: FLUME-1940, FLUME-1957 - Logging Snapshots of Flume Metrics on Shutdown

Posted by Israel Ekpo <is...@aicer.org>.

> On April 16, 2013, 7:10 a.m., Mike Percy wrote:
> > Looks good overall. However I think there is an issue where there are certain assumptions made about keys existing when they may not be set.
> > I am getting a NullPointerException at stop() time (when I kill Flume via Ctrl-C) when running with this config file:
> > 
> > agent.sources = seqGenSrc
> > agent.channels = memoryChannel
> > agent.sinks = nullSink
> > 
> > agent.channels.memoryChannel.type = memory
> > agent.channels.memoryChannel.capacity = 100
> > 
> > agent.sources.seqGenSrc.type = seq
> > agent.sources.seqGenSrc.channels = memoryChannel
> > 
> > agent.sinks.nullSink.type = null
> > agent.sinks.nullSink.channel = memoryChannel
> > 
> > I tweaked the patch a little to validate (basically using Preconditions.checkNotNull(counterMap.get(counter), ...)) to get a helpful error message for this and I see the following:
> > 
> > Exception in thread "agent-shutdown-hook" java.lang.NullPointerException: Unknown counter: channel.start.time
> > 	at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:204)
> > 	at org.apache.flume.instrumentation.MonitoredCounterGroup.get(MonitoredCounterGroup.java:192)
> > 	at org.apache.flume.instrumentation.MonitoredCounterGroup.stop(MonitoredCounterGroup.java:153)
> > 	at org.apache.flume.channel.MemoryChannel.stop(MemoryChannel.java:355)
> > 	at org.apache.flume.lifecycle.LifecycleSupervisor.stop(LifecycleSupervisor.java:106)
> > 	at org.apache.flume.node.Application.stop(Application.java:92)
> > 	at org.apache.flume.node.Application$1.run(Application.java:302)

I will be posting an updated patch soon.

channel.start.time and channel.stop.time are artificial keys I just added to a copy of the orignal map.


- Israel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10416/#review19246
-----------------------------------------------------------


On April 11, 2013, 4:44 a.m., Israel Ekpo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10416/
> -----------------------------------------------------------
> 
> (Updated April 11, 2013, 4:44 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> Logging Metrics from the stop() method for the following components: ChannelCounter, ChannelProcessorCounter, SinkCounter, SinkProcessorCounter, SourceCounter
> 
> 
> This addresses bugs FLUME-1940 and FLUME-1957.
>     https://issues.apache.org/jira/browse/FLUME-1940
>     https://issues.apache.org/jira/browse/FLUME-1957
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/instrumentation/MonitoredCounterGroup.java 502fe9e 
> 
> Diff: https://reviews.apache.org/r/10416/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Israel Ekpo
> 
>


Re: Review Request: FLUME-1940, FLUME-1957 - Logging Snapshots of Flume Metrics on Shutdown

Posted by Mike Percy <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10416/#review19246
-----------------------------------------------------------


Looks good overall. However I think there is an issue where there are certain assumptions made about keys existing when they may not be set.
I am getting a NullPointerException at stop() time (when I kill Flume via Ctrl-C) when running with this config file:

agent.sources = seqGenSrc
agent.channels = memoryChannel
agent.sinks = nullSink

agent.channels.memoryChannel.type = memory
agent.channels.memoryChannel.capacity = 100

agent.sources.seqGenSrc.type = seq
agent.sources.seqGenSrc.channels = memoryChannel

agent.sinks.nullSink.type = null
agent.sinks.nullSink.channel = memoryChannel

I tweaked the patch a little to validate (basically using Preconditions.checkNotNull(counterMap.get(counter), ...)) to get a helpful error message for this and I see the following:

Exception in thread "agent-shutdown-hook" java.lang.NullPointerException: Unknown counter: channel.start.time
	at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:204)
	at org.apache.flume.instrumentation.MonitoredCounterGroup.get(MonitoredCounterGroup.java:192)
	at org.apache.flume.instrumentation.MonitoredCounterGroup.stop(MonitoredCounterGroup.java:153)
	at org.apache.flume.channel.MemoryChannel.stop(MemoryChannel.java:355)
	at org.apache.flume.lifecycle.LifecycleSupervisor.stop(LifecycleSupervisor.java:106)
	at org.apache.flume.node.Application.stop(Application.java:92)
	at org.apache.flume.node.Application$1.run(Application.java:302)

- Mike Percy


On April 11, 2013, 4:44 a.m., Israel Ekpo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10416/
> -----------------------------------------------------------
> 
> (Updated April 11, 2013, 4:44 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> Logging Metrics from the stop() method for the following components: ChannelCounter, ChannelProcessorCounter, SinkCounter, SinkProcessorCounter, SourceCounter
> 
> 
> This addresses bugs FLUME-1940 and FLUME-1957.
>     https://issues.apache.org/jira/browse/FLUME-1940
>     https://issues.apache.org/jira/browse/FLUME-1957
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/instrumentation/MonitoredCounterGroup.java 502fe9e 
> 
> Diff: https://reviews.apache.org/r/10416/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Israel Ekpo
> 
>