You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Hao Hao (Code Review)" <ge...@cloudera.org> on 2017/12/15 01:21:29 UTC
[kudu-CR] KUDU-1704: add java client support for READ OWN WRITES mode
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8847
Change subject: KUDU-1704: add java client support for READ_OWN_WRITES mode
......................................................................
KUDU-1704: add java client support for READ_OWN_WRITES mode
Change-Id: I6239521c022147257859e399f55c6f3f945af465
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
4 files changed, 145 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/8847/1
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has removed a vote on this change.
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 8
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 10:
lgtm, thanks
waiting to get some validation from the jepsen work and then it's gtg
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 08 Mar 2018 23:46:54 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 8:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG@6
PS7, Line 6:
: KUDU-1704: add java client support for READ_YOUR_WRITES mode
> ok, thats fine. really want to make sure we don't drop it though, if we're
Yeah, totally makes sense to me. I will certainly pick up the jepsen test with RYW.
http://gerrit.cloudera.org:8080/#/c/8847/7/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/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@817
PS7, Line 817: case OPENING:
> nit, comma after "set"
Done
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@818
PS7, Line 818: nner. This kind of a
> pet peeve: propagated timestamp or propagation timestamp? are these differe
Done
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1044
PS7, Line 1044: run
> nit: "are running" or just "run"
Done
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1047
PS7, Line 1047: // scan mode, from leader replica. In this test writes are
> nit: comma after "scan mode"
Done
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1062
PS7, Line 1062: // Similar to testReadYourWritesSyncLeaderReplica, but in this
: // test writes are performed in MANUAL_FLUSH (batches) flush modes.
: @Test(timeout = 100000)
: public void testReadYourWritesBa
> This text is repeated. Likely add it to the first test and then make the ot
Done
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1086
PS7, Line 1086:
> avoid magic numbers (set a final var)
Done
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 8
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 06 Mar 2018 00:35:23 +0000
Gerrit-HasComments: Yes
[kudu-CR] [WIP]KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: [WIP]KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/8847/1/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/8847/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@132
PS1, Line 132: * Reads in this mode are not repeatable: two stale reads, even if they
: * provide the same propagated timestamp bound, can execute at different
: * timestamps and thus return inconsistent results. However, it allows
: * read-your-writes for each client, as the picked timestamp must be higher
: * than the one of the last write or read, known from the propagated timestamp.
> let's reword this according to what we discussed in the other patch
Done
http://gerrit.cloudera.org:8080/#/c/8847/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@138
PS1, Line 138: * In ACID terms this corresponds to Isolation mode: "Read Committed"
> This isn't true, is it?
Done
http://gerrit.cloudera.org:8080/#/c/8847/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/8847/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@986
PS1, Line 986: testScanOwnSyncWrites
> nit: testReadOwnWritesSync?
Done
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 12 Jan 2018 03:41:54 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 7:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/8847/7/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/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@133
PS7, Line 133: return
nit: s/thus return/thus may return/
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@200
PS7, Line 200: propagationTimestamp
in the c++ client this has a different name, right? please keep names for methods/fields as strictly consistent between clients as possible
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@315
PS7, Line 315: unnecessarily wait.
grammar
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@415
PS7, Line 415: updates
s/updates/update
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@418
PS7, Line 418: if (readMode == ReadMode.READ_YOUR_WRITES &&
: resp.scanTimestamp != AsyncKuduClient.NO_TIMESTAMP) {
: client.updateLastPropagatedTimestamp(resp.scanTimestamp);
: } else if (resp.propagatedTimestamp != AsyncKuduClient.NO_TIMESTAMP) {
: client.updateLastPropagatedTimestamp(resp.propagatedTimestamp);
: }
this could be slightly simplified (choose a timestamp with the ifs, call client.updateLastPropagatedTimestamp() only once. more generally though I'm not sure I understand what this is doing, likely worth it to add a comment explaining when and why we get these timestamps and why we chose the one we chose.
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@817
PS7, Line 817: // If the last propagated timestamp is set send it with the scan.
nit, comma after "set"
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@818
PS7, Line 818: propagation timestamp
pet peeve: propagated timestamp or propagation timestamp? are these different? if not maybe choose one and use it consistently, likely too late for the other client, but still on time for this one.
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1044
PS7, Line 1044: running
nit: "are running" or just "run"
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1047
PS7, Line 1047: // scan mode from leader replica. In this test writes are
nit: comma after "scan mode"
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1062
PS7, Line 1062: // This is a test that verifies, when multiple clients running
: // simultaneously, a client can get read-your-writes and
: // read-your-reads session guarantees using READ_YOUR_WRITES
: // scan mode from leader replica
This text is repeated. Likely add it to the first test and then make the other tests point to it.
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1080
PS7, Line 1080: readYourWrites
do other java tests that have concurrency also use raw threads? or do they use executors/tasks and futures?
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1086
PS7, Line 1086: 4
avoid magic numbers (set a final var)
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1150
PS7, Line 1150: // Fail the main thread if ever encounter AssertionError.
: if (assertionError != null) {
: fail("the test should not throw AssertionError: " + assertionError);
: }
hum, not sure what this is doing...
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 05 Mar 2018 23:14:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8847
to look at the new patch set (#6).
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
KUDU-1704: add java client support for READ_YOUR_WRITES mode
Change-Id: I6239521c022147257859e399f55c6f3f945af465
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
4 files changed, 229 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/8847/6
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 6
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [WIP] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8847
to look at the new patch set (#3).
Change subject: [WIP] KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
[WIP] KUDU-1704: add java client support for READ_YOUR_WRITES mode
Change-Id: I6239521c022147257859e399f55c6f3f945af465
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
4 files changed, 135 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/8847/3
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1150
PS7, Line 1150: // Fail the main thread if ever encounter AssertionError.
: if (assertionError != null) {
: fail("the test should not throw AssertionError: " + assertionError);
: }
> Since the JUnit only captures assertion errors in the main thread of the te
right this kind of mess is why futures are much better (and the impl in line 918 seems to have a bug btw).
you could not catch the exception in the thread and just have
ExecutorService threadPool = Executors.newFixedThreadPool(3);
Future<?> future = threadPool.submit(new MyTest());
when you do future.get() if an exception occurred it'll propagate to the caller thread, no need to catch any exception inside runnable
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:43:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1150
PS7, Line 1150:
:
:
:
> right this kind of mess is why futures are much better (and the impl in lin
I see, makes sense. Updated.
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 08 Mar 2018 00:48:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 4:
I'm slacking on this review a bit until we have something solid on the c++ side (which we seem close to now). it's easier to review by analogy :)
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 21 Feb 2018 00:44:02 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 7:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/8847/7/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/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@133
PS7, Line 133: return
> nit: s/thus return/thus may return/
Done
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@200
PS7, Line 200: propagationTimestamp
> in the c++ client this has a different name, right? please keep names for m
Done
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@315
PS7, Line 315: unnecessarily wait.
> grammar
Done
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@415
PS7, Line 415: updates
> s/updates/update
Done
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@418
PS7, Line 418: if (readMode == ReadMode.READ_YOUR_WRITES &&
: resp.scanTimestamp != AsyncKuduClient.NO_TIMESTAMP) {
: client.updateLastPropagatedTimestamp(resp.scanTimestamp);
: } else if (resp.propagatedTimestamp != AsyncKuduClient.NO_TIMESTAMP) {
: client.updateLastPropagatedTimestamp(resp.propagatedTimestamp);
: }
> this could be slightly simplified (choose a timestamp with the ifs, call cl
I guess you are saying the comment above is not clear enough. Will update. So in general, for READ_YOUR_WRITES since we return the chosen snapshot timestamp, we should use it for as the propagation timestamp to avoid bump up the propagation timestamp unnecessarily. Since for RYW scan it is good, as long as the chosen timestamp of next read is greater than the previous one.
There is an identical flow in C++ Client https://gerrit.cloudera.org/#/c/8823/20/src/kudu/client/scanner-internal.cc@491
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1080
PS7, Line 1080: readYourWrites
> do other java tests that have concurrency also use raw threads? or do they
There is another java test inside this test class use raw threads. https://gerrit.cloudera.org/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@918
http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1150
PS7, Line 1150: // Fail the main thread if ever encounter AssertionError.
: if (assertionError != null) {
: fail("the test should not throw AssertionError: " + assertionError);
: }
> hum, not sure what this is doing...
Since the JUnit only captures assertion errors in the main thread of the test, and it is not aware of exceptions from within new spawn threads. I added this line to fail the main thread if there is any error in the spawn ones. Will update the comment to be more clear.
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 05 Mar 2018 23:59:21 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8847
to look at the new patch set (#5).
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
KUDU-1704: add java client support for READ_YOUR_WRITES mode
Change-Id: I6239521c022147257859e399f55c6f3f945af465
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
4 files changed, 196 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/8847/5
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [WIP] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: [WIP] KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 3: Verified+1
Override it as the failed test DeleteTableITest.TestNoDeleteTombstonedTablets is not related to the change.
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 12 Jan 2018 05:48:39 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8847
to look at the new patch set (#10).
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
KUDU-1704: add java client support for READ_YOUR_WRITES mode
Change-Id: I6239521c022147257859e399f55c6f3f945af465
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
4 files changed, 236 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/8847/10
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8847
to look at the new patch set (#9).
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
KUDU-1704: add java client support for READ_YOUR_WRITES mode
Change-Id: I6239521c022147257859e399f55c6f3f945af465
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
4 files changed, 233 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/8847/9
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG@6
PS7, Line 6:
: KUDU-1704: add java client support for READ_YOUR_WRITES mode
more generally, how's the jepsen test going? it'd be a great validation of this client's implementation, since it uses it.
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 05 Mar 2018 23:15:27 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8847
to look at the new patch set (#4).
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
KUDU-1704: add java client support for READ_YOUR_WRITES mode
Change-Id: I6239521c022147257859e399f55c6f3f945af465
---
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/KuduScanToken.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
5 files changed, 230 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/8847/4
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 8: Verified+1
The failed test is unrelated flaky
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 8
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 06 Mar 2018 01:26:18 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 10:
as discussed offline, ok with merging this as long as we make sure that we're going to finish the jepsen part of the tests
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 12 Mar 2018 21:07:00 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 12 Mar 2018 21:06:25 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8847
to look at the new patch set (#8).
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
KUDU-1704: add java client support for READ_YOUR_WRITES mode
Change-Id: I6239521c022147257859e399f55c6f3f945af465
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
4 files changed, 241 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/8847/8
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 8
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [WIP]KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8847
to look at the new patch set (#2).
Change subject: [WIP]KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
[WIP]KUDU-1704: add java client support for READ_YOUR_WRITES mode
Change-Id: I6239521c022147257859e399f55c6f3f945af465
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
4 files changed, 135 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/8847/2
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [WIP] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has removed a vote on this change.
Change subject: [WIP] KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 9:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/8847/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/8847/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1085
PS9, Line 1085: tasks_num
tiny nit: java uses camel case :)
http://gerrit.cloudera.org:8080/#/c/8847/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1144
PS9, Line 1144: future.get();
add a note indicating that any exceptions or assertion error will bubble up here.
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 08 Mar 2018 21:18:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG@6
PS7, Line 6:
: KUDU-1704: add java client support for READ_YOUR_WRITES mode
> Unfortunately, did not get a change to spend much time on it yet, but shoul
ok, thats fine. really want to make sure we don't drop it though, if we're willing to go the distance in implementing RYW we should also test it properly. super weird corner cases often pop up and jepsen is one of the best tools to let you know that went wrong and why.
http://gerrit.cloudera.org:8080/#/c/8847/7/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/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@418
PS7, Line 418: if (readMode == ReadMode.READ_YOUR_WRITES &&
: resp.scanTimestamp != AsyncKuduClient.NO_TIMESTAMP) {
: client.updateLastPropagatedTimestamp(resp.scanTimestamp);
: } else if (resp.propagatedTimestamp != AsyncKuduClient.NO_TIMESTAMP) {
: client.updateLastPropagatedTimestamp(resp.propagatedTimestamp);
: }
> I guess you are saying the comment above is not clear enough. Will update.
yeah, that's why I said slightly simplified. I agree that we might want to keep the flow for consistency.
the point about the comment was more important though
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 06 Mar 2018 00:13:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
KUDU-1704: add java client support for READ_YOUR_WRITES mode
Change-Id: I6239521c022147257859e399f55c6f3f945af465
Reviewed-on: http://gerrit.cloudera.org:8080/8847
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <da...@gmail.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/KuduScanToken.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
4 files changed, 236 insertions(+), 7 deletions(-)
Approvals:
Kudu Jenkins: Verified
David Ribeiro Alves: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 11
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 10:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/8847/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/8847/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1085
PS9, Line 1085: tasksNum
> tiny nit: java uses camel case :)
Right... thanks for catching it.
http://gerrit.cloudera.org:8080/#/c/8847/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1144
PS9, Line 1144: Waits for th
> add a note indicating that any exceptions or assertion error will bubble up
Done
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 08 Mar 2018 21:50:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ OWN WRITES mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_OWN_WRITES mode
......................................................................
Patch Set 1:
(3 comments)
I know you're rewording some of this but had these unposted comments. might as well post them
http://gerrit.cloudera.org:8080/#/c/8847/1/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/8847/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@132
PS1, Line 132: * Reads in this mode are not repeatable: two stale reads, even if they
: * provide the same propagated timestamp bound, can execute at different
: * timestamps and thus return inconsistent results. However, it allows
: * read-your-writes for each client, as the picked timestamp must be higher
: * than the one of the last write or read, known from the propagated timestamp.
let's reword this according to what we discussed in the other patch
http://gerrit.cloudera.org:8080/#/c/8847/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@138
PS1, Line 138: * In ACID terms this corresponds to Isolation mode: "Read Committed"
This isn't true, is it?
http://gerrit.cloudera.org:8080/#/c/8847/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/8847/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@986
PS1, Line 986: testScanOwnSyncWrites
nit: testReadOwnWritesSync?
same below
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 02 Jan 2018 22:35:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG@6
PS7, Line 6:
: KUDU-1704: add java client support for READ_YOUR_WRITES mode
> more generally, how's the jepsen test going? it'd be a great validation of
Unfortunately, did not get a change to spend much time on it yet, but should wrap up other things I am working on soon.
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 06 Mar 2018 00:01:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 )
Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
......................................................................
Patch Set 10: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/8847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 08 Mar 2018 23:45:49 +0000
Gerrit-HasComments: No