Re: Review Request 53297: Initial version of adding metrics into samza rest.

2016-11-10 Thread Prateek Maheshwari

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


Ship it!




Ship It!

- Prateek Maheshwari


On Nov. 10, 2016, 4:22 p.m., Shanthoosh Venkataraman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53297/
> ---
> 
> (Updated Nov. 10, 2016, 4:22 p.m.)
> 
> 
> Review request for samza and Jake Maes.
> 
> 
> Repository: samza
> 
> 
> Description
> ---
> 
> This patch aims at enabling users to define custom reporters to send metrics 
> from the monitors. Configurations required for the definition of the metrics 
> reporters follows the same convention as of the samza jobs.
> 
> 
> Diffs
> -
> 
>   docs/learn/documentation/versioned/rest/monitors.md 
> 46678bbe5fed99f767c3324dc9578ee1a64cec66 
>   samza-core/src/main/java/org/apache/samza/util/MetricsReporterLoader.java 
> PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 
> e0468ee89c89fd720834461771ebb36475475bcb 
>   
> samza-core/src/main/scala/org/apache/samza/metrics/ContainerProcessManagerMetrics.scala
>  f24beb1e099dd44b15b475e0a4a7f70560c6965e 
>   samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java 
> 2a3e83a24a5343bb53b93fc9d0a647c1b253714b 
>   samza-rest/src/test/java/org/apache/samza/rest/TestSamzaRestService.java 
> PRE-CREATION 
>   
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala
>  8a5b4aaea6e11a5af999f12d50e5b6135dbc70ca 
> 
> Diff: https://reviews.apache.org/r/53297/diff/
> 
> 
> Testing
> ---
> 
> Unit tests are done to verify the intended functionality.
> 
> 
> Thanks,
> 
> Shanthoosh Venkataraman
> 
>



Re: Review Request 53297: Initial version of adding metrics into samza rest.

2016-11-10 Thread Shanthoosh Venkataraman

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

(Updated Nov. 11, 2016, 12:22 a.m.)


Review request for samza and Jake Maes.


Repository: samza


Description
---

This patch aims at enabling users to define custom reporters to send metrics 
from the monitors. Configurations required for the definition of the metrics 
reporters follows the same convention as of the samza jobs.


Diffs (updated)
-

  docs/learn/documentation/versioned/rest/monitors.md 
46678bbe5fed99f767c3324dc9578ee1a64cec66 
  samza-core/src/main/java/org/apache/samza/util/MetricsReporterLoader.java 
PRE-CREATION 
  samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 
e0468ee89c89fd720834461771ebb36475475bcb 
  
samza-core/src/main/scala/org/apache/samza/metrics/ContainerProcessManagerMetrics.scala
 f24beb1e099dd44b15b475e0a4a7f70560c6965e 
  samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java 
2a3e83a24a5343bb53b93fc9d0a647c1b253714b 
  samza-rest/src/test/java/org/apache/samza/rest/TestSamzaRestService.java 
PRE-CREATION 
  
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala 
8a5b4aaea6e11a5af999f12d50e5b6135dbc70ca 

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


Testing
---

Unit tests are done to verify the intended functionality.


Thanks,

Shanthoosh Venkataraman



Re: Review Request 53297: Initial version of adding metrics into samza rest.

2016-11-10 Thread Shanthoosh Venkataraman


> On Nov. 5, 2016, 6:47 p.m., Jake Maes wrote:
> > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java, line 
> > 75
> > 
> >
> > I don't think the MetricsConfig constructure takes a subset. 
> > 
> > I think it takes the root and expects to find the "metrics" prefix
> 
> Jake Maes wrote:
> s/constructure/constructor
> 
> phonetic brain fail
> 
> Shanthoosh Venkataraman wrote:
> Yes, that's true. It expects the root and finds the metrics prefix. 
> Hence, stripPrefix is passed on as false, so that prefix isn't removed. This 
> just selects the config subset that starts with METRICS_PREFIX, without 
> removing the prefix. The goal is to not pass on the entire config object and 
> pass only metrics related config into MetricsConfig constructor.
> 
> Prateek Maheshwari wrote:
> Regarding the goal: That's not how the XConfig classes are intended to be 
> used, so it's not really necessary to do this. For example, see the implicit 
> conversion b/w Config and XConfig. Prefer passing in config.

Done.


- Shanthoosh


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


On Nov. 10, 2016, 1:04 a.m., Shanthoosh Venkataraman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53297/
> ---
> 
> (Updated Nov. 10, 2016, 1:04 a.m.)
> 
> 
> Review request for samza and Jake Maes.
> 
> 
> Repository: samza
> 
> 
> Description
> ---
> 
> This patch aims at enabling users to define custom reporters to send metrics 
> from the monitors. Configurations required for the definition of the metrics 
> reporters follows the same convention as of the samza jobs.
> 
> 
> Diffs
> -
> 
>   docs/learn/documentation/versioned/rest/monitors.md 
> 46678bbe5fed99f767c3324dc9578ee1a64cec66 
>   samza-core/src/main/java/org/apache/samza/util/MetricsReporterLoader.java 
> PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 
> e0468ee89c89fd720834461771ebb36475475bcb 
>   
> samza-core/src/main/scala/org/apache/samza/metrics/ContainerProcessManagerMetrics.scala
>  f24beb1e099dd44b15b475e0a4a7f70560c6965e 
>   samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java 
> 47b0663637f6db187d86961377ee3ee203b73fdb 
>   samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java 
> 2a3e83a24a5343bb53b93fc9d0a647c1b253714b 
>   samza-rest/src/test/java/org/apache/samza/rest/TestSamzaRestService.java 
> PRE-CREATION 
>   
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala
>  8a5b4aaea6e11a5af999f12d50e5b6135dbc70ca 
> 
> Diff: https://reviews.apache.org/r/53297/diff/
> 
> 
> Testing
> ---
> 
> Unit tests are done to verify the intended functionality.
> 
> 
> Thanks,
> 
> Shanthoosh Venkataraman
> 
>



Re: Review Request 53297: Initial version of adding metrics into samza rest.

2016-11-10 Thread Shanthoosh Venkataraman


> On Nov. 10, 2016, 6:57 p.m., Prateek Maheshwari wrote:
> > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java, line 27
> > 
> >
> > Unused import.

Removed.


> On Nov. 10, 2016, 6:57 p.m., Prateek Maheshwari wrote:
> > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java, line 
> > 164
> > 
> >
> > debug?

I think this should be info. It will be useful to know the registered reporters 
while debugging.


> On Nov. 10, 2016, 6:57 p.m., Prateek Maheshwari wrote:
> > samza-core/src/main/java/org/apache/samza/util/MetricsReporterLoader.java, 
> > line 39
> > 
> >
> > I meant remove the whole javadoc including @params. There's no useful 
> > additional information here.

Removed.


> On Nov. 10, 2016, 6:57 p.m., Prateek Maheshwari wrote:
> > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java, line 
> > 107
> > 
> >
> > Should this be called during start()? Same for stop().

To accomplish this, both(SamzaRestService,SamzaMonitorService) of them had to 
be coupled. Logically they are seperate entities. One responsible for resources 
and other monitors. Currently MonitorService & RestService are not coupled with 
each other. It would be useful keep them seperate(In the future, we could have 
SamzaRestService and SamzaMonitorService deployed seperately when we have too 
many(Monitors & Resources) in them). I would prefer not to have this coupling.


> On Nov. 10, 2016, 6:57 p.m., Prateek Maheshwari wrote:
> > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala,
> >  line 56
> > 
> >
> > Maybe use JavaCoverters._ and .asScala?

Done.


> On Nov. 10, 2016, 6:57 p.m., Prateek Maheshwari wrote:
> > samza-core/src/main/scala/org/apache/samza/metrics/ContainerProcessManagerMetrics.scala,
> >  line 46
> > 
> >
> > Maybe use JavaCoverters._ and .asScala?

Done.


> On Nov. 10, 2016, 6:57 p.m., Prateek Maheshwari wrote:
> > samza-core/src/main/java/org/apache/samza/util/MetricsReporterLoader.java, 
> > line 30
> > 
> >
> > Remove "contains reusable methods which" and "based upon 
> > configuration". Don't mind removing the whole comment either.

Removed.


> On Nov. 10, 2016, 6:57 p.m., Prateek Maheshwari wrote:
> > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java, line 
> > 103
> > 
> >
> > Don't understand this interface. What's a SchedulingProvider, and how 
> > is it different than a ScheduledExecutorService?
> > 
> > schedulingProvider should be probably be stopped in stop() in this 
> > class and not in SamzaMonitorService since this class owns it. Either that, 
> > or move instantiation to SamzaMonitorService too.

SchedulingProvider interface looks similar to that of ExecutorService. Not sure 
of the value add this interface provides. I guess, this is not required and we 
could just use ExeuctorService. Added a jira for it, since it's not in the 
scope of this patch. We could re-visit it later.


> On Nov. 10, 2016, 6:57 p.m., Prateek Maheshwari wrote:
> > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java, line 
> > 166
> > 
> >
> > debug?

I think this should be info. Will be useful to know this when debugging.


> On Nov. 10, 2016, 6:57 p.m., Prateek Maheshwari wrote:
> > docs/learn/documentation/versioned/rest/monitors.md, line 98
> > 
> >
> > Maybe: "are the same as that of Samza jobs." and make "that of Samza 
> > jobs" the configuration link and remove next line?

Done.


> On Nov. 10, 2016, 6:57 p.m., Prateek Maheshwari wrote:
> > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java, line 
> > 91
> > 
> >
> > See previous comment about just passing the entire config. Does that 
> > not work?

Done.


- Shanthoosh


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


On Nov. 10, 2016, 1:04 a.m., Shanthoosh Venkataraman wrote:
> 
> ---
> This is an automatically generated e-mail. 

Re: SAMZA-469 - scala 2.11 / move to java?

2016-11-10 Thread Navina Ramesh
Hi Thunder,
Sorry about the late response. I was refreshing my memory of SAMZA-469 :)

Is there something preventing this from getting into a release?
> Seems to me like the main roadblock on upgrading to scala 2.11 was
related to JDK6. We have already moved away from JDK6 and JDK7. So, I don't
see anything blocking us from upgrading to scala 2.11, if someone can
invest some time and push it out the door :)

There is mention in that issue of going away from scala altogether.  Is
there another issue which details this? When would that happen?
> By going away, I believe we decided to not add more scala code/dependency
to the code base so that eventually, we can replace them all with jdk based
lambdas. There is no concrete plan on when and how this will happen. So,
imo, it wouldn't be anytime soon :D

if some effort is needed to make the move to 2.11, we would be glad to help
out!
> That will be much appreciated! I don't think we have seen any strong
motivation for doing this within LinkedIn. If you guys can help us upgrade,
that will be awesome!!

Thanks!
Navina

On Mon, Nov 7, 2016 at 11:30 AM, Thunder Stumpges 
wrote:

> Hi all,
>
> We are looking to get our projects upgraded to scala 2.11 as some of our
> dependencies are dropping their compiling of 2.10 libraries. We found
> SAMZA-469 which looks to have had a patch for some time, however the last
> couple comments seem pretty shaky about when (or if!?) support will be
> available.
>
> Is there something preventing this from getting into a release?
>
> There is mention in that issue of going away from scala altogether.  Is
> there another issue which details this? When would that happen?
>
> Thanks, and if some effort is needed to make the move to 2.11, we would be
> glad to help out!
> Thunder Stumges
>
>


-- 
Navina R.


Re: Review Request 53453: Add optional interface for SystemConsumer checkpontListener() for checkpoint notifications

2016-11-10 Thread Boris Shkolnik

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

(Updated Nov. 10, 2016, 6:58 p.m.)


Review request for samza.


Bugs: SAMZA-1042
https://issues.apache.org/jira/browse/SAMZA-1042


Repository: samza


Description
---

Add optional interface for SystemConsumer checkpontListener() for checkpoint 
notifications.


Diffs (updated)
-

  samza-api/src/main/java/org/apache/samza/checkpoint/CheckpointListener.java 
PRE-CREATION 
  samza-api/src/main/java/org/apache/samza/system/SystemConsumer.java 
8dfcc7499659442aabd3085a8787475fe38f29db 
  samza-core/src/main/scala/org/apache/samza/checkpoint/OffsetManager.scala 
c41eadb70f4675816245f7ac40f0db2fc16335f0 
  samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 
e0468ee89c89fd720834461771ebb36475475bcb 
  samza-core/src/test/scala/org/apache/samza/checkpoint/TestOffsetManager.scala 
cb78223f1b59a78bbeb1e42b5974670a53def504 

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


Testing
---

gradlew test.
manual testing with LiKafkaConsumer.


Thanks,

Boris Shkolnik



Re: Review Request 53297: Initial version of adding metrics into samza rest.

2016-11-10 Thread Prateek Maheshwari

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



Looks pretty good to me. Some final minor comments and questions.


docs/learn/documentation/versioned/rest/monitors.md (line 98)


Maybe: "are the same as that of Samza jobs." and make "that of Samza jobs" 
the configuration link and remove next line?



samza-core/src/main/java/org/apache/samza/util/MetricsReporterLoader.java (line 
30)


Remove "contains reusable methods which" and "based upon configuration". 
Don't mind removing the whole comment either.



samza-core/src/main/java/org/apache/samza/util/MetricsReporterLoader.java (line 
39)


I meant remove the whole javadoc including @params. There's no useful 
additional information here.



samza-core/src/main/java/org/apache/samza/util/MetricsReporterLoader.java (line 
52)


Minor, your call: Not a big fan of the "hanging towers of parameters" code 
style. Maybe extract variable?



samza-core/src/main/scala/org/apache/samza/metrics/ContainerProcessManagerMetrics.scala
 (line 45)


Maybe use JavaCoverters._ and .asScala?



samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java (line 27)


Unused import.



samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java (line 91)


See previous comment about just passing the entire config. Does that not 
work?



samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java (line 103)


Don't understand this interface. What's a SchedulingProvider, and how is it 
different than a ScheduledExecutorService?

schedulingProvider should be probably be stopped in stop() in this class 
and not in SamzaMonitorService since this class owns it. Either that, or move 
instantiation to SamzaMonitorService too.



samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java (line 107)


Should this be called during start()? Same for stop().



samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java (line 164)


debug?



samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java (line 166)


debug?



samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala 
(line 46)


Maybe use JavaCoverters._ and .asScala?


- Prateek Maheshwari


On Nov. 9, 2016, 5:04 p.m., Shanthoosh Venkataraman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53297/
> ---
> 
> (Updated Nov. 9, 2016, 5:04 p.m.)
> 
> 
> Review request for samza and Jake Maes.
> 
> 
> Repository: samza
> 
> 
> Description
> ---
> 
> This patch aims at enabling users to define custom reporters to send metrics 
> from the monitors. Configurations required for the definition of the metrics 
> reporters follows the same convention as of the samza jobs.
> 
> 
> Diffs
> -
> 
>   docs/learn/documentation/versioned/rest/monitors.md 
> 46678bbe5fed99f767c3324dc9578ee1a64cec66 
>   samza-core/src/main/java/org/apache/samza/util/MetricsReporterLoader.java 
> PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 
> e0468ee89c89fd720834461771ebb36475475bcb 
>   
> samza-core/src/main/scala/org/apache/samza/metrics/ContainerProcessManagerMetrics.scala
>  f24beb1e099dd44b15b475e0a4a7f70560c6965e 
>   samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java 
> 47b0663637f6db187d86961377ee3ee203b73fdb 
>   samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java 
> 2a3e83a24a5343bb53b93fc9d0a647c1b253714b 
>   samza-rest/src/test/java/org/apache/samza/rest/TestSamzaRestService.java 
> PRE-CREATION 
>   
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala
>  8a5b4aaea6e11a5af999f12d50e5b6135dbc70ca 
> 
> Diff: https://reviews.apache.org/r/53297/diff/
> 
> 
> Testing
> ---
> 
> Unit tests are done to verify the intended functionality.
> 
> 
> Thanks,
> 
> Shanthoosh Venkataraman
> 
>



Re: Review Request 53453: Add optional interface for SystemConsumer checkpontListener() for checkpoint notifications

2016-11-10 Thread Boris Shkolnik

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

(Updated Nov. 10, 2016, 6:54 p.m.)


Review request for samza.


Bugs: SAMZA-1042
https://issues.apache.org/jira/browse/SAMZA-1042


Repository: samza


Description
---

Add optional interface for SystemConsumer checkpontListener() for checkpoint 
notifications.


Diffs
-

  samza-api/src/main/java/org/apache/samza/checkpoint/CheckpointListener.java 
PRE-CREATION 
  samza-api/src/main/java/org/apache/samza/system/SystemConsumer.java 
8dfcc7499659442aabd3085a8787475fe38f29db 
  samza-core/src/main/scala/org/apache/samza/checkpoint/OffsetManager.scala 
c41eadb70f4675816245f7ac40f0db2fc16335f0 
  samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 
e0468ee89c89fd720834461771ebb36475475bcb 
  samza-core/src/test/scala/org/apache/samza/checkpoint/TestOffsetManager.scala 
cb78223f1b59a78bbeb1e42b5974670a53def504 

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


Testing (updated)
---

gradlew test.
manual testing with LiKafkaConsumer.


Thanks,

Boris Shkolnik



Re: Review Request 53453: Add optional interface for SystemConsumer checkpontListener() for checkpoint notifications

2016-11-10 Thread Boris Shkolnik


> On Nov. 9, 2016, 8:48 p.m., Navina Ramesh wrote:
> > Is there any practical implementation of this interface . eg. adding to 
> > kafkasystemconsumer? Or is it expected in the future?

I have used it with LiKafka for manual testing, but nothing to check in..


- Boris


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


On Nov. 4, 2016, 11:23 p.m., Boris Shkolnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53453/
> ---
> 
> (Updated Nov. 4, 2016, 11:23 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-1042
> https://issues.apache.org/jira/browse/SAMZA-1042
> 
> 
> Repository: samza
> 
> 
> Description
> ---
> 
> Add optional interface for SystemConsumer checkpontListener() for checkpoint 
> notifications.
> 
> 
> Diffs
> -
> 
>   samza-api/src/main/java/org/apache/samza/checkpoint/CheckpointListener.java 
> PRE-CREATION 
>   samza-api/src/main/java/org/apache/samza/system/SystemConsumer.java 
> 8dfcc7499659442aabd3085a8787475fe38f29db 
>   samza-core/src/main/scala/org/apache/samza/checkpoint/OffsetManager.scala 
> c41eadb70f4675816245f7ac40f0db2fc16335f0 
>   samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 
> e0468ee89c89fd720834461771ebb36475475bcb 
>   
> samza-core/src/test/scala/org/apache/samza/checkpoint/TestOffsetManager.scala 
> cb78223f1b59a78bbeb1e42b5974670a53def504 
> 
> Diff: https://reviews.apache.org/r/53453/diff/
> 
> 
> Testing
> ---
> 
> gradlew test.
> manual testing.
> 
> 
> Thanks,
> 
> Boris Shkolnik
> 
>



Re: Review Request 53453: Add optional interface for SystemConsumer checkpontListener() for checkpoint notifications

2016-11-10 Thread Boris Shkolnik


> On Nov. 8, 2016, 11:52 p.m., Prateek Maheshwari wrote:
> > samza-core/src/main/scala/org/apache/samza/checkpoint/OffsetManager.scala, 
> > line 239
> > 
> >
> > Are you missing a foreach here?
> > 
> > I think you need something like:
> > lastProcessedOffsets.get(taskName)
> >   .foreach { sspToOffsets => sspToOffsets.foreach { case (ssp, 
> > checkpoint) => 
> > offsetManagerMetrics.checkpointedOffsets(ssp).set(checkpoint) } }
> > 
> > If the version above is correct, can we add a test for this? Its easy 
> > to miss.

Interesting, Scala was doing the right thing, although it is not clear why. I 
had some tests for it and they passed. 
So I rewrote this part to make it clearer.


> On Nov. 8, 2016, 11:52 p.m., Prateek Maheshwari wrote:
> > samza-core/src/main/scala/org/apache/samza/checkpoint/OffsetManager.scala, 
> > line 77
> > 
> >
> > Strongly prefer not adding empty Map() as default value here (and maybe 
> > clean up the other one too). See my comment on Xinyu's HDFS performance RB 
> > for explanation.

In general I agree with you, but in this case it is kind of hard to do it, 
because having a parameter without a default between parameters with default 
will confuse compiler when only some of the arguments are provided.


- Boris


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


On Nov. 4, 2016, 11:23 p.m., Boris Shkolnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53453/
> ---
> 
> (Updated Nov. 4, 2016, 11:23 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-1042
> https://issues.apache.org/jira/browse/SAMZA-1042
> 
> 
> Repository: samza
> 
> 
> Description
> ---
> 
> Add optional interface for SystemConsumer checkpontListener() for checkpoint 
> notifications.
> 
> 
> Diffs
> -
> 
>   samza-api/src/main/java/org/apache/samza/checkpoint/CheckpointListener.java 
> PRE-CREATION 
>   samza-api/src/main/java/org/apache/samza/system/SystemConsumer.java 
> 8dfcc7499659442aabd3085a8787475fe38f29db 
>   samza-core/src/main/scala/org/apache/samza/checkpoint/OffsetManager.scala 
> c41eadb70f4675816245f7ac40f0db2fc16335f0 
>   samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 
> e0468ee89c89fd720834461771ebb36475475bcb 
>   
> samza-core/src/test/scala/org/apache/samza/checkpoint/TestOffsetManager.scala 
> cb78223f1b59a78bbeb1e42b5974670a53def504 
> 
> Diff: https://reviews.apache.org/r/53453/diff/
> 
> 
> Testing
> ---
> 
> gradlew test.
> manual testing.
> 
> 
> Thanks,
> 
> Boris Shkolnik
> 
>