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

Change subject: KUDU-1868: Part 1: Add timer-based RPC timeouts
......................................................................


Patch Set 5:

(1 comment)

I think you need to rebase due to my KuduPartitioner patch. Apologies for that.

http://gerrit.cloudera.org:8080/#/c/12338/5/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/12338/5/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@2377
PS5, Line 2377:     return null;
> We can't schedule a timeout on the timer, so we'd need to implement a Faile
Yes, this is what I am advocating for. It's a small wrapper that would prevent 
future NPEs. If FailedTimeout already existed I suspect you would use it.

The downside to adding this is about 30 lines of trivial code, the downside to 
not adding it is wrapping ever call to cancel in a null check and NPEs where we 
forget to do that.

I feel strong enough about it that I am happy to do this in a follow up patch.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d823b63ac0a41cc5e42b63a7c19e0ef777e1dea
Gerrit-Change-Number: 12338
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wdberke...@gmail.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Grant Henke <granthe...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com>
Gerrit-Comment-Date: Thu, 14 Feb 2019 19:54:27 +0000
Gerrit-HasComments: Yes

Reply via email to