You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yuqi Du (Code Review)" <ge...@cloudera.org> on 2022/04/15 15:49:56 UTC
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Yuqi Du has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18420
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
So a very offen very common scenarios,
when leader tserver restart, Scanner will read rows from the first
ScanResponse's lastPrimaryKey(), that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 171 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/1
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 1
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#20).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
In common scenarios, when tablet server hosting the leader replica
restarts, Scanners will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 170 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/20
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 20
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#19).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
In common scenarios, when tablet server hosting the leader replica
restarts, Scanners will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 170 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/19
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 19
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#2).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
So a very offen very common scenarios,
when leader tserver restart, Scanner will read rows from the first
ScanResponse's lastPrimaryKey(), that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 172 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/2
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 2
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 26: Code-Review+1
(1 comment)
Basically LGTM, but would like a comment added
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@204
PS24, Line 204: this.token = token;
> Simply, I need scan request at least 2 times.
That makes sense. It is a good practice to add details like this in comments. Please add a comment so it's clear to maintainers and future code authors why this is important for this test.
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 26
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 07 May 2022 01:08:15 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 6:
(1 comment)
Does C++ client has the same bug?
http://gerrit.cloudera.org:8080/#/c/18420/6/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:
http://gerrit.cloudera.org:8080/#/c/18420/6/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@76
PS6, Line 76: while (retries-- <= 0) {
Is it 'retries-- >= 0'
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 6
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 18 Apr 2022 03:55:10 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 28:
Thanks for addressing all the feedback!
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 28
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 07 May 2022 05:05:22 +0000
Gerrit-HasComments: No
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 15: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 15
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 21 Apr 2022 06:32:31 +0000
Gerrit-HasComments: No
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#13).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
So a very offen very common scenarios,
when leader tserver restart, Scanner will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 172 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/13
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 13
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 18: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 18
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 28 Apr 2022 15:15:25 +0000
Gerrit-HasComments: No
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#17).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
In common scenarios, when tablet server hosting the leader replica
restarts, Scanners will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 171 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/17
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 17
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 19:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@54
PS15, Line 54: 20000
> OK, I see -- thank you for the explanation.
In the unit test case, I has setted batchSizeBytes 1.
It's better than 1MB.
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@83
PS15, Line 83: harness.getClient().isCreateTableDone(TABLE_NAME))
> I guess you wouldn't waste the last 1 second -- there is 'break' at line 79
It's a unit test, waste a second not a problem, I can simply reduce the last `isCreateTableDone` check.
I expain it, when `retry` is 0, and isDone is false, then sleep(1000) and loop finish, we can give the ut another change.
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 19
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 30 Apr 2022 16:33:58 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#10).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
So a very offen very common scenarios,
when leader tserver restart, Scanner will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 172 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/10
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 10
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#23).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
In common scenarios, when tablet server hosting the leader replica
restarts, Scanners will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 170 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/23
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 23
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#8).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
So a very offen very common scenarios,
when leader tserver restart, Scanner will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 172 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/8
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 8
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 15:
(16 comments)
Thank you for the fix!
Overall looks good, just a few nits.
http://gerrit.cloudera.org:8080/#/c/18420/15//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18420/15//COMMIT_MSG@12
PS15, Line 12: So a very offen very common scenarios
nit: how about
In common scenarios
http://gerrit.cloudera.org:8080/#/c/18420/15//COMMIT_MSG@13
PS15, Line 13: when leader tserver restart
nit: how about
when tablet server hosting the leader replica restarts
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java:
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java@58
PS15, Line 58: rowErrors: {
nit: why to duplicate the 'rowErrors' tag with for every element? Maybe, have it just once for all the elements in rowErrors?
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java:
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java@46
PS15, Line 46: tserver server
nit: use either 'tablet server' or 'tserver', but not 'tserver server'
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java@48
PS15, Line 48: injection at
nit: ... injection happens at ...
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java@48
PS15, Line 48: The
nit: the
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@54
PS15, Line 54: 90000
Just curious: is it crucial to have 90000 instead of 20000 for some reason? Does it affect other test scenarios somehow?
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@83
PS15, Line 83: harness.getClient().isCreateTableDone(TABLE_NAME))
nit: why to add this? Maybe, just add one more retry instead just for the sake of clarity?
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@109
PS15, Line 109: The log for detail errors
nit: how about
Log on error details
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@192
PS15, Line 192: Injecting
nit: Inject
This and the following part of the sentence look a duplicate. Maybe, consolidate them?
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@195
PS15, Line 195: scan
nit: scanning
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@195
PS15, Line 195: non-fault tolerant scanner
nit: a non-fault tolerant scanner
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@195
PS15, Line 195: fault tolerant scanner
nit: a fault tolerant scanner
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@217
PS15, Line 217: enableFaultInject
nit: enableFaultInjection
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@237
PS15, Line 237: failureInjected
nit: faultInjected ?
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@289
PS15, Line 289: assertTrue(threadPool.isShutdown());
What's the significance of this assertion? Could it ever happen that after calling threadPool.shutdown() and threadPool.awaitTermination() at lines 279,280 the pool isn't shutdown at this point?
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 15
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 22 Apr 2022 05:55:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#26).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
In common scenarios, when tablet server hosting the leader replica
restarts, Scanners will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 158 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/26
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 26
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#27).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
In common scenarios, when tablet server hosting the leader replica
restarts, Scanners will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 160 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/27
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 27
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#12).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
So a very offen very common scenarios,
when leader tserver restart, Scanner will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 172 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/12
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 12
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 24:
(5 comments)
Sorry for the delay! Looking better!
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@83
PS24, Line 83: assertTrue(isDone || harness.getClient().isCreateTableDone(TABLE_NAME));
Why was this necessary? Doesn't the createTable() call wait for the table creation to complete by default? Or would it make sense to increase the timeout?
I'm also curious why this change is necessary in the first place. It doesn't seem like this patch affects table creation.
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@94
PS24, Line 94: id != Integer.MIN_VALUE &&
What's special about MIN_VALUE here?
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@93
PS24, Line 93: int id = random.nextInt();
: if (id != Integer.MIN_VALUE && !primaryKeys.contains(id)) {
: row.addInt(0, id);
: primaryKeys.add(id);
: } else {
: i--;
: continue;
: }
nit: I think it'd be a bit more straightforward to read this code as:
int key = random.nextInt();
while (primaryKeys.contains(key)) {
key = random.nextInt();
}
row.addInt(0, key);
primaryKeys.add(key);
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@204
PS24, Line 204: .batchSizeBytes(1)
nit: please comment _why_ it's important for this test to have a small batch size. What client-side behavior are we trying to encourage?
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@279
PS24, Line 279: rowCount += result.get();
Should we check for -1 errors here too?
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 24
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Wed, 04 May 2022 05:31:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 25:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@83
PS24, Line 83: int id = random.nextInt();
> +1
Yes , you are right , restore it.
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@94
PS24, Line 94:
> What's special about MIN_VALUE here?
At line 239, Integer.MIN_VALUE as a sentinel for check condition.
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@93
PS24, Line 93: session.apply(insert);
: }
: session.flush();
: session.close();
: // Log on error details.
: if (session.countPendingErrors() > 0) {
: LOG.info("RowErrorsAndOverflowStatus: {}", session.getPendingErrors().toString());
: }
> nit: I think it'd be a bit more straightforward to read this code as:
OK , I 'll do it.
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@204
PS24, Line 204: this.token = token;
> +1
Simply, I need scan request at least 2 times.
https://gerrit.cloudera.org/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java#54
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@279
PS24, Line 279: *
> Should we check for -1 errors here too?
At line 285 can cover it. So I think no check -1 is ok.
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 25
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 05 May 2022 06:34:05 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#9).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
So a very offen very common scenarios,
when leader tserver restart, Scanner will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 172 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/9
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 9
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 15:
The failed tests are not related, I've removed it.
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 15
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 21 Apr 2022 06:33:09 +0000
Gerrit-HasComments: No
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 18:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@54
PS15, Line 54: 90000
> The table has 3 tablets.
OK, I see -- thank you for the explanation.
You can try using the batchSizeBytes setter for KuduScannerBuilder: https://kudu.apache.org/apidocs/org/apache/kudu/client/KuduScanner.KuduScannerBuilder.html
It's better to keep the amount of data smaller overall and use more data only when necessary -- it adds up when running multiple test scenarios, increasing the overall runtime of the tests.
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@83
PS15, Line 83: harness.getClient().isCreateTableDone(TABLE_NAME))
> I just don't waste the last 1 second.
I guess you wouldn't waste the last 1 second -- there is 'break' at line 79, and if anything, you could simply increase the number of retries by one.
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 18
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 28 Apr 2022 17:38:40 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#16).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
In common scenarios, when tablet server hosting the leader replica
restarts, Scanners will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 171 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/16
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 16
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#7).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
So a very offen very common scenarios,
when leader tserver restart, Scanner will read rows from the first
ScanResponse's lastPrimaryKey(), that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 172 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/7
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 7
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#11).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
So a very offen very common scenarios,
when leader tserver restart, Scanner will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 172 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/11
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 11
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#14).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
So a very offen very common scenarios,
when leader tserver restart, Scanner will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 172 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/14
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 14
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 15:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18420/15//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18420/15//COMMIT_MSG@12
PS15, Line 12: f
nit: often
http://gerrit.cloudera.org:8080/#/c/18420/15/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/18420/15/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@693
PS15, Line 693: if (isFaultTolerant && resp.lastPrimaryKey != null) {
Nice catch! BTW, in looking at the differences between 'cb' defined at L558, and this gotNextRow callback, I'm curious: should we be updating other fields? Like the propagated timestamp? Or resource metrics?
It shouldn't be in this patch, but thought I'd raise the question, while we're in this area of the code. Are there good reasons we _shouldn't_ be updating those?
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 15
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 22 Apr 2022 06:31:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
In common scenarios, when tablet server hosting the leader replica
restarts, Scanners will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Reviewed-on: http://gerrit.cloudera.org:8080/18420
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 160 insertions(+), 3 deletions(-)
Approvals:
Kudu Jenkins: Verified
Andrew Wong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 28
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 28: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 28
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sun, 08 May 2022 16:40:27 +0000
Gerrit-HasComments: No
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#4).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
So a very offen very common scenarios,
when leader tserver restart, Scanner will read rows from the first
ScanResponse's lastPrimaryKey(), that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 171 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/4
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 4
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#6).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
So a very offen very common scenarios,
when leader tserver restart, Scanner will read rows from the first
ScanResponse's lastPrimaryKey(), that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 172 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/6
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 6
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#5).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
So a very offen very common scenarios,
when leader tserver restart, Scanner will read rows from the first
ScanResponse's lastPrimaryKey(), that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 172 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/5
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 5
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#3).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
So a very offen very common scenarios,
when leader tserver restart, Scanner will read rows from the first
ScanResponse's lastPrimaryKey(), that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 172 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/3
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 3
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has removed a vote on this change.
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 15
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 18:
Does C++ client has the same bug?
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 18
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 28 Apr 2022 15:46:28 +0000
Gerrit-HasComments: No
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#21).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
In common scenarios, when tablet server hosting the leader replica
restarts, Scanners will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 170 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/21
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 21
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#25).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
In common scenarios, when tablet server hosting the leader replica
restarts, Scanners will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 158 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/25
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 25
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 16:
(17 comments)
http://gerrit.cloudera.org:8080/#/c/18420/15//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18420/15//COMMIT_MSG@12
PS15, Line 12: In common scenarios, when tablet serv
> nit: how about
Done
http://gerrit.cloudera.org:8080/#/c/18420/15//COMMIT_MSG@12
PS15, Line 12: e
> nit: often
Done
http://gerrit.cloudera.org:8080/#/c/18420/15//COMMIT_MSG@13
PS15, Line 13: restarts, Scanners will rea
> nit: how about
Done
http://gerrit.cloudera.org:8080/#/c/18420/15/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/18420/15/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@693
PS15, Line 693: if (isFaultTolerant && resp.lastPrimaryKey != null) {
> Nice catch! BTW, in looking at the differences between 'cb' defined at L558
ok. I think you are right.
Now resource metrics no problem, and exteral consisency propagated timestamp is a little complex,
and I'll take some time to confirm it.
As you said, the two infos update can fix at another commit.
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java:
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java@46
PS15, Line 46: tserver in the
> nit: use either 'tablet server' or 'tserver', but not 'tserver server'
Done
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java@48
PS15, Line 48: the
> nit: the
Done
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java@48
PS15, Line 48: injection ha
> nit: ... injection happens at ...
Done
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@54
PS15, Line 54: 90000
> Just curious: is it crucial to have 90000 instead of 20000 for some reason?
The table has 3 tablets.
In the testcase, we pick a tablet, need to send 2 scan requests to the leader replica at least.
A ScanRequest' response maybe has many rows, the response's row count is not controlled by request, basicly decided by 1MB and average row size.
So I think using a larger ROW_COUNT is ok and cover the case, although it's not good enough.
All testcases has passed, maybe some tests slower than before, but I didnot feel it.
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@83
PS15, Line 83: harness.getClient().isCreateTableDone(TABLE_NAME))
> nit: why to add this? Maybe, just add one more retry instead just for the
I just don't waste the last 1 second.
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@109
PS15, Line 109: Log on error details.
> nit: how about
Done
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@192
PS15, Line 192: Inject fa
> nit: Inject
Sentences below removed.
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@195
PS15, Line 195: e sc
> nit: scanning
Done
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@195
PS15, Line 195: g and a non-fault tolerant
> nit: a non-fault tolerant scanner
Done
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@195
PS15, Line 195: a fault tolerant scann
> nit: a fault tolerant scanner
Done
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@217
PS15, Line 217: ltInjection = ena
> nit: enableFaultInjection
Done
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@237
PS15, Line 237: (rri.hasNext())
> nit: faultInjected ?
Done
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@289
PS15, Line 289: * Injecting failures (i.e. drop clien
> What's the significance of this assertion? Could it ever happen that after
Removed.
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 16
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 26 Apr 2022 08:33:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18420/6/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:
http://gerrit.cloudera.org:8080/#/c/18420/6/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@76
PS6, Line 76: while (retries-- >= 0) {
> Is it 'retries-- >= 0'
Done
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 7
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 18 Apr 2022 06:31:25 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 15: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 15
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 21 Apr 2022 06:39:47 +0000
Gerrit-HasComments: No
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#22).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
In common scenarios, when tablet server hosting the leader replica
restarts, Scanners will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 170 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/22
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 22
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18420
to look at the new patch set (#24).
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
[java] Fix a scan bug which will read repetitive rows.
When isFaultTolerant is true, from 2nd ScanRequest, in its response
callback function, lastPrimaryKey is not updated.
In common scenarios, when tablet server hosting the leader replica
restarts, Scanners will read rows from the first
ScanResponse's lastPrimaryKey, that will return some repetitive rows.
Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowErrorsAndOverflowStatus.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
4 files changed, 170 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/18420/24
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 24
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 24:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:
http://gerrit.cloudera.org:8080/#/c/18420/15/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@54
PS15, Line 54: 20000
> In the unit test case, I has setted batchSizeBytes 1.
OK, that sounds good!
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@83
PS24, Line 83: assertTrue(isDone || harness.getClient().isCreateTableDone(TABLE_NAME));
> Why was this necessary? Doesn't the createTable() call wait for the table c
+1
After looking at that a bit more, I see that by default CreateTableOptions has 'wait' set to 'true', meaning the code at line 73 does wait for the creation of the table to complete. Indeed -- it's not clear to me why to have this code here at all. Maybe, I'm missing something, so it would be great if you add comment to explain why it was necessary to add this loop with isCreateTableDone()
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@204
PS24, Line 204: .batchSizeBytes(1)
> nit: please comment _why_ it's important for this test to have a small batc
+1
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 24
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Wed, 04 May 2022 16:39:29 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 27: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 27
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 07 May 2022 05:04:05 +0000
Gerrit-HasComments: No
[kudu-CR] [java] Fix a scan bug which will read repetitive rows.
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18420 )
Change subject: [java] Fix a scan bug which will read repetitive rows.
......................................................................
Patch Set 28:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:
http://gerrit.cloudera.org:8080/#/c/18420/24/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java@204
PS24, Line 204:
> That makes sense. It is a good practice to add details like this in comment
Done
--
To view, visit http://gerrit.cloudera.org:8080/18420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d6be9df10c1a45cd971b52a0351028c1f5a023f
Gerrit-Change-Number: 18420
Gerrit-PatchSet: 28
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 07 May 2022 06:16:35 +0000
Gerrit-HasComments: Yes