> On Oct. 27, 2015, 10:48 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java,
> >  line 167
> > <https://reviews.apache.org/r/39674/diff/1/?file=1108668#file1108668line167>
> >
> >     Maybe trace() or debug() - if the cache is enabled could this happen 
> > frequently on "bigger" clusters?  Maybe info() should be for when the new 
> > service flushes the cache on the X minute config value.

In theory it shouldn't really happen too often aside from after initial 
startup. But you're right, it's going to put a lot of INFO level stuff. I'll 
change this to DEBUG. The think there is an INFO level message when the flush 
is about to happen already.


> On Oct. 27, 2015, 10:48 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java,
> >  lines 1247-1249
> > <https://reviews.apache.org/r/39674/diff/1/?file=1108668#file1108668line1247>
> >
> >     Just curious, why no build() with args or private ctor for these 
> > variables (yay C# uppercase names)

A leftover from my Android days. Method invocation was expensive and Java 
should already have the encapsultion anti-pattern problem solved by this point 
(but they don't). I can make these ctor'd and exposed via gets ...


- Jonathan


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


On Oct. 26, 2015, 9:36 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39674/
> -----------------------------------------------------------
> 
> (Updated Oct. 26, 2015, 9:36 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jayush Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-13570
>     https://issues.apache.org/jira/browse/AMBARI-13570
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Alert-related SQL queries and updates are responsible for the majority of 
> calls to the database in most deployments. This is due to the fact that 
> alerts update their timestamp and latest text on every alert, regardless of 
> state change.
> 
> This patch introduces an optional, experimental feature where 
> `AlertCurrentEntity` instances can be cached. This produces several problems:
> - The cached alerts are never reflected in queries from JPA
> - The cached alerts are never persisted back to the database
> 
> We solve this by keeping the managed entities in memory and flushing them 
> periodically. The Alert DAO also augments the list from JPA with cached 
> entities.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/annotations/Experimental.java 
> 5a4915a 
>   
> ambari-server/src/main/java/org/apache/ambari/annotations/ExperimentalFeature.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
>  d482ce9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  04ab1ef 
>   
> ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertReceivedListener.java
>  e59f63e 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java 
> 0429534 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertCurrentEntity.java
>  66b2a83 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/services/CachedAlertFlushService.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java
>  573c02f 
> 
> Diff: https://reviews.apache.org/r/39674/diff/
> 
> 
> Testing
> -------
> 
> I have already files a Jira to track comprehensive tests: 
> https://issues.apache.org/jira/browse/AMBARI-13569
> 
> I know that this patch doesn't cover any of the new cached functionality in 
> testing, but that's mainly due to the short time constraints and the fact 
> that the feature is experimental and off by default.
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>

Reply via email to