You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yifan Zhang (Code Review)" <ge...@cloudera.org> on 2020/03/16 06:11:22 UTC
[kudu-CR] [java] fix bug in getClosestServerInfo code
Yifan Zhang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15444
Change subject: [java] fix bug in getClosestServerInfo code
......................................................................
[java] fix bug in getClosestServerInfo code
When client handle tablet not found error, it removes the tablet server
form the RemoteTablet's locations. If all tablet servers are removed,
the code in getClosestServerInfo will throw a division by zero exception.
This patch fixed it and added a test to verify getClosestServerInfo
returns null when all locations of a tablet are invalid.
Change-Id: Ib4dc471b5044ea4b5bd6202ccd2a707d6e229ea0
---
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
2 files changed, 14 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/44/15444/1
--
To view, visit http://gerrit.cloudera.org:8080/15444
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib4dc471b5044ea4b5bd6202ccd2a707d6e229ea0
Gerrit-Change-Number: 15444
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
[kudu-CR] [java] fix bug in getClosestServerInfo code
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15444
to look at the new patch set (#2).
Change subject: [java] fix bug in getClosestServerInfo code
......................................................................
[java] fix bug in getClosestServerInfo code
When client handles tablet not found error, it removes the tablet server
from the RemoteTablet's locations. If all tablet servers are removed,
the code in getClosestServerInfo will throw a division by zero exception.
This patch fixed it and added a test to verify getClosestServerInfo
returns null when all locations of a tablet are invalid.
Change-Id: Ib4dc471b5044ea4b5bd6202ccd2a707d6e229ea0
---
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
2 files changed, 16 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/44/15444/2
--
To view, visit http://gerrit.cloudera.org:8080/15444
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib4dc471b5044ea4b5bd6202ccd2a707d6e229ea0
Gerrit-Change-Number: 15444
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] fix bug in getClosestServerInfo code
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15444 )
Change subject: [java] fix bug in getClosestServerInfo code
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15444/1/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java:
http://gerrit.cloudera.org:8080/#/c/15444/1/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@199
PS1, Line 199: ServerInfo result = null;
How about just an early return null here if tabletServers is empty? Functionally the same result, but less code will be executed and may be clearer too.
--
To view, visit http://gerrit.cloudera.org:8080/15444
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4dc471b5044ea4b5bd6202ccd2a707d6e229ea0
Gerrit-Change-Number: 15444
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 16 Mar 2020 07:18:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] fix bug in getClosestServerInfo code
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15444 )
Change subject: [java] fix bug in getClosestServerInfo code
......................................................................
[java] fix bug in getClosestServerInfo code
When client handles tablet not found error, it removes the tablet server
from the RemoteTablet's locations. If all tablet servers are removed,
the code in getClosestServerInfo will throw a division by zero exception.
This patch fixed it and added a test to verify getClosestServerInfo
returns null when all locations of a tablet are invalid.
Change-Id: Ib4dc471b5044ea4b5bd6202ccd2a707d6e229ea0
Reviewed-on: http://gerrit.cloudera.org:8080/15444
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
2 files changed, 16 insertions(+), 0 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/15444
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib4dc471b5044ea4b5bd6202ccd2a707d6e229ea0
Gerrit-Change-Number: 15444
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
[kudu-CR] [java] fix bug in getClosestServerInfo code
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15444 )
Change subject: [java] fix bug in getClosestServerInfo code
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/15444
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4dc471b5044ea4b5bd6202ccd2a707d6e229ea0
Gerrit-Change-Number: 15444
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Mon, 16 Mar 2020 07:49:37 +0000
Gerrit-HasComments: No
[kudu-CR] [java] fix bug in getClosestServerInfo code
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15444 )
Change subject: [java] fix bug in getClosestServerInfo code
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15444/1/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java:
http://gerrit.cloudera.org:8080/#/c/15444/1/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@199
PS1, Line 199: if (tabletServers.isEmpty()) {
> How about just an early return null here if tabletServers is empty? Functio
Done
--
To view, visit http://gerrit.cloudera.org:8080/15444
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4dc471b5044ea4b5bd6202ccd2a707d6e229ea0
Gerrit-Change-Number: 15444
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Mon, 16 Mar 2020 07:27:25 +0000
Gerrit-HasComments: Yes