> On Jan. 22, 2014, 8:50 p.m., Bill Havanki wrote:
> > src/server/src/main/java/org/apache/accumulo/server/Accumulo.java, line 226
> > <https://reviews.apache.org/r/17192/diff/1/?file=435500#file435500line226>
> >
> >     I think the doubling of the sleep period in the last statement of the 
> > encompassing while loop will interfere here. The first time through, sleep 
> > will be set to TTL + 1 seconds (assuming it's set to something valid). The 
> > next time through, that sleep will have been doubled and will be selected 
> > for the next period instead of TTL + 1 again, because the doubled value is 
> > bigger.
> >     
> >     Also, the TTL value is a loop invariant so you could lift it out.
> 
> Sean Busbey wrote:
>     is the interference from the doubling a problem? we just need to make 
> sure we don't ask for a host lookup and get a cached failure. Or should the 
> doubling not happen if we increased sleep based on this ttl, to avoid backing 
> off too far?
>     
>     any worry about the ttl changing between invocations by pulling it out of 
> the loop? It's not likely to change while running, but could.
> 
> Sean Busbey wrote:
>     Actually, not invariant because the source of the underlying 
> UnknownHostException could change in the loop. Acceptable cost of 
> deduplicating the "your ttl is infinite" error handling talked about on 
> ACCUMULO-2224's review?
> 
> Bill Havanki wrote:
>     The doubling isn't really a problem, but if we claim that we follow the 
> TTL, then as a user it would surprise me that we'd be doubling it between 
> checks. Documentation somewhere could cover it.
>     
>     I wouldn't care about the TTL changing between invocations.
>     
>     The TTL value is invariant, but the behavior of 
> getAddressCacheNegativeTtl isn't. This is where that refactoring creates a 
> problem. (Review for future reference is https://reviews.apache.org/r/17132). 
> I'm -0 on leaving it like it is.

do we claim to follow the TTL? I think this is just a matter of adjusting our 
back off window to make sure we aren't getting stale results.

should the existing log message about sleeping specify that it's a back-off 
mechanism?

something like

  log.info("Backing off due to failure. Current sleep window is " + sleep / 
1000. + " seconds");

would that make it more understandable as the value increases?


- Sean


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


On Jan. 22, 2014, 9:37 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17192/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2014, 9:37 p.m.)
> 
> 
> Review request for accumulo and Bill Havanki.
> 
> 
> Bugs: ACCUMULO-2225
>     https://issues.apache.org/jira/browse/ACCUMULO-2225
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> ACCUMULO-2225 handle IllegalArgumentExceptions from Hadoop on host lookup 
> failure.
>     
>     Looks for cases where we treat IOExceptions out of Hadoop specially, then 
> attempts to replicate for UnknownHostExceptions that have been wrapped in 
> IllegalArgumentExceptions.
> 
> 
> Diffs
> -----
> 
>   
> src/core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java 
> e02c19785c373170b2b0d47266a6988adeec7e17 
>   src/server/src/main/java/org/apache/accumulo/server/Accumulo.java 
> 253962bca5f2573fdfa87b82e145938e190842ed 
>   
> src/server/src/main/java/org/apache/accumulo/server/master/tableOps/DeleteTable.java
>  1c4d4ad8af30350d93f6dcfe2291463a90c835d3 
>   
> src/server/src/main/java/org/apache/accumulo/server/tabletserver/Compactor.java
>  29b8455a2dce5ecf2195d9704980e3e9749683f4 
>   
> src/server/src/main/java/org/apache/accumulo/server/test/randomwalk/security/SecurityHelper.java
>  c8d1ea0872564643342387373d85b5ac87a57540 
>   
> src/server/src/main/java/org/apache/accumulo/server/util/TabletOperations.java
>  4a39050a17f84293c52f09b62932be5b957c8a91 
> 
> Diff: https://reviews.apache.org/r/17192/diff/
> 
> 
> Testing
> -------
> 
> builds on both hadoop profiles. starting functional tests now in combination 
> with 17132
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>

Reply via email to