Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15136 )

Change subject: KUDU-1438: [java] Upgrade to Netty 4
......................................................................


Patch Set 6:

(3 comments)

I fixed TestAuthTokenReacquire which was failing ~20% of the time and now 
hasn't failed in 300+ runs. This also fixed the other flaky tests seen 
(DefaultSourceTest, DistributedDataGeneratorTest).

http://gerrit.cloudera.org:8080/#/c/15136/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15136/6//COMMIT_MSG@27
PS6, Line 27: deafult
> default
Done


http://gerrit.cloudera.org:8080/#/c/15136/6//COMMIT_MSG@34
PS6, Line 34: negligable
> negligible
Done


http://gerrit.cloudera.org:8080/#/c/15136/6/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/15136/6/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@373
PS6, Line 373:     // Netty uses 'simple' leak detection by default which 
carries a small overhead.
             :     // To avoid this overhead in production use, we disable leak 
detection unless
             :     // a level is explicitly set.
> You sure we want to do this? Is the overhead measurable? The webpage you li
Yeah, I can remove it to keep things more simple.

While performance testing I made this change. I didn't see a measurable impact, 
though I did see some other projects do something similar.

My rationale was this. The simple leak detection will log if a leak occurs 
without enough context to necessarily identify the leak and without calling 
attention to the log itself. This means a user would likely need to hit some 
other memory issue before they even go looking for this. If users suspect a 
leak, they could still set this property to their desired level and check the 
logs to get more context on the issue. At that point they could use the 
Advanced or paranoid level to get enough detail to identify the leak as well.

We can always add this back in if/when it makes sense.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic75bd15a982187039ff2e1510af9390d304f7626
Gerrit-Change-Number: 15136
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 07 Feb 2020 17:18:15 +0000
Gerrit-HasComments: Yes

Reply via email to