You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Song Jiacheng (Code Review)" <ge...@cloudera.org> on 2023/11/20 09:24:01 UTC
[kudu-CR] KUDU-3526 [java] Scanner should bound with a tserver in java client.
Song Jiacheng has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20715
Change subject: KUDU-3526 [java] Scanner should bound with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bound with a tserver in java client.
For now, scan requests sent by java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver which it first communicates with.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will response with the "scanner not found"
exception.
We should make the scanner bound with the tserver, like how it does
in c++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
3 files changed, 19 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/1
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 1
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
[kudu-CR] KUDU-3526 [java] Scanner should bound with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#7).
Change subject: KUDU-3526 [java] Scanner should bound with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bound with a tserver in java client.
For now, scan requests sent by java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver which it first communicates with.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will response with the "scanner not found"
exception.
We should make the scanner bound with the tserver, like how it does
in c++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
3 files changed, 23 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/7
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 7
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Song Jiacheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
Patch Set 18:
> Patch Set 18: Code-Review+1
>
> (1 comment)
Thanks for the review!
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 18
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Tue, 26 Dec 2023 09:28:30 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Song Jiacheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
Patch Set 23:
(13 comments)
Thanks for the review!
http://gerrit.cloudera.org:8080/#/c/20715/19//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20715/19//COMMIT_MSG@10
PS19, Line 10: It is because scanner does not bind with the tserver
: with which it first communicates.
> This is applicable only to scanners with LEADER_ONLY replica selection poli
This problem is triggered when getReplicaSelectedServerInfo cound not find a tserver because of the leader demotion. For the policy CLOSEST_REPLICA I think this will not happen.
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@290
PS19, Line 290: /**
> It would be great to document this new field.
Done
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@632
PS19, Line 632: resourceMetrics.update(resp.resourceMetricsPb);
: }
> Should tsUUID be updated in this case as well? What if the application tri
Done
http://gerrit.cloudera.org:8080/#/c/20715/19/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/20715/19/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@306
PS19, Line 306: Get information on tablet server by
> Get information on tablet server by its UUID.
Done
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@307
PS19, Line 307: tablet server
> What's "replica server"?
Done
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@308
PS19, Line 308: tablet server
> ?
Sorry, I'm also very confused how it came from.
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@636
PS19, Line 636: scanner
> nit: rename into 'referenceServerHostPort'?
Done
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@638
PS19, Line 638: ostAndPo
> nit: rename into 'referenceTabletId'
Done
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@650
PS19, Line 650: uilder process
> nit: this name for a variable is confusing -- should it be something like l
Done
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@654
PS19, Line 654:
> I guess it's better to do this via assertEventuallyTrue() with some reasona
Tried to search for the method but did not find it. Thanks!
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@670
PS19, Line 670: HostAndPort targetHp = harness.findLeaderTabletServer(targetTablet);
: return !targetHp.equals(referenceServerHostPort);
> Doesn't demoteLeader() simply nullifies a field of the RemoteTablet instanc
These two operations, demoteLeader() and emptyTabletsCacheForTable() , are actually the behaviors of the client when another request has sent to the wrong leader tablet server and the change of leadership has been acknowledged. Not only the demoteLeader().
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@676
PS19, Line 676: // Simulate that another request(like Bat
> nit: it's be clear what's going below on even without this comment; this co
Done
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@678
PS19, Line 678: kuduScanner.currentTablet().demoteLeader(
: kuduScanner.currentTablet().getLeaderServerInfo().getUuid());
: asy
> Should we check that at least one batch of rows has been returned below? O
Done
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 23
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 04 Jan 2024 13:51:29 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
Patch Set 11:
(2 comments)
Thanks for adding the requested test!
Just to be picky, I changed back the line which was causing the error:
@@ -1325,7 +1325,8 @@ public class AsyncKuduClient implements AutoCloseable {
// Important to increment the attempts before the next if statement since
// getSleepTimeForRpc() relies on it if the client is null or dead.
nextRequest.attempt++;
- final ServerInfo info = tablet.getTabletServerByUuid(scanner.getTsUUID());
+ final ServerInfo info = tablet.getReplicaSelectedServerInfo(nextRequest.getReplicaSelection(),
+ location);
Build and run the test, but I dont get any errors. Did I miss something?
Just wanna make sure that the test fails - repros the issue.
http://gerrit.cloudera.org:8080/#/c/20715/11/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/11/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@636
PS11, Line 636: Restart
nit: Step down
right? ie the command you are building below is kudu tablet leader_step_down
http://gerrit.cloudera.org:8080/#/c/20715/11/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@657
PS11, Line 657: LOG.debug("Scaned " + kuduScanner.nextRows().numRows);
Rather than just logging it, could you please add assertions, that we dont get 'scanner not found' error?
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 11
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Comment-Date: Tue, 12 Dec 2023 13:52:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#8).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 79 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/8
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 8
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yifan Zhang, Attila Bukor, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#24).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 121 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/24
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 24
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
[kudu-CR] KUDU-3526 [java] Scanner should bound with a tserver in java client.
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bound with a tserver in java client.
......................................................................
Patch Set 7:
(4 comments)
I wanted to ask for a test, just to encapsulate the initial problem, when the leader changes.
I did a quick search, and found one Java test, which uses leader_step_down: https://github.com/apache/kudu/blob/master/java/kudu-client/src/test/java/org/apache/kudu/client/TestScanToken.java#L241-L263
Using this example, do you think you can add a test which reproduceses the original issue?
Thanks!
http://gerrit.cloudera.org:8080/#/c/20715/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20715/7//COMMIT_MSG@9
PS7, Line 9: by java
nit: by the Java
http://gerrit.cloudera.org:8080/#/c/20715/7//COMMIT_MSG@10
PS7, Line 10: does not bound with the
: tserver which it first communicates with.
nit: is not bound to the tserver with which it first communicates.
http://gerrit.cloudera.org:8080/#/c/20715/7//COMMIT_MSG@16
PS7, Line 16: response
nit: respond
http://gerrit.cloudera.org:8080/#/c/20715/7//COMMIT_MSG@19
PS7, Line 19: We should make the scanner bound with the tserver, like how it does
: in c++ client.
nit: We should bind the scanner to the tserver, similar to how it is done in the C++ client.
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 7
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Tue, 21 Nov 2023 11:38:52 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Song Jiacheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
Patch Set 11:
(2 comments)
> Patch Set 11:
>
> (2 comments)
>
> Thanks for adding the requested test!
>
> Just to be picky, I changed back the line which was causing the error:
>
> @@ -1325,7 +1325,8 @@ public class AsyncKuduClient implements AutoCloseable {
> // Important to increment the attempts before the next if statement since
> // getSleepTimeForRpc() relies on it if the client is null or dead.
> nextRequest.attempt++;
> - final ServerInfo info = tablet.getTabletServerByUuid(scanner.getTsUUID());
> + final ServerInfo info = tablet.getReplicaSelectedServerInfo(nextRequest.getReplicaSelection(),
> + location);
>
> Build and run the test, but I dont get any errors. Did I miss something?
> Just wanna make sure that the test fails - repros the issue.
Thanks for the review!
It seems that the LeaderStepDown is not finished before the next scan requests while testing it in release mode. I added a loop to wait for the transfer of leadership. So it will cause the "scanner not found" error without the fix.
http://gerrit.cloudera.org:8080/#/c/20715/11/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/11/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@636
PS11, Line 636: Restart
> nit: Step down
Sorry, forgot to change the comment.
http://gerrit.cloudera.org:8080/#/c/20715/11/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@657
PS11, Line 657: LOG.debug("Scaned " + kuduScanner.nextRows().numRows);
> Rather than just logging it, could you please add assertions, that we dont
Actually the error thread is not the main thread so the assert mostly not works.
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 11
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Comment-Date: Mon, 18 Dec 2023 11:01:41 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#9).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with
scanner not found error. It is because that scanner does not bound
with the tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 79 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/9
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 9
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yifan Zhang, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#20).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 116 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/20
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 20
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yifan Zhang, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#23).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because scanner does not bind with the tserver
with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 116 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/23
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 23
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Song Jiacheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
Patch Set 8:
(4 comments)
> Patch Set 7:
>
> (4 comments)
>
> I wanted to ask for a test, just to encapsulate the initial problem, when the leader changes.
>
> I did a quick search, and found one Java test, which uses leader_step_down: https://github.com/apache/kudu/blob/master/java/kudu-client/src/test/java/org/apache/kudu/client/TestScanToken.java#L241-L263
> Using this example, do you think you can add a test which reproduceses the original issue?
>
> Thanks!
Thank you for the review!
I have added a unit test which will fail without the fix. Could you please help me review this again? Thanks!
http://gerrit.cloudera.org:8080/#/c/20715/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20715/7//COMMIT_MSG@9
PS7, Line 9: by the
> nit: by the Java
Done
http://gerrit.cloudera.org:8080/#/c/20715/7//COMMIT_MSG@10
PS7, Line 10: does not bound with the
: tserver with which it first communicates.
> nit: is not bound to the tserver with which it first communicates.
Done
http://gerrit.cloudera.org:8080/#/c/20715/7//COMMIT_MSG@16
PS7, Line 16: respond
> nit: respond
Done
http://gerrit.cloudera.org:8080/#/c/20715/7//COMMIT_MSG@19
PS7, Line 19: We should bind the scanner to the tserver, similar to how it is done
: in the C++ cli
> nit: We should bind the scanner to the tserver, similar to how it is done i
Done
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 8
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Comment-Date: Wed, 29 Nov 2023 07:27:45 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bound with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#6).
Change subject: KUDU-3526 [java] Scanner should bound with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bound with a tserver in java client.
For now, scan requests sent by java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver which it first communicates with.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will response with the "scanner not found"
exception.
We should make the scanner bound with the tserver, like how it does
in c++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
3 files changed, 22 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/6
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 6
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#16).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with "scanner
not found" error. It is because that scanner does not bound with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 99 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/16
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 16
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
Patch Set 25:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20715/25//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20715/25//COMMIT_MSG@10
PS25, Line 10: bound
nit: bind was correct
http://gerrit.cloudera.org:8080/#/c/20715/25/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/25/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@676
PS25, Line 676: // This exception happens when a scanner is opened and then the leader of the tablet
The wording is a bit confusing, as there is no exception right now, only before this commit.
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 25
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Mon, 08 Jan 2024 07:58:54 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yifan Zhang, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#21).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 116 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/21
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 21
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
Patch Set 19:
(13 comments)
It seems not many applications use Java Kudu client for fetching/reading data: Impala uses Kudu C++ client for that, so don't be surprised to hit more bugs on this path.
Thank you for addressing the issue!
http://gerrit.cloudera.org:8080/#/c/20715/19//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20715/19//COMMIT_MSG@10
PS19, Line 10: It is because that scanner does not bound with the
: tserver with which it first communicates.
This is applicable only to scanners with LEADER_ONLY replica selection policy, right? Or scanners with other policy selection are affected as well?
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@290
PS19, Line 290: private String tsUUID = null;
It would be great to document this new field.
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@632
PS19, Line 632: scanFinished();
: return Deferred.fromResult(resp.data); // there
Should tsUUID be updated in this case as well? What if the application tries to close the scanner explicitly after this, when there was only one scan batch returned: does it work as expected with this code?
http://gerrit.cloudera.org:8080/#/c/20715/19/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/20715/19/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@306
PS19, Line 306: Get the replica server info by uuid
Get information on tablet server by its UUID.
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@307
PS19, Line 307: replica server
What's "replica server"?
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@308
PS19, Line 308: replica server
?
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@636
PS19, Line 636: leaderHp
nit: rename into 'referenceServerHostPort'?
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@638
PS19, Line 638: tabletID
nit: rename into 'referenceTabletId'
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@650
PS19, Line 650: quiesceTserver
nit: this name for a variable is confusing -- should it be something like leaderStepDown or similar?
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@654
PS19, Line 654: while (true) {
I guess it's better to do this via assertEventuallyTrue() with some reasonable timeout (10 seconds?). You could find how it's done in other Java Kudu tests.
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@670
PS19, Line 670: // Simulate that another request(like Batch) has sent to the wrong leader tablet server and
: // the change of leadership has been acknowledged. The response will demote the leader.
Doesn't demoteLeader() simply nullifies a field of the RemoteTablet instance? If so, why to add this comment then?
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@676
PS19, Line 676: // Send the "next" type of scan requests.
nit: it's be clear what's going below on even without this comment; this comment doesn't seem to bring any extra clarity
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@678
PS19, Line 678: while (kuduScanner.hasMoreRows()) {
: kuduScanner.nextRows();
: }
Should we check that at least one batch of rows has been returned below? Otherwise, this tests is a dud if kuduScanner.hasMoreRows() returns 'false'.
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 19
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Wed, 03 Jan 2024 23:54:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#18).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found" exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 100 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/18
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 18
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#10).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with
scanner not found error. It is because that scanner does not bound
with the tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client. Please see KUDU-3526 for details.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 79 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/10
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 10
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
Patch Set 28: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20715/27/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/20715/27/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@312
PS27, Line 312: return tabletServers.get(uuid);
> For now it returns null if uuid is null, which I think it should return. Th
That makes sense. Thanks for your clarification.
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 28
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 18 Jan 2024 08:36:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
Patch Set 27:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20715/27/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/27/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@291
PS27, Line 291: bounded
nit: bound
http://gerrit.cloudera.org:8080/#/c/20715/27/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/20715/27/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@312
PS27, Line 312: return tabletServers.get(uuid);
nit: Should we also handle the case where uuid is null?
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 27
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Sun, 14 Jan 2024 06:50:39 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bound with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#3).
Change subject: KUDU-3526 [java] Scanner should bound with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bound with a tserver in java client.
For now, scan requests sent by java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver which it first communicates with.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will response with the "scanner not found"
exception.
We should make the scanner bound with the tserver, like how it does
in c++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
3 files changed, 19 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/3
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 3
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
Patch Set 28: Code-Review+2
(3 comments)
Thank you very much for the fix!
http://gerrit.cloudera.org:8080/#/c/20715/19//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20715/19//COMMIT_MSG@10
PS19, Line 10: It is because that scanner does not bind with the
: tserver with which it first communicates.
> This problem is triggered when getReplicaSelectedServerInfo cound not find
Yep, that makes sense to me. Thank you for clarifying.
http://gerrit.cloudera.org:8080/#/c/20715/19/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/20715/19/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@308
PS19, Line 308: tablet server
> Sorry, I'm also very confused how it came from.
Maybe, that was from the very early revisions when the terminology wasn't quite flushed out yet. Thank you for fixing these comments!
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/19/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@670
PS19, Line 670: HostAndPort targetHp = harness.findLeaderTabletServer(targetTablet);
: return !targetHp.equals(referenceServerHostPort);
> These two operations, demoteLeader() and emptyTabletsCacheForTable() , are
OK, now I see the intent behind this comment. Thank you for clarifying.
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 28
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 18 Jan 2024 23:41:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#11).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with
scanner not found error. It is because that scanner does not bound
with the tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client. See KUDU-3526 for details.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 79 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/11
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 11
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
Patch Set 23:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20715/23/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/23/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@294
PS23, Line 294: null
nit: this is not necessary in Java
http://gerrit.cloudera.org:8080/#/c/20715/23/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/23/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@678
PS23, Line 678: kuduScanner.currentTablet().demoteLeader(
IIUC this is what triggered the bug before your fix, correct? Can you add a note why it's needed here to avoid any confusion?
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 23
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 05 Jan 2024 16:46:14 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yifan Zhang, Attila Bukor, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#26).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because that scanner does not bind with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 116 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/26
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 26
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#17).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 100 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/17
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 17
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#13).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with "scanner
not found" error. It is because that scanner does not bound with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 99 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/13
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 13
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Song Jiacheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
Patch Set 18:
(1 comment)
Thanks for the review!
http://gerrit.cloudera.org:8080/#/c/20715/11/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/11/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@657
PS11, Line 657: LocatedTablet targetTablet = null;
> nit: Maybe we can catch the exception and check its error message does not
Yes, I tried it, but the exception could not be caught. I think the reason is that the thread which cause error is not the main thread of the test.
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 18
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Tue, 26 Dec 2023 08:46:27 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bound with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#4).
Change subject: KUDU-3526 [java] Scanner should bound with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bound with a tserver in java client.
For now, scan requests sent by java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver which it first communicates with.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will response with the "scanner not found"
exception.
We should make the scanner bound with the tserver, like how it does
in c++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
3 files changed, 19 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/4
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 4
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3526 [java] Scanner should bound with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#5).
Change subject: KUDU-3526 [java] Scanner should bound with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bound with a tserver in java client.
For now, scan requests sent by java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver which it first communicates with.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will response with the "scanner not found"
exception.
We should make the scanner bound with the tserver, like how it does
in c++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
3 files changed, 19 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/5
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 5
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#14).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 99 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/14
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 14
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#15).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with "scanner
not found" error. It is because that scanner does not bound with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found" exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 99 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/15
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 15
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
Patch Set 18: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20715/11/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/11/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@657
PS11, Line 657: LocatedTablet targetTablet = null;
> Actually the error thread is not the main thread so the assert mostly not w
nit: Maybe we can catch the exception and check its error message does not contain 'scanner not found'.
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 18
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 22 Dec 2023 12:36:08 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because that scanner does not bind with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Reviewed-on: http://gerrit.cloudera.org:8080/20715
Tested-by: Kudu Jenkins
Reviewed-by: Yifan Zhang <ch...@163.com>
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 116 insertions(+), 7 deletions(-)
Approvals:
Kudu Jenkins: Verified
Yifan Zhang: Looks good to me, approved
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 29
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yifan Zhang, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#22).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 116 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/22
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 22
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yifan Zhang, Attila Bukor, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#28).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because that scanner does not bind with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 116 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/28
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 28
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#12).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 99 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/12
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 12
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Yifan Zhang, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#19).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 104 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/19
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 19
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yifan Zhang, Attila Bukor, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#25).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policies. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found"
exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 121 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/25
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 25
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Song Jiacheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
Patch Set 25:
(4 comments)
> Patch Set 25:
>
> (2 comments)
Thanks for the review.
I am kind of confused since I think the notes there are already trying to explain why to call RemoteTablet.demoteLeader and AsyncClient.emptyTabletsCacheForTable. The two methods are used to simulate the scenario.
http://gerrit.cloudera.org:8080/#/c/20715/25//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20715/25//COMMIT_MSG@10
PS25, Line 10: bound
> nit: bind was correct
Done
http://gerrit.cloudera.org:8080/#/c/20715/23/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/23/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@294
PS23, Line 294:
> nit: this is not necessary in Java
Done
http://gerrit.cloudera.org:8080/#/c/20715/23/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/23/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@678
PS23, Line 678: // with a not leader exception. And the retry of the request will refresh the tablets
> IIUC this is what triggered the bug before your fix, correct? Can you add a
Yes, and added some details.
http://gerrit.cloudera.org:8080/#/c/20715/25/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/25/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@676
PS25, Line 676: // This exception happens when a scanner is opened and then the leader of the tablet
> The wording is a bit confusing, as there is no exception right now, only be
The words are trying to describe the process without the fix. Actually I think this is not necessary, the following paragraph is enough.
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 25
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Mon, 08 Jan 2024 08:16:44 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Song Jiacheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
Patch Set 28:
(2 comments)
> Patch Set 27:
>
> (2 comments)
Thanks for the view!
http://gerrit.cloudera.org:8080/#/c/20715/27/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/27/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@291
PS27, Line 291: bound w
> nit: bound
Done
http://gerrit.cloudera.org:8080/#/c/20715/27/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/20715/27/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@312
PS27, Line 312: return tabletServers.get(uuid);
> nit: Should we also handle the case where uuid is null?
For now it returns null if uuid is null, which I think it should return. The client will try to find the target tablet server via cache or sending request.
final ServerInfo info = tablet.getTabletServerByUuid(scanner.getTsUUID());
if (info == null) {
return delayedSendRpcToTablet(nextRequest, new RecoverableException(Status.RemoteError(
String.format("No information on servers hosting tablet %s, will retry later",
tablet.getTabletId()))));
}
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 28
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Tue, 16 Jan 2024 08:16:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bound with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#2).
Change subject: KUDU-3526 [java] Scanner should bound with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bound with a tserver in java client.
For now, scan requests sent by java client might fail with scanner
not found error. It is because that scanner does not bound with the
tserver which it first communicates with.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will response with the "scanner not found"
exception.
We should make the scanner bound with the tserver, like how it does
in c++ client. See KUDU-3526 for details.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
3 files changed, 19 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/2
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 2
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yifan Zhang, Attila Bukor, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20715
to look at the new patch set (#27).
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client.
......................................................................
KUDU-3526 [java] Scanner should bind with a tserver in java client.
For now, scan requests sent by the java client might fail with scanner
not found error. It is because that scanner does not bind with the
tserver with which it first communicates.
The code in method scanNextRows of java client is still trying to
search for the tserver by the locations and selection policy. So if
the leader is changed and the next scan request is sent to the new
leader, the tserver will respond with the "scanner not found" exception.
We should bind the scanner to the tserver, similar to how it is done
in the C++ client.
Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
4 files changed, 116 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/27
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 27
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>