samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jake Maes <jma...@apache.org>
Subject Re: Review Request 52492: Adding monitor to clean up stale local stores of jobs/tasks.
Date Wed, 12 Oct 2016 15:43:42 GMT


> On Oct. 10, 2016, 7:27 p.m., Jake Maes wrote:
> > samza-rest/src/main/java/org/apache/samza/monitor/LocalStoreMonitor.java, line 84
> > <https://reviews.apache.org/r/52492/diff/2/?file=1527756#file1527756line84>
> >
> >     Shouldn't this be checked only once in the constructor?
> 
> Shanthoosh Venkataraman wrote:
>     Current monitor loading implementation doesn't load monitors that throws exceptions
during instantiation. If we throw up in the constructor, the monitor won't even load and user
may not be aware of the problem. If we don't validate in constructor, there's a good chance
that the user might notice the problem seeing lot of exceptions getting thrown from monitor
method.

Either way the user needs to see the exception in the log. In one case it's a single exception
and in the other it creates so much noise you can't see other problems in the log. I prefer
the former.


- Jake


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


On Oct. 11, 2016, 12:53 a.m., Shanthoosh Venkataraman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52492/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2016, 12:53 a.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> This patch contains the samza-rest monitor that periodically cleans up the stale local
stores of dead jobs/tasks. It performs the store deletion in two phases. Initially it deletes
the offset file in the local task stores if the following condition is true. ((jobIsNotRunning
|| preferedHost != nmHost) && offsetFilelastModifiedTime is greater than deleteRetention).
During the subsequent run, it deletes the local task stores if it does not contain offset
file. Please refer to the design doc of SAMZA-656 (https://issues.apache.org/jira/secure/attachment/12828083/DESIGN-SAMZA-656.pdf)
for more details.
> 
> 
> Diffs
> -----
> 
>   build.gradle 2bea27b75288d3103178bc3762b9556f6e69cdd1 
>   samza-rest/src/main/java/org/apache/samza/monitor/YarnLocalStoreMonitor.java PRE-CREATION

>   samza-rest/src/main/java/org/apache/samza/monitor/YarnLocalStoreMonitorConfig.java
PRE-CREATION 
>   samza-rest/src/main/java/org/apache/samza/monitor/YarnLocalStoreMonitorFactory.java
PRE-CREATION 
>   samza-rest/src/main/java/org/apache/samza/rest/resources/JobsResource.java a566db598c284d69ea61af88fdc0851483d5a089

>   samza-rest/src/main/java/org/apache/samza/rest/resources/ResourceConstants.java PRE-CREATION

>   samza-rest/src/test/java/org/apache/samza/monitor/TestYarnLocalStoreMonitor.java PRE-CREATION

> 
> Diff: https://reviews.apache.org/r/52492/diff/
> 
> 
> Testing
> -------
> 
> Unit testing and manual testing are done to verify the functionality.
> 
> 
> Thanks,
> 
> Shanthoosh Venkataraman
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message