[kudu-CR] KUDU-1644 use range partition info for pruning
wangning has posted comments on this change. ( http://gerrit.cloudera.org:8080/16903 ) Change subject: KUDU-1644 use range partition info for pruning .. Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc File src/kudu/common/scan_spec-test.cc: http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc@443 PS3, Line 443: // Verify the splitted values can merge into original set without overlapping. > nit:original Done http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc@473 PS3, Line 473: // both hash and range aspects. > nit:on both hash and range aspects? rather than either. Done http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc@694 PS3, Line 694: // on both hash and range aspects. > See above comment. Done -- To view, visit http://gerrit.cloudera.org:8080/16903 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10 Gerrit-Change-Number: 16903 Gerrit-PatchSet: 4 Gerrit-Owner: wangning <1994wangn...@gmail.com> Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: wangning <1994wangn...@gmail.com> Gerrit-Comment-Date: Thu, 07 Jan 2021 07:42:00 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1644 use range partition info for pruning
Hello Mahesh Reddy, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16903 to look at the new patch set (#4). Change subject: KUDU-1644 use range partition info for pruning .. KUDU-1644 use range partition info for pruning When pruning in-list predicate values, range partition can also be taken into consideration. Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10 --- M src/kudu/common/partition.cc M src/kudu/common/partition.h M src/kudu/common/scan_spec-test.cc M src/kudu/common/scan_spec.cc M src/kudu/common/scan_spec.h M src/kudu/tools/kudu-admin-test.cc M src/kudu/tserver/tablet_service.cc 7 files changed, 662 insertions(+), 277 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/16903/4 -- To view, visit http://gerrit.cloudera.org:8080/16903 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10 Gerrit-Change-Number: 16903 Gerrit-PatchSet: 4 Gerrit-Owner: wangning <1994wangn...@gmail.com> Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: wangning <1994wangn...@gmail.com>
[kudu-CR] [transactions] remove cached TransactionEntry's state
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16928 ) Change subject: [transactions] remove cached TransactionEntry's state .. [transactions] remove cached TransactionEntry's state With changelist fa21bf618, an atomic field was added to TransactionEntry to contain a cached state for an entry. The motivation was to provide lock-free access the state of the entry: accessing the metadata stored as a COW object in the 'metadata_' field involves taking read lock for that. The transaction keepalive tracking task needs to check the status of an entry every time it runs (10 seconds by default), and there may be many txn records records. However, that approach didn't provide an automatic update of the cached field upon modification of the underlying 'metadata_' object, and updating that manually proved to be cumbersome and bug prone. A few alternatives to update the cached field automatically were considered, but after realizing that we don't have hard data showing a lot of lock contention or resources spent on lock acquisition, the former approach was deemed to be a premature optimization. With this changelist, the cached field is removed and the TransactionEntry::state() accessor now takes a read lock to access the state of the transactional entry. Change-Id: Iab1ffe4312d21b732d7f37f7b54f28e43c979e35 Reviewed-on: http://gerrit.cloudera.org:8080/16928 Reviewed-by: Andrew Wong Tested-by: Kudu Jenkins --- M src/kudu/transactions/txn_status_entry.cc M src/kudu/transactions/txn_status_entry.h M src/kudu/transactions/txn_status_manager.cc 3 files changed, 9 insertions(+), 22 deletions(-) Approvals: Andrew Wong: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/16928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iab1ffe4312d21b732d7f37f7b54f28e43c979e35 Gerrit-Change-Number: 16928 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [transactions] remove cached TransactionEntry's state
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16928 ) Change subject: [transactions] remove cached TransactionEntry's state .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/16928/1//COMMIT_MSG Commit Message: PS1: Thanks for adding the context here! -- To view, visit http://gerrit.cloudera.org:8080/16928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iab1ffe4312d21b732d7f37f7b54f28e43c979e35 Gerrit-Change-Number: 16928 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 07 Jan 2021 02:27:34 + Gerrit-HasComments: Yes
[kudu-CR] [transactions] remove cached TransactionEntry's state
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16928 Change subject: [transactions] remove cached TransactionEntry's state .. [transactions] remove cached TransactionEntry's state With changelist fa21bf618, an atomic field was added to TransactionEntry to contain a cached state for an entry. The motivation was to provide lock-free access the state of the entry: accessing the metadata stored as a COW object in the 'metadata_' field involves taking read lock for that. The transaction keepalive tracking task needs to check the status of an entry every time it runs (10 seconds by default), and there may be many txn records records. However, that approach didn't provide an automatic update of the cached field upon modification of the underlying 'metadata_' object, and updating that manually proved to be cumbersome and bug prone. A few alternatives to update the cached field automatically were considered, but after realizing that we don't have hard data showing a lot of lock contention or resources spent on lock acquisition, the former approach was deemed to be a premature optimization. With this changelist, the cached field is removed and the TransactionEntry::state() accessor now takes a read lock to access the state of the transactional entry. Change-Id: Iab1ffe4312d21b732d7f37f7b54f28e43c979e35 --- M src/kudu/transactions/txn_status_entry.cc M src/kudu/transactions/txn_status_entry.h M src/kudu/transactions/txn_status_manager.cc 3 files changed, 9 insertions(+), 22 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/16928/1 -- To view, visit http://gerrit.cloudera.org:8080/16928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iab1ffe4312d21b732d7f37f7b54f28e43c979e35 Gerrit-Change-Number: 16928 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-2612 Java client transaction API
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 ) Change subject: KUDU-2612 Java client transaction API .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1181 PS2, Line 1181: let = Preconditions.c > After thinking about this a bit more, I found that for asynchronous program Yes, I was wondering why we have a blocking call in the class that was meant for async ones. Your suggestion make sense to me. Thanks a lot! -- To view, visit http://gerrit.cloudera.org:8080/16894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07 Gerrit-Change-Number: 16894 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 07 Jan 2021 00:39:39 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 Java client transaction API
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Hao Hao, Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16894 to look at the new patch set (#5). Change subject: KUDU-2612 Java client transaction API .. KUDU-2612 Java client transaction API This patch focused on the API than the actual functionality under the hood. The functionality to do the heavy-lifting (i.e. issuing RPC calls to TxnManager, retrying in case of transient errors, tests, etc.) will be posted as a separate patch as per our discussion with Andrew and Hao. The proposed API is mirroring the API for the C++ client with a few twists because of the presence of the async-style objects in the Kudu Java client API. The asynchronous API bindings (i.e. bindings with Deferred) aren't provided in this patch. I'm not sure it makes any sense in investing in that at this point given that I'm not aware of any users of the asynchrnous Kudu client API except for Java Kudu client itself. If necessary, we can add AsyncKuduTransaction with appropriate semantics later on. Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java 4 files changed, 429 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/16894/5 -- To view, visit http://gerrit.cloudera.org:8080/16894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07 Gerrit-Change-Number: 16894 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong
[kudu-CR] KUDU-2612 Java client transaction API
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Hao Hao, Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16894 to look at the new patch set (#4). Change subject: KUDU-2612 Java client transaction API .. KUDU-2612 Java client transaction API This patch focused on the API than the actual functionality under the hood. The functionality to do the heavy-lifting (i.e. issuing RPC calls to TxnManager, retrying in case of transient errors, tests, etc.) will be posted as a separate patch as per our discussion with Andrew and Hao. The proposed API is mirroring the API for the C++ client with a few twists because of the presence of the async-style objects in the Kudu Java client API. The asynchronous API bindings (i.e. bindings with Deferred) aren't provided in this patch. I'm not sure it makes any sense in investing in that at this point given that I'm not aware of any users of the asynchrnous Kudu client API except for Java Kudu client itself. If necessary, we can add AsyncKuduTransaction with appropriate semantics later on. Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java 4 files changed, 425 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/16894/4 -- To view, visit http://gerrit.cloudera.org:8080/16894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07 Gerrit-Change-Number: 16894 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong
[kudu-CR] KUDU-2612 Java client transaction API
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 ) Change subject: KUDU-2612 Java client transaction API .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1181 PS2, Line 1181: This call is blocking > OK, then I'll change it to returning Deferred as well. After thinking about this a bit more, I found that for asynchronous programming it's necessary to provide asynchronous interface for all transactional operations as well. I'm not sure I want to invest time in that right now. So, I'm removing newTransaction() method from AsyncKuduClient and leave newTransaction() in KuduClient that returns regular synchronous-style interface for operations. -- To view, visit http://gerrit.cloudera.org:8080/16894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07 Gerrit-Change-Number: 16894 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 06 Jan 2021 23:30:41 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1644 use range partition info for pruning
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16903 ) Change subject: KUDU-1644 use range partition info for pruning .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc File src/kudu/common/scan_spec-test.cc: http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc@443 PS3, Line 443: // Verify the splitted values can merge into originl set without overlapping. nit:original http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc@473 PS3, Line 473: // either hash and range aspects. nit:on both hash and range aspects? rather than either. http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc@694 PS3, Line 694: // on either hash and range aspects. See above comment. -- To view, visit http://gerrit.cloudera.org:8080/16903 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10 Gerrit-Change-Number: 16903 Gerrit-PatchSet: 3 Gerrit-Owner: wangning <1994wangn...@gmail.com> Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: wangning <1994wangn...@gmail.com> Gerrit-Comment-Date: Wed, 06 Jan 2021 22:31:06 + Gerrit-HasComments: Yes
[kudu-CR] [docker] Add support for openSUSE in the Docker build
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16885 ) Change subject: [docker] Add support for openSUSE in the Docker build .. Patch Set 4: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-runtime-env.sh File docker/bootstrap-runtime-env.sh: http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-runtime-env.sh@95 PS2, Line 95: openssl > krb5-devel might be able to be simplified to a smaller runtime only depende I was curious about the status of those -devel packages: sometimes it seemed like they are in the list for run-time packages, sometimes not. But I guess in most cases I was confusing those environments :) I'm totally OK if addressing this in a follow-up changelist. And I guess there isn't any rush for that. -- To view, visit http://gerrit.cloudera.org:8080/16885 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c1ce282fda67db5043445a139c888bc74c44680 Gerrit-Change-Number: 16885 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 06 Jan 2021 22:30:00 + Gerrit-HasComments: Yes
[kudu-CR] [docs] Add note about Javadoc compatibility
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16924 ) Change subject: [docs] Add note about Javadoc compatibility .. [docs] Add note about Javadoc compatibility An earlier commit (3d344ecefe) disabled building javadoc on JDK 10+ due to some compatibility issues. When attempting to build javadoc, the actual building will be skipped and it will fail making the docs. If the javadoc build is re-enalbed for JDK 10+, it fails with the below error: javadoc: error - Class org.apache.yetus.audience.tools.IncludePublicAnnotationsStandardDoclet is not a valid doclet. Note: As of JDK 13, the com.sun.javadoc API is no longer supported. This commit adds a note about this compatibility issue to the docs building readme to make sure people build docs on JDK 8. Change-Id: I1a84edd44e890c37fcf857f7904b6de10948f0fb Reviewed-on: http://gerrit.cloudera.org:8080/16924 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke --- M README.adoc 1 file changed, 3 insertions(+), 0 deletions(-) Approvals: Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1a84edd44e890c37fcf857f7904b6de10948f0fb Gerrit-Change-Number: 16924 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2612 Java client transaction API
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 ) Change subject: KUDU-2612 Java client transaction API .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1181 PS2, Line 1181: This call is blocking > I like the idea of keeping the deferred purity of the interface. OK, then I'll change it to returning Deferred as well. Thank you for the feedback! -- To view, visit http://gerrit.cloudera.org:8080/16894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07 Gerrit-Change-Number: 16894 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 06 Jan 2021 20:34:18 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 Java client transaction API
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16894 )
Change subject: KUDU-2612 Java client transaction API
..
Patch Set 2:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@43
PS2, Line 43: @c
> typo?
Done
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@47
PS2, Line 47: soon
> when is soon explicitly?
Done
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@50
PS2, Line 50: @InterfaceStability.Evolving
> Should we mark this as InterfaceStability.Unstable until the transaction fe
It's a good point, thanks.
Done.
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@70
PS2, Line 70: this.client = client;
> when does txnId and keepaliveMillis get set when using this constructor?
This is done after receiving response for the BeginTransaction() RPC. I was
planning to have the actual functionality in this patch, but after recent
sync-up with Andrew and Hao there was a decision to keep this patch as a hollow
API-only shell.
The actual functionality comes in a follow-up patch.
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@248
PS2, Line 248: rolling back a transaction
> Is close only called when rolling back?
This is an override for AutoCloseable::close(). It's possible to call
'close()' explicitly, and it's totally fine to call
'KuduTransaction::rollback()' at any time explicitly when needed: e.g., when an
exception is thrown by a write operation created from in transactional
KuduSession.
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java
File
java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java@108
PS2, Line 108: public void setEnableKeepalive(boolean enableKeepalive) {
> Why isn't this toggled on the transaction itself?
A couple of reasons:
* in case of newly created transaction, the keepalive heartbeating should be
automatically enabled always, so not much sense in such a control there
* in case of the 'star' topology, when deserializing a transaction from a
token there isn't much sense of sending keepalive messages for every txn
created out of token once the transaction commit/rollback is controlled by the
top-level txn (e.g., the transaction started by Spark driver), otherwise it
will lead to duplicated keepalive messages
Basically, the only case when it's necessary to enable keepalive for a
transaction created by deserializing a txn token is the 'ring' topology.
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java@134
PS2, Line 134: public byte[] serialize() throws IOException {
> What's the reason for a separate class for this versus putting in KuduTrans
Because of the consistency reasons.
(a) It's necessary to have the control over the keepalive property for a
transaction deserialized from a token, where the creator of the token (not the
caller of the deserialize() method) controls the keepalive behavior
(b) adding serialize() into the KuduTransaction() along with keepalive control
makes the deserialize() method inconsistent since it makes the caller think
that deserialize() is also affected by the setEnableKeepalive() settings.
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTransaction.java
File
java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTransaction.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTransaction.java@31
PS2, Line 31: public class TestKuduTransaction {
> Does it make sense to add a test for the expected behavior when the transac
Yeah, many more tests are being added.
I'll add the tests you mentioned in the follow-up patch with the actual
functionality. Since PS3 this becomes a hollow API shell-only patch -- I'm
moving all the tests out into the follow-up patch.
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
[kudu-CR] (wip) KUDU-2612: restrict TxnStatusManager calls to be made by the leader only
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16648 ) Change subject: (wip) KUDU-2612: restrict TxnStatusManager calls to be made by the leader only .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/16648/3/src/kudu/integration-tests/txn_status_table-itest.cc File src/kudu/integration-tests/txn_status_table-itest.cc: http://gerrit.cloudera.org:8080/#/c/16648/3/src/kudu/integration-tests/txn_status_table-itest.cc@725 PS3, Line 725: FLAGS_leader_failure_max_missed_heartbeat_periods = 1; : FLAGS_raft_heartbeat_interval_ms = 30; I'm curious: do this settings provide stable behavior under TSAN/ASAN? http://gerrit.cloudera.org:8080/#/c/16648/3/src/kudu/integration-tests/txn_status_table-itest.cc@749 PS3, Line 749: (int t = 0; t < 5; t++) Any idea how many re-elections happen while these transaction are being started? In other words, maybe make every thread starting several transactions in succession, spaced by some interval (e.g., 3 * raft_heartbeat_interval_ms) for better coverage? http://gerrit.cloudera.org:8080/#/c/16648/3/src/kudu/integration-tests/txn_status_table-itest.cc@756 PS3, Line 756: ASSERT_OK(s); : ASSERT_TRUE(txn_status.has_user()); : ASSERT_STREQ(kUser, txn_status.user().c_str()); : ASSERT_TRUE(txn_status.has_state()); : ASSERT_EQ(TxnStatePB::OPEN, txn_status.state()); I'm curious how does this scenario behave when any of this fails? How to join the threads in such case? I remember one of the approaches to address that is either using CHECK instead of ASSERT, so the test crashes, or simply increment failure count and then report on those once every thread is complete. -- To view, visit http://gerrit.cloudera.org:8080/16648 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I42c1ad095dcb4bdffcbe0ecf9631a60bac208c2a Gerrit-Change-Number: 16648 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 06 Jan 2021 19:12:35 + Gerrit-HasComments: Yes
[kudu-CR] [build] Automate using GCC-8 on SLES 12
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16927 ) Change subject: [build] Automate using GCC-8 on SLES 12 .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/16927/2/build-support/enable_devtoolset.sh File build-support/enable_devtoolset.sh: http://gerrit.cloudera.org:8080/#/c/16927/2/build-support/enable_devtoolset.sh@46 PS2, Line 46: "$CC" -a ! "$CXX" > I thought about this but chose to match the existing behavior and expect th yep, this makes sense to me http://gerrit.cloudera.org:8080/#/c/16927/2/build-support/enable_devtoolset.sh@53 PS2, Line 53: /usr/bin/gcc-8 > I was on the fence about it. We can definitely do that in a follow up and t ack -- To view, visit http://gerrit.cloudera.org:8080/16927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I50237cc900c9450bf1b5fbc7866353c17e3ef10b Gerrit-Change-Number: 16927 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 06 Jan 2021 18:45:23 + Gerrit-HasComments: Yes
[kudu-CR] [build] Automate using GCC-8 on SLES 12
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16927 ) Change subject: [build] Automate using GCC-8 on SLES 12 .. [build] Automate using GCC-8 on SLES 12 When building Kudu on SLES 12 a newer version of GCC is required. This patch adjusts the documentation and automates the use of GCC-8 via the `enable_devtoolset.sh` script. This matches how we use newer dev toolsets on RHEL based systems. Change-Id: I50237cc900c9450bf1b5fbc7866353c17e3ef10b Reviewed-on: http://gerrit.cloudera.org:8080/16927 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- A build-support/ccache-gcc-8/c++ A build-support/ccache-gcc-8/cc M build-support/enable_devtoolset.sh M docs/installation.adoc M thirdparty/preflight.py 5 files changed, 92 insertions(+), 16 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I50237cc900c9450bf1b5fbc7866353c17e3ef10b Gerrit-Change-Number: 16927 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Automate using GCC-8 on SLES 12
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16927 ) Change subject: [build] Automate using GCC-8 on SLES 12 .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/16927/2/build-support/enable_devtoolset.sh File build-support/enable_devtoolset.sh: http://gerrit.cloudera.org:8080/#/c/16927/2/build-support/enable_devtoolset.sh@46 PS2, Line 46: "$CC" -a ! "$CXX" > Just to make sure: we don't want to handle the case case if CC is set but C I thought about this but chose to match the existing behavior and expect that case to be rare. We can improve this script in follow up changes to be more flexible. http://gerrit.cloudera.org:8080/#/c/16927/2/build-support/enable_devtoolset.sh@53 PS2, Line 53: /usr/bin/gcc-8 > nit: may it be a case when /usr/bin/gcc-8 isn't available? Should we bail I was on the fence about it. We can definitely do that in a follow up and the failure should be obvious if it is missing as is too. -- To view, visit http://gerrit.cloudera.org:8080/16927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I50237cc900c9450bf1b5fbc7866353c17e3ef10b Gerrit-Change-Number: 16927 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 06 Jan 2021 18:07:18 + Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-2181 Procedure for copying sys catalog on adding master
Bankim Bhavsar has posted comments on this change. (
http://gerrit.cloudera.org:8080/16830 )
Change subject: [master] KUDU-2181 Procedure for copying sys catalog on adding
master
..
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16830/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16830/6//COMMIT_MSG@26
PS6, Line 26: If not,
: above steps will have to be repeated.
> I see. It probably makes sense to build this verification into a small tool
CLI tool to add master that invokes ChangeConfig can definitely include this
verification step.
http://gerrit.cloudera.org:8080/#/c/16830/7/src/kudu/master/dynamic_multi_master-test.cc
File src/kudu/master/dynamic_multi_master-test.cc:
http://gerrit.cloudera.org:8080/#/c/16830/7/src/kudu/master/dynamic_multi_master-test.cc@173
PS7, Line 173: int64_t updated_gc_count;
: while (MonoTime::Now() < deadline) {
: updated_gc_count = get_sys_catalog_wal_gc_count();
: if (updated_gc_count > orig_gc_count) {
: break;
: }
> Could we check this metric before creating each table? Then we wouldn't nee
Done
--
To view, visit http://gerrit.cloudera.org:8080/16830
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I142c1dec442ec72c38c5be9d62cdf270e441d6e3
Gerrit-Change-Number: 16830
Gerrit-PatchSet: 7
Gerrit-Owner: Bankim Bhavsar
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 06 Jan 2021 17:55:16 +
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-2181 Procedure for copying sys catalog on adding master
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16830 to look at the new patch set (#8). Change subject: [master] KUDU-2181 Procedure for copying sys catalog on adding master .. [master] KUDU-2181 Procedure for copying sys catalog on adding master This change outlines procedure to copy system catalog for the newly added master using existing CLI tools and the master ChangeConfig RPC. Only functional change is to hookup the master runtime flag --master_consensus_allow_status_msg_for_failed_peer to Raft consensus. New master could go into a FAILED_RECOVERABLE state if the leader master's system catalog WAL has been GC'ed. This change allows the new master to be promoted after copying the system catalog externally. Outline of the test procedure: 1) Runtime flag --master_consensus_allow_status_msg_for_failed_peer must be turned on for existing masters. 2) Start the new master with --master_address_add_new_master= and --master_addresses that contains itself and existing masters. 3) Invoke ChangeConfig to add the master. 4) Verify the new master is part of the Raft config even if it's a LEARNER/NON_VOTER or goes into FAILED_RECOVERABLE state. If not, above steps will have to be repeated. 5) If the new master is promoted to being a VOTER then following tablet copy steps can be skipped. 6) Shutdown the new master. 7) Delete the system catalog on the new master. 8) Copy the system catalog from the leader master to the new master. 9) Bring up the new master. 10) Verify the new master is promoted as VOTER. If the new master doesn't get promoted to a VOTER then double check whether the new master is part of the Raft config for masters by running "kudu master list". - If yes, repeat procedure from step 6. - Else repeat the entire procedure Change-Id: I142c1dec442ec72c38c5be9d62cdf270e441d6e3 --- M src/kudu/master/dynamic_multi_master-test.cc M src/kudu/master/sys_catalog.cc 2 files changed, 300 insertions(+), 141 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/16830/8 -- To view, visit http://gerrit.cloudera.org:8080/16830 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I142c1dec442ec72c38c5be9d62cdf270e441d6e3 Gerrit-Change-Number: 16830 Gerrit-PatchSet: 8 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Automate using GCC-8 on SLES 12
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16927 ) Change subject: [build] Automate using GCC-8 on SLES 12 .. Patch Set 2: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/16927/2/build-support/enable_devtoolset.sh File build-support/enable_devtoolset.sh: http://gerrit.cloudera.org:8080/#/c/16927/2/build-support/enable_devtoolset.sh@46 PS2, Line 46: "$CC" -a ! "$CXX" Just to make sure: we don't want to handle the case case if CC is set but CXX isn't, right? I guess such environment may bring some inconsistent result down the road, but I guess this script isn't the place to handle that anyways. http://gerrit.cloudera.org:8080/#/c/16927/2/build-support/enable_devtoolset.sh@53 PS2, Line 53: /usr/bin/gcc-8 nit: may it be a case when /usr/bin/gcc-8 isn't available? Should we bail early (i.e. right here) in such case? -- To view, visit http://gerrit.cloudera.org:8080/16927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I50237cc900c9450bf1b5fbc7866353c17e3ef10b Gerrit-Change-Number: 16927 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 06 Jan 2021 17:55:00 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 Java client transaction API
Grant Henke has posted comments on this change. (
http://gerrit.cloudera.org:8080/16894 )
Change subject: KUDU-2612 Java client transaction API
..
Patch Set 2:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1181
PS2, Line 1181: This call is blocking
> You mean we should follow the suite of majority of methods in this interfac
I like the idea of keeping the deferred purity of the interface.
I can imagine someone writing an async application that writes to kudu which
asynchronously chains together opening, writing, and closing a transaction. It
could also be useful to unify the handling of exceptions when making calls to
this interface.
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@43
PS2, Line 43: @c
typo?
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@47
PS2, Line 47: soon
when is soon explicitly?
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@50
PS2, Line 50: @InterfaceStability.Evolving
Should we mark this as InterfaceStability.Unstable until the transaction
feature is ready for use?
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@70
PS2, Line 70: this.client = client;
when does txnId and keepaliveMillis get set when using this constructor?
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@248
PS2, Line 248: rolling back a transaction
Is close only called when rolling back?
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java
File
java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java@108
PS2, Line 108: public void setEnableKeepalive(boolean enableKeepalive) {
Why isn't this toggled on the transaction itself?
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java@134
PS2, Line 134: public byte[] serialize() throws IOException {
What's the reason for a separate class for this versus putting in
KuduTransaction next to the deserialize method?
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTransaction.java
File
java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTransaction.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTransaction.java@31
PS2, Line 31: public class TestKuduTransaction {
Does it make sense to add a test for the expected behavior when the transaction
feature is missing/disabled server side?
I think it would be good to test the auto close functionality too.
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 06 Jan 2021 17:51:40 +
Gerrit-HasComments: Yes
[kudu-CR] [consensus] Allow sending status-only request messages to FAILED peer
Bankim Bhavsar has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16899 ) Change subject: [consensus] Allow sending status-only request messages to FAILED peer .. [consensus] Allow sending status-only request messages to FAILED peer This change adds the ability for a leader to send status-only messages to a peer even if it's in FAILED_UNRECOVERABLE state. This ability is turned off by default and controlled via a PeerMessageQueue parameter. Without this change when the system catalog is copied externally the new master remains in FAILED_UNRECOVERABLE state and doesn't get promoted to being a VOTER despite the system catalog being up to date. The procedure for end-to-end testing that hooks up masters to use this Raft config is a separate change. Change-Id: I229cc739c1b5ec7b11ce05d5e6b1b8e9d654d6f7 Reviewed-on: http://gerrit.cloudera.org:8080/16899 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong --- M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h 5 files changed, 92 insertions(+), 10 deletions(-) Approvals: Kudu Jenkins: Verified Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16899 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I229cc739c1b5ec7b11ce05d5e6b1b8e9d654d6f7 Gerrit-Change-Number: 16899 Gerrit-PatchSet: 3 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [docs] Add note about Javadoc compatibility
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16924 ) Change subject: [docs] Add note about Javadoc compatibility .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a84edd44e890c37fcf857f7904b6de10948f0fb Gerrit-Change-Number: 16924 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 06 Jan 2021 16:57:05 + Gerrit-HasComments: No
[kudu-CR] [build] Automate using GCC-8 on SLES 12
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16927 to look at the new patch set (#2). Change subject: [build] Automate using GCC-8 on SLES 12 .. [build] Automate using GCC-8 on SLES 12 When building Kudu on SLES 12 a newer version of GCC is required. This patch adjusts the documentation and automates the use of GCC-8 via the `enable_devtoolset.sh` script. This matches how we use newer dev toolsets on RHEL based systems. Change-Id: I50237cc900c9450bf1b5fbc7866353c17e3ef10b --- A build-support/ccache-gcc-8/c++ A build-support/ccache-gcc-8/cc M build-support/enable_devtoolset.sh M docs/installation.adoc M thirdparty/preflight.py 5 files changed, 92 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/16927/2 -- To view, visit http://gerrit.cloudera.org:8080/16927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I50237cc900c9450bf1b5fbc7866353c17e3ef10b Gerrit-Change-Number: 16927 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [docker] Add support for openSUSE in the Docker build
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16885 ) Change subject: [docker] Add support for openSUSE in the Docker build .. Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-dev-env.sh File docker/bootstrap-dev-env.sh: http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-dev-env.sh@185 PS2, Line 185: zypper > OK, but then I'm curious: why do we use DEBIAN_FRONTEND for Ubuntu/Debian Ubuntu/Debian build hang on the kerberos and tzdata package installs if DEBIAN_FRONTEND isn't used. I don't see that problem here. http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-java-env.sh File docker/bootstrap-java-env.sh: http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-java-env.sh@82 PS2, Line 82: clean > To remove repo metadata as well, I guess. As per https://en.opensuse.org/S Done http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-runtime-env.sh File docker/bootstrap-runtime-env.sh: http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-runtime-env.sh@95 PS2, Line 95: openssl > Ah, I see -- it seems I confused run-time and development images. Does krb krb5-devel might be able to be simplified to a smaller runtime only dependency. I would like to do that in a follow up if you are okay with that. -- To view, visit http://gerrit.cloudera.org:8080/16885 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c1ce282fda67db5043445a139c888bc74c44680 Gerrit-Change-Number: 16885 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 06 Jan 2021 15:14:10 + Gerrit-HasComments: Yes
[kudu-CR] [docker] Add support for openSUSE in the Docker build
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16885 to look at the new patch set (#4). Change subject: [docker] Add support for openSUSE in the Docker build .. [docker] Add support for openSUSE in the Docker build This patch adds support for openSUSE/leap:15 as a base image in the Docker build. This is useful for testing and validating Kudu usage on openSUSE/SLES. I fixed the logic in `get_os_tag` to handle the illegal `/` character in the resulting tag. This results in a cleaner tag such as `apache/kudu:latest-leap15`. Change-Id: I9c1ce282fda67db5043445a139c888bc74c44680 --- M docker/bootstrap-dev-env.sh M docker/bootstrap-java-env.sh M docker/bootstrap-python-env.sh M docker/bootstrap-runtime-env.sh M docker/docker-build.py 5 files changed, 113 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/85/16885/4 -- To view, visit http://gerrit.cloudera.org:8080/16885 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9c1ce282fda67db5043445a139c888bc74c44680 Gerrit-Change-Number: 16885 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [Java] Upgrade dependencies
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16874 ) Change subject: [Java] Upgrade dependencies .. [Java] Upgrade dependencies Upgrades the Java dependencies and Gradle versions. Major version upgrades: - guava 29.0-jre -> 30.1-jre - scopt 3.7.1 -> 4.0.0 Minor version upgrades: - jmh 1.26 -> 1.27 - log4j 2.13.3 -> 2.14.0 - micrometer 1.5.5 -> 1.6.2 - mockito 3.5.13 -> 3.6.28 - protobuf 3.13.0 -> 3.14.0 - yetus 0.12.0 -> 0.13.0 Maintenance version upgrades: - jetty 9.4.32.v20200930 -> 9.4.35.v20201120 - netty 4.1.52.Final -> 4.1.56.Final - scalatest 3.2.2 -> 3.2.3 Gradle upgrades: - gradle 6.6.1 -> 6.7.1 - gradle-versions-plugin 0.33.0 -> 0.36.0 - gradle-protobuf-plugin 0.8.13 -> 0.8.14 - nebula-clojure-plugin 9.4.1 -> 9.4.2 - spotbugs-gradle-plugin 4.5.0 -> 4.6.0 - gradle-error-prone-plugin 1.2.1 -> 1.3.0 - gradle-animal-sniffer-plugin 1.5.1 -> 1.5.2 - jmh-gradle-plugin 0.5.1 -> 0.5.2 Change-Id: Ib4b319a67f629bcb60ae54187cee1c3257d6e1a2 Reviewed-on: http://gerrit.cloudera.org:8080/16874 Reviewed-by: Attila Bukor Tested-by: Kudu Jenkins --- M java/buildSrc/build.gradle M java/gradle/dependencies.gradle M java/gradle/wrapper/gradle-wrapper.properties M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestOptions.scala M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java 5 files changed, 27 insertions(+), 26 deletions(-) Approvals: Attila Bukor: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/16874 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ib4b319a67f629bcb60ae54187cee1c3257d6e1a2 Gerrit-Change-Number: 16874 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Automate using GCC-8 on SLES 12
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16927 Change subject: [build] Automate using GCC-8 on SLES 12 .. [build] Automate using GCC-8 on SLES 12 When building Kudu on SLES 12 a newer version of GCC is required. This patch adjusts the documentation and automates the use of GCC-8 via the `enable_devtoolset.sh` script. This matches how we use newer dev toolsets on RHEL based systems. Change-Id: I50237cc900c9450bf1b5fbc7866353c17e3ef10b --- A build-support/ccache-gcc-8/c++ A build-support/ccache-gcc-8/cc M build-support/enable_devtoolset.sh M docs/installation.adoc M thirdparty/preflight.py 5 files changed, 92 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/16927/1 -- To view, visit http://gerrit.cloudera.org:8080/16927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I50237cc900c9450bf1b5fbc7866353c17e3ef10b Gerrit-Change-Number: 16927 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] KUDU-1644 use range partition info for pruning
wangning has posted comments on this change. (
http://gerrit.cloudera.org:8080/16903 )
Change subject: KUDU-1644 use range partition info for pruning
..
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/16903/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16903/1//COMMIT_MSG@9
PS1, Line 9: When pruning in-list predicate values, range partition can also be
taken
> I can see this being particularly useful if range partitioning is defined p
We don't have heavy scenarios on range-schema based inList query. So I would do
a small scale benchmark about this patch.
http://gerrit.cloudera.org:8080/#/c/16903/1/src/kudu/common/scan_spec-test.cc
File src/kudu/common/scan_spec-test.cc:
http://gerrit.cloudera.org:8080/#/c/16903/1/src/kudu/common/scan_spec-test.cc@584
PS1, Line 584: AddInPredicate(, "a", { 0, 1, 2, 3, 4, 5, 6, 7,
8, 9 });
: AddInPredicate(, "b", { 0, 1, 2, 3, 4, 5, 6, 7,
8, 9 });
> So far, we have good test coverage for the cases where 'a' is pruned. Could
I just realize that range key can be differ from hash key. I adjusted some code
for it. Thx for point out.
http://gerrit.cloudera.org:8080/#/c/16903/1/src/kudu/common/scan_spec-test.cc@601
PS1, Line 601: b
> Should this be "b"?
Ah, yes, that explains why the previous patched result is so weird.
--
To view, visit http://gerrit.cloudera.org:8080/16903
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
Gerrit-Change-Number: 16903
Gerrit-PatchSet: 3
Gerrit-Owner: wangning <1994wangn...@gmail.com>
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: wangning <1994wangn...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 09:38:05 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1644 use range partition info for pruning
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16903 to look at the new patch set (#3). Change subject: KUDU-1644 use range partition info for pruning .. KUDU-1644 use range partition info for pruning When pruning in-list predicate values, range partition can also be taken into consideration. Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10 --- M src/kudu/common/partition.cc M src/kudu/common/partition.h M src/kudu/common/scan_spec-test.cc M src/kudu/common/scan_spec.cc M src/kudu/common/scan_spec.h M src/kudu/tools/kudu-admin-test.cc M src/kudu/tserver/tablet_service.cc 7 files changed, 661 insertions(+), 276 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/16903/3 -- To view, visit http://gerrit.cloudera.org:8080/16903 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10 Gerrit-Change-Number: 16903 Gerrit-PatchSet: 3 Gerrit-Owner: wangning <1994wangn...@gmail.com> Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1644 use range partition info for pruning
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16903 to look at the new patch set (#2). Change subject: KUDU-1644 use range partition info for pruning .. KUDU-1644 use range partition info for pruning When pruning in-list predicate values, range partition can also be taken into consideration. Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10 --- M src/kudu/common/partition.cc M src/kudu/common/partition.h M src/kudu/common/scan_spec-test.cc M src/kudu/common/scan_spec.cc M src/kudu/common/scan_spec.h M src/kudu/tools/kudu-admin-test.cc M src/kudu/tserver/tablet_service.cc 7 files changed, 661 insertions(+), 276 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/16903/2 -- To view, visit http://gerrit.cloudera.org:8080/16903 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10 Gerrit-Change-Number: 16903 Gerrit-PatchSet: 2 Gerrit-Owner: wangning <1994wangn...@gmail.com> Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2612 Java client transaction API
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 ) Change subject: KUDU-2612 Java client transaction API .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1181 PS2, Line 1181: This call is blocking > Would you mind explain a bit more about why it is blocking? You mean we should follow the suite of majority of methods in this interface and return something like Deferred and add 'KuduTransaction newTransaction()' into the KuduClient's interface? I didn't see much value in returning Deferred because I don't quite see the use cases for that. Let me know if you think otherwise and we should have this returning Deferred, keeping the deferred purity of this interface. -- To view, visit http://gerrit.cloudera.org:8080/16894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07 Gerrit-Change-Number: 16894 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 06 Jan 2021 09:03:42 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 Java client transaction API
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Hao Hao, Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16894 to look at the new patch set (#3). Change subject: KUDU-2612 Java client transaction API .. KUDU-2612 Java client transaction API This patch focused on the API than the actual functionality under the hood. The functionality to do the heavy-lifting (i.e. issuing RPC calls to TxnManager, retrying in case of transient errors, tests, etc.) will be posted as a separate patch as per our discussion with Andrew and Hao. The proposed API is mirroring the API for the C++ client with a few twists because of the presence of the async-style objects in the Kudu Java client API. Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java 5 files changed, 468 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/16894/3 -- To view, visit http://gerrit.cloudera.org:8080/16894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07 Gerrit-Change-Number: 16894 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong
