[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

2018-03-13 Thread Todd Lipcon (Code Review) 
Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9489 )

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..

KUDU-2230: java: sync client exception stack traces should make sense

Previously the exceptions thrown by the synchronous client always had
a stack trace somewhere deep inside our async callback chain. So, when
the user eventually caught this exception, it would be very hard to even
know what part of their code failed, since their own code would not even
show up in the call chain.

This patch simply replaces the exception's stack trace with the stack at
the "join" point where the exception is transformed back to a
KuduException. An exception which stores the original stack trace is
attached as a "suppressed" exception.

Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Reviewed-on: http://gerrit.cloudera.org:8080/9489
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
Reviewed-by: Alexey Serbin 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java
2 files changed, 30 insertions(+), 6 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Dan Burkert: Looks good to me, but someone else must approve
  Alexey Serbin: Looks good to me, approved

--
To view, visit http://gerrit.cloudera.org:8080/9489
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

2018-03-13 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9489 )

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 2:

> Actually I just remembered why I didn't switch the cause. The cause of an 
> exception can only be set once using 'initCause' and not replaced later. So, 
> to use 'cause', we'd need to instantiate a new KuduException in the 
> transformException function.
>
> In order to do so, we'd have two options:
>
> a) use some reflection to figure out which KuduException subclass we had and 
> try to re-instantiate the same one. That seems ugly.
> b) use clone() and make sure that all KuduException subclasses are Cloneable. 
> Also kind of a pain, no?

Agreed. Both of those are bad news bears. Ship It.


--
To view, visit http://gerrit.cloudera.org:8080/9489
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 17:12:02 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

2018-03-12 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9489 )

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 2:

> Patch Set 2:
>
> > I was thinking that "suppressed" more accurately conveys that it's probably 
> > irrelevant to what the user should look at, vs a normal "cause". Happy to 
> > use cause if y'all think it is less strange.
>
> With "cause", do the stack traces become totally unwieldy?

Actually I just remembered why I didn't switch the cause. The cause of an 
exception can only be set once using 'initCause' and not replaced later. So, to 
use 'cause', we'd need to instantiate a new KuduException in the 
transformException function.

In order to do so, we'd have two options:

a) use some reflection to figure out which KuduException subclass we had and 
try to re-instantiate the same one. That seems ugly.
b) use clone() and make sure that all KuduException subclasses are Cloneable. 
Also kind of a pain, no?


--
To view, visit http://gerrit.cloudera.org:8080/9489
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:20:46 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

2018-03-12 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9489 )

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 2:

> I was thinking that "suppressed" more accurately conveys that it's probably 
> irrelevant to what the user should look at, vs a normal "cause". Happy to use 
> cause if y'all think it is less strange.

With "cause", do the stack traces become totally unwieldy?


--
To view, visit http://gerrit.cloudera.org:8080/9489
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:05:00 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

2018-03-12 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9489 )

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 2:

> Patch Set 2:
>
> Why did you choose to model the original exception as suppressed rather than 
> as the cause of the new exception? From my reading of suppressed exceptions, 
> it seems like it's used when there's no causal link between two exceptions 
> (e.g. a second exception thrown while cleaning up in a try-with-resources 
> block). But in this case there is a causal link.
>
> Dan was also curious about this (he asked the same thing on Slack).

I was thinking that "suppressed" more accurately conveys that it's probably 
irrelevant to what the user should look at, vs a normal "cause". Happy to use 
cause if y'all think it is less strange.


--
To view, visit http://gerrit.cloudera.org:8080/9489
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:02:44 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

2018-03-12 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9489 )

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 2:

Why did you choose to model the original exception as suppressed rather than as 
the cause of the new exception? From my reading of suppressed exceptions, it 
seems like it's used when there's no causal link between two exceptions (e.g. a 
second exception thrown while cleaning up in a try-with-resources block). But 
in this case there is a causal link.

Dan was also curious about this (he asked the same thing on Slack).


--
To view, visit http://gerrit.cloudera.org:8080/9489
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 22:55:51 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

2018-03-12 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9489 )

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9489/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java:

http://gerrit.cloudera.org:8080/#/c/9489/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java@110
PS2, Line 110:   StackTraceElement[] stack = new 
Exception().getStackTrace();
> actually given that this is a recursive method that's probably not robust.
Yeah, but since it's assumed exceptions happen not so often, probably that's 
not so crucial this to be robust.



--
To view, visit http://gerrit.cloudera.org:8080/9489
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 20:08:15 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

2018-03-12 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9489 )

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9489/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java:

http://gerrit.cloudera.org:8080/#/c/9489/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java@110
PS2, Line 110:   StackTraceElement[] stack = new 
Exception().getStackTrace();
> Might be useful to pop a few frames off the stack here, so that it doesn't
actually given that this is a recursive method that's probably not robust. LGTM



--
To view, visit http://gerrit.cloudera.org:8080/9489
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 19:10:04 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

2018-03-12 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9489 )

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 2:

(1 comment)

This is a neat trick, I like it.

http://gerrit.cloudera.org:8080/#/c/9489/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java:

http://gerrit.cloudera.org:8080/#/c/9489/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java@110
PS2, Line 110:   StackTraceElement[] stack = new 
Exception().getStackTrace();
Might be useful to pop a few frames off the stack here, so that it doesn't 
originate in transformException.



--
To view, visit http://gerrit.cloudera.org:8080/9489
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 19:08:13 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

2018-03-12 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9489 )

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9489/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java:

http://gerrit.cloudera.org:8080/#/c/9489/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java@92
PS1, Line 92: }
> Thank you for the explanation.
Ended up using the "suppressed exception" functionality of Throwable for this. 
Take a look



--
To view, visit http://gerrit.cloudera.org:8080/9489
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 18:34:52 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

2018-03-12 Thread Todd Lipcon (Code Review) 
Hello Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9489

to look at the new patch set (#2).

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..

KUDU-2230: java: sync client exception stack traces should make sense

Previously the exceptions thrown by the synchronous client always had
a stack trace somewhere deep inside our async callback chain. So, when
the user eventually caught this exception, it would be very hard to even
know what part of their code failed, since their own code would not even
show up in the call chain.

This patch simply replaces the exception's stack trace with the stack at
the "join" point where the exception is transformed back to a
KuduException. An exception which stores the original stack trace is
attached as a "suppressed" exception.

Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java
2 files changed, 30 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/9489/2
--
To view, visit http://gerrit.cloudera.org:8080/9489
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

2018-03-06 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9489 )

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9489/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java:

http://gerrit.cloudera.org:8080/#/c/9489/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java@92
PS1, Line 92:   StackTraceElement[] stack = new Exception().getStackTrace();
> I was thinking about saving the original but I think if we print it out in
Thank you for the explanation.

Yep, I think that adding the original stack trace into the debug or trace level 
message would make sense.



--
To view, visit http://gerrit.cloudera.org:8080/9489
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:27:06 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

2018-03-06 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9489 )

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9489/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java:

http://gerrit.cloudera.org:8080/#/c/9489/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java@92
PS1, Line 92:   StackTraceElement[] stack = new Exception().getStackTrace();
> Would it make sense to safe the original stack into some member of KuduExce
I was thinking about saving the original but I think if we print it out in logs 
it defeats the purpose of removing confusion here. If we just save it in a 
member and don't print it anywhere it doesn't have much use... so I elected to 
just ditch it.

One option I considered was to log it at TRACE level or something so that we 
can get to it if we need to in a debug situation. Do you think that solves the 
use case you had in mind?



--
To view, visit http://gerrit.cloudera.org:8080/9489
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 21:34:22 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

2018-03-06 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9489 )

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9489/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java:

http://gerrit.cloudera.org:8080/#/c/9489/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java@92
PS1, Line 92:   StackTraceElement[] stack = new Exception().getStackTrace();
Would it make sense to safe the original stack into some member of 
KuduException and then print it out along with the new stack in the overridden 
KuduException.printStackStrace() method?



--
To view, visit http://gerrit.cloudera.org:8080/9489
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 06 Mar 2018 21:30:08 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

2018-03-05 Thread Todd Lipcon (Code Review) 
Hello Alexey Serbin, Jean-Daniel Cryans,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/9489

to review the following change.


Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..

KUDU-2230: java: sync client exception stack traces should make sense

Previously the exceptions thrown by the synchronous client always had
a stack trace somewhere deep inside our async callback chain. So, when
the user eventually caught this exception, it would be very hard to even
know what part of their code failed, since their own code would not even
show up in the call chain.

This patch simply replaces the exception's stack trace with the stack at
the "join" point where the exception is transformed back to a
KuduException. We lose a bit of info that might be helpful for debugging
of internals, but the positive here strongly outweighs the negative.

Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java
2 files changed, 11 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/9489/1
--
To view, visit http://gerrit.cloudera.org:8080/9489
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans