You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Sean Busbey <se...@manvsbeard.com> on 2014/01/22 20:13:04 UTC
Review Request 17192: ACCUMULO-2225 handle IllegalArgumentExceptions from
Hadoop on host lookup failure.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17192/
-----------------------------------------------------------
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
Re: Review Request 17192: ACCUMULO-2225 handle IllegalArgumentExceptions
from Hadoop on host lookup failure.
Posted by Bill Havanki <bh...@clouderagovt.com>.
> On Jan. 22, 2014, 3:50 p.m., Bill Havanki wrote:
> > src/server/src/main/java/org/apache/accumulo/server/Accumulo.java, line 229
> > <https://reviews.apache.org/r/17192/diff/1/?file=435500#file435500line229>
> >
> > You probably want a log statement here for the final UnknownHostException, noting that no more retries will be attempted or something to that effect.
>
> Sean Busbey wrote:
> oh right! I had meant to do that. at error sound fine?
Sure!
> On Jan. 22, 2014, 3: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?
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.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17192/#review32532
-----------------------------------------------------------
On Jan. 22, 2014, 4: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, 4: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
>
>
Re: Review Request 17192: ACCUMULO-2225 handle IllegalArgumentExceptions
from Hadoop on host lookup failure.
Posted by Sean Busbey <se...@manvsbeard.com>.
> 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.
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.
> On Jan. 22, 2014, 8:50 p.m., Bill Havanki wrote:
> > src/server/src/main/java/org/apache/accumulo/server/Accumulo.java, line 229
> > <https://reviews.apache.org/r/17192/diff/1/?file=435500#file435500line229>
> >
> > You probably want a log statement here for the final UnknownHostException, noting that no more retries will be attempted or something to that effect.
oh right! I had meant to do that. at error sound fine?
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17192/#review32532
-----------------------------------------------------------
On Jan. 22, 2014, 7:13 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, 7:13 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
>
>
Re: Review Request 17192: ACCUMULO-2225 handle IllegalArgumentExceptions
from Hadoop on host lookup failure.
Posted by Sean Busbey <se...@manvsbeard.com>.
> 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.
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?
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17192/#review32532
-----------------------------------------------------------
On Jan. 22, 2014, 7:13 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, 7:13 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
>
>
Re: Review Request 17192: ACCUMULO-2225 handle IllegalArgumentExceptions
from Hadoop on host lookup failure.
Posted by Sean Busbey <se...@manvsbeard.com>.
> 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
>
>
Re: Review Request 17192: ACCUMULO-2225 handle IllegalArgumentExceptions
from Hadoop on host lookup failure.
Posted by Bill Havanki <bh...@clouderagovt.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17192/#review32532
-----------------------------------------------------------
src/server/src/main/java/org/apache/accumulo/server/Accumulo.java
<https://reviews.apache.org/r/17192/#comment61412>
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.
src/server/src/main/java/org/apache/accumulo/server/Accumulo.java
<https://reviews.apache.org/r/17192/#comment61406>
You probably want a log statement here for the final UnknownHostException, noting that no more retries will be attempted or something to that effect.
src/server/src/main/java/org/apache/accumulo/server/Accumulo.java
<https://reviews.apache.org/r/17192/#comment61410>
wrappend -> wrapping
- Bill Havanki
On Jan. 22, 2014, 2:13 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, 2:13 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
>
>
Re: Review Request 17192: ACCUMULO-2225 handle IllegalArgumentExceptions
from Hadoop on host lookup failure.
Posted by Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17192/#review32548
-----------------------------------------------------------
src/server/src/main/java/org/apache/accumulo/server/Accumulo.java
<https://reviews.apache.org/r/17192/#comment61425>
Also, "Uknown"
src/server/src/main/java/org/apache/accumulo/server/Accumulo.java
<https://reviews.apache.org/r/17192/#comment61426>
There doesn't really need to be duplication, since IIRC IllegalArgument is already a RuntimeExceptione.
- Mike Drob
On Jan. 22, 2014, 7:13 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, 7:13 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
>
>
Re: Review Request 17192: ACCUMULO-2225 handle IllegalArgumentExceptions
from Hadoop on host lookup failure.
Posted by Bill Havanki <bh...@clouderagovt.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17192/#review32566
-----------------------------------------------------------
Ship it!
Ship It!
- Bill Havanki
On Jan. 22, 2014, 5:28 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, 5:28 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
>
>
Re: Review Request 17192: ACCUMULO-2225 handle IllegalArgumentExceptions
from Hadoop on host lookup failure.
Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17192/
-----------------------------------------------------------
(Updated Jan. 22, 2014, 10:28 p.m.)
Review request for accumulo and Bill Havanki.
Changes
-------
updated logs / comments based on feedback from bhavanki
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 (updated)
-----
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
Re: Review Request 17192: ACCUMULO-2225 handle IllegalArgumentExceptions
from Hadoop on host lookup failure.
Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
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.
Changes
-------
first round of modifications based on feedbakc.
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 (updated)
-----
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
Re: Review Request 17192: ACCUMULO-2225 handle IllegalArgumentExceptions
from Hadoop on host lookup failure.
Posted by Sean Busbey <se...@manvsbeard.com>.
> On Jan. 22, 2014, 8:54 p.m., Mike Drob wrote:
> > src/server/src/main/java/org/apache/accumulo/server/Accumulo.java, line 280
> > <https://reviews.apache.org/r/17192/diff/1/?file=435500#file435500line280>
> >
> > Why not catch IllegalArgumentException separately?
One downside of catching it separately is duplication of code between the two cases. unlike the case in ACCUMULO-2213, we're already grabbing everything in this catch.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17192/#review32537
-----------------------------------------------------------
On Jan. 22, 2014, 7:13 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, 7:13 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
>
>
Re: Review Request 17192: ACCUMULO-2225 handle IllegalArgumentExceptions
from Hadoop on host lookup failure.
Posted by Sean Busbey <se...@manvsbeard.com>.
> On Jan. 22, 2014, 8:54 p.m., Mike Drob wrote:
> > src/server/src/main/java/org/apache/accumulo/server/Accumulo.java, line 213
> > <https://reviews.apache.org/r/17192/diff/1/?file=435500#file435500line213>
> >
> > Should this be configurable?
>
> Sean Busbey wrote:
> It could be, but more configurable options are not necessarily useful for the user. The only time this would matter would be
>
> * if the user knew that had such a flaky network / dns that they needed to allow more chances
> * if the user knew they had such a stable dns set up that no retries are needed (but they still might misconfigure with the wrong host name)
>
> The latter seems unlikely. For the former, I think it's past the cost/benefit of working in the face of failure. At some point the there has to be a threshold of preferring to fix the underlying problem.
>
> Given the amount of complexity it would add (this method doesn't grab any configuration directly now, should the retry count be updateable via ZK or only static, etc), I don't think the knob is worth having.
Filed ACCUMULO-2241
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17192/#review32537
-----------------------------------------------------------
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
>
>
Re: Review Request 17192: ACCUMULO-2225 handle IllegalArgumentExceptions
from Hadoop on host lookup failure.
Posted by Sean Busbey <se...@manvsbeard.com>.
> On Jan. 22, 2014, 8:54 p.m., Mike Drob wrote:
> > src/server/src/main/java/org/apache/accumulo/server/Accumulo.java, line 213
> > <https://reviews.apache.org/r/17192/diff/1/?file=435500#file435500line213>
> >
> > Should this be configurable?
It could be, but more configurable options are not necessarily useful for the user. The only time this would matter would be
* if the user knew that had such a flaky network / dns that they needed to allow more chances
* if the user knew they had such a stable dns set up that no retries are needed (but they still might misconfigure with the wrong host name)
The latter seems unlikely. For the former, I think it's past the cost/benefit of working in the face of failure. At some point the there has to be a threshold of preferring to fix the underlying problem.
Given the amount of complexity it would add (this method doesn't grab any configuration directly now, should the retry count be updateable via ZK or only static, etc), I don't think the knob is worth having.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17192/#review32537
-----------------------------------------------------------
On Jan. 22, 2014, 7:13 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, 7:13 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
>
>
Re: Review Request 17192: ACCUMULO-2225 handle IllegalArgumentExceptions
from Hadoop on host lookup failure.
Posted by Sean Busbey <se...@manvsbeard.com>.
> On Jan. 22, 2014, 8:54 p.m., Mike Drob wrote:
> > src/server/src/main/java/org/apache/accumulo/server/Accumulo.java, line 280
> > <https://reviews.apache.org/r/17192/diff/1/?file=435500#file435500line280>
> >
> > Why not catch IllegalArgumentException separately?
>
> Sean Busbey wrote:
> One downside of catching it separately is duplication of code between the two cases. unlike the case in ACCUMULO-2213, we're already grabbing everything in this catch.
ex, presuming we're only trying to change the behavior for those IAE's that are wrapping UHE:
try {
Method setSafeMode = dfs.getClass().getMethod("setSafeMode", safeModeAction);
return (Boolean)setSafeMode.invoke(dfs, get);
} catch (IllegalArgumentException exception) {
/* Send the wrapped Unknown Host back as-is, so we can deal with it in the same place as the others */
if (exception.getCause() instanceof UnknownHostException) {
throw exception;
} else {
throw new RuntimeException("cannot find method setSafeMode", ex);
}
} catch (Exception ex) {
throw new RuntimeException("cannot find method setSafeMode", ex);
}
or should it just re-throw all IAEs as-is? that would be much simpler.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17192/#review32537
-----------------------------------------------------------
On Jan. 22, 2014, 7:13 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, 7:13 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
>
>
Re: Review Request 17192: ACCUMULO-2225 handle IllegalArgumentExceptions
from Hadoop on host lookup failure.
Posted by Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17192/#review32537
-----------------------------------------------------------
src/server/src/main/java/org/apache/accumulo/server/Accumulo.java
<https://reviews.apache.org/r/17192/#comment61413>
Should this be configurable?
src/server/src/main/java/org/apache/accumulo/server/Accumulo.java
<https://reviews.apache.org/r/17192/#comment61414>
nit: spelling "wrappend"
src/server/src/main/java/org/apache/accumulo/server/Accumulo.java
<https://reviews.apache.org/r/17192/#comment61415>
Why not catch IllegalArgumentException separately?
- Mike Drob
On Jan. 22, 2014, 7:13 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, 7:13 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
>
>