[kudu-CR] [tserver] introduce filter type for ListTablets
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16560 ) Change subject: [tserver] introduce filter_type for ListTablets .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/16560/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16560/1//COMMIT_MSG@16 PS1, Line 16: of Kudu ma > explicitly Done http://gerrit.cloudera.org:8080/#/c/16560/1/src/kudu/tserver/mini_tablet_server.cc File src/kudu/tserver/mini_tablet_server.cc: http://gerrit.cloudera.org:8080/#/c/16560/1/src/kudu/tserver/mini_tablet_server.cc@158 PS1, Line 158: vector MiniTabletServer::ListTablets( > warning: the parameter 'type_filter' is copied for each invocation but only Done http://gerrit.cloudera.org:8080/#/c/16560/1/src/kudu/tserver/mini_tablet_server.cc@158 PS1, Line 158: vector MiniTabletServer::ListTablets( > +1 Done http://gerrit.cloudera.org:8080/#/c/16560/1/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/16560/1/src/kudu/tserver/tablet_service.cc@2103 PS1, Line 2103: > What if request doesn't list DEFAULT_TABLE in its 'filter_type' field? Done -- To view, visit http://gerrit.cloudera.org:8080/16560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3f8d34a8c377fcdef0162eaf042498a6d41baa4a Gerrit-Change-Number: 16560 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 08 Oct 2020 05:40:32 + Gerrit-HasComments: Yes
[kudu-CR] [tserver] introduce filter type for ListTablets
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16560 )
Change subject: [tserver] introduce filter_type for ListTablets
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16560/1/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/16560/1/src/kudu/integration-tests/txn_status_table-itest.cc@208
PS1, Line 208: TEST_F(TxnStatusTableITest, TestDontListTablets) {
> I'm not sure I see the coverage for the following things:
Done
--
To view, visit http://gerrit.cloudera.org:8080/16560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f8d34a8c377fcdef0162eaf042498a6d41baa4a
Gerrit-Change-Number: 16560
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 08 Oct 2020 05:40:55 +
Gerrit-HasComments: Yes
[kudu-CR] [tserver] introduce filter type for ListTablets
Alexey Serbin has uploaded a new patch set (#2) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/16560 ) Change subject: [tserver] introduce filter_type for ListTablets .. [tserver] introduce filter_type for ListTablets This patch adds 'filter_type' field into ListTabletsRequestPB. If the field is not set, it's interpreted as if the field were set to [DEFAULT_TABLE]. That's to return only user tables by default even if the transaction status table is present. By introducing such a behavior, this patch addresses the failure of 50+ tests when a transaction status table is created by TxnManager upon first startup of Kudu master (see the upcoming patch at https://gerrit.cloudera.org/#/c/16527). Those tests depend on the fact that a cluster starts up with no tablets. The optionality of the table_type() interface in TabletMetadata felt needlessly clunky in implementing this, so I made the field non-optional instead, opting to explicitly use DEFAULT_TABLE where appropriate. I left the public constructors alone to avoid requiring common.pb.h in every test that wanted to create a DEFAULT_TABLE tablet. Change-Id: I3f8d34a8c377fcdef0162eaf042498a6d41baa4a --- M src/kudu/integration-tests/txn_status_table-itest.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tablet/tablet_replica.cc M src/kudu/tserver/mini_tablet_server.cc M src/kudu/tserver/mini_tablet_server.h M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/tserver.proto 10 files changed, 181 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/16560/2 -- To view, visit http://gerrit.cloudera.org:8080/16560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3f8d34a8c377fcdef0162eaf042498a6d41baa4a Gerrit-Change-Number: 16560 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2612 p5 (b): add highest seen txn id into CoordinatorOpResultPB
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16526 ) Change subject: KUDU-2612 p5 (b): add highest_seen_txn_id into CoordinatorOpResultPB .. Patch Set 6: Code-Review+2 Carrying over Hao Hao's +2 from PS5. -- To view, visit http://gerrit.cloudera.org:8080/16526 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifcb4d90bc10a5695c3f54229688ccdcaf56011d0 Gerrit-Change-Number: 16526 Gerrit-PatchSet: 6 Gerrit-Owner: Alexey Serbin 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: Thu, 08 Oct 2020 03:21:09 + Gerrit-HasComments: No
[kudu-CR] KUDU-2612 p5 (b): add highest seen txn id into CoordinatorOpResultPB
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16526 ) Change subject: KUDU-2612 p5 (b): add highest_seen_txn_id into CoordinatorOpResultPB .. KUDU-2612 p5 (b): add highest_seen_txn_id into CoordinatorOpResultPB This patch adds the highest transaction identifier seen by the TxnStatusManager (TxnStatusManager coordinates the lifecycle of transactional operations for a particular tablet of the transaction status table). The new field is populated when responding to CoordinateTransaction() RPC of the BEGIN_TXN type. The signature and the implementation of the related methods in the TxnStatusManager and TxnSystemClient classes have been updated as well, along with corresponding tests to cover the newly introduced functionality. The newly introduced field will be used in a follow-up patch containing the initial implementation of the TxnManager component. The rationale behind having this new field is to allow the TxnManager to 'calibrate' its last seen txn_id, so it could make less trial-and-error iterations when reserving an identifier for a transaction. The CoordinatorOpResultPB::highest_seen_txn_id would not be needed if TxnStatusManager (or their federation) was able to reserve an identifier for a new transaction on their own. Of course, that would require some sort of interaction among TxnStatusManager instances (like request forwarding or alike). But without such a provision, the TxnManager needs the newly introduced field to succeed with its trial-and-error approach while finding the next available identifier for a newly initiated transaction. This is a follow-up to 95f4109518e7b81b9115a4e8bacbd157dcecad0c. Change-Id: Ifcb4d90bc10a5695c3f54229688ccdcaf56011d0 Reviewed-on: http://gerrit.cloudera.org:8080/16526 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/integration-tests/txn_status_table-itest.cc M src/kudu/tablet/txn_coordinator.h M src/kudu/transactions/txn_status_manager-test.cc M src/kudu/transactions/txn_status_manager.cc M src/kudu/transactions/txn_status_manager.h M src/kudu/transactions/txn_system_client.cc M src/kudu/transactions/txn_system_client.h M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tserver_admin.proto 10 files changed, 177 insertions(+), 48 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16526 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ifcb4d90bc10a5695c3f54229688ccdcaf56011d0 Gerrit-Change-Number: 16526 Gerrit-PatchSet: 7 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2612 p5 (b): add highest seen txn id into CoordinatorOpResultPB
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16526 )
Change subject: KUDU-2612 p5 (b): add highest_seen_txn_id into
CoordinatorOpResultPB
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16526/3/src/kudu/transactions/txn_status_manager.cc
File src/kudu/transactions/txn_status_manager.cc:
http://gerrit.cloudera.org:8080/#/c/16526/3/src/kudu/transactions/txn_status_manager.cc@164
PS3, Line 164: // Always set the 'highest_seen_txn_id' on return.
: SCOPED_CLEANUP({
: if (highest_seen_txn_id) {
: std::lock_guard l(lock_);
: *highest_seen_txn_id = highest_txn_id_;
: }
: });
> Sounds good to me, would you mind adding more to the comment to explain the
@awong: the issues with extra lock acquisition and posting back -2 and -1 magic
number are addressed now
@hao.hao: I added a blurb to explain to motivation behind all this fuss
--
To view, visit http://gerrit.cloudera.org:8080/16526
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcb4d90bc10a5695c3f54229688ccdcaf56011d0
Gerrit-Change-Number: 16526
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin
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: Thu, 08 Oct 2020 00:16:22 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 p5 (b): add highest seen txn id into CoordinatorOpResultPB
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16526 to look at the new patch set (#6). Change subject: KUDU-2612 p5 (b): add highest_seen_txn_id into CoordinatorOpResultPB .. KUDU-2612 p5 (b): add highest_seen_txn_id into CoordinatorOpResultPB This patch adds the highest transaction identifier seen by the TxnStatusManager (TxnStatusManager coordinates the lifecycle of transactional operations for a particular tablet of the transaction status table). The new field is populated when responding to CoordinateTransaction() RPC of the BEGIN_TXN type. The signature and the implementation of the related methods in the TxnStatusManager and TxnSystemClient classes have been updated as well, along with corresponding tests to cover the newly introduced functionality. The newly introduced field will be used in a follow-up patch containing the initial implementation of the TxnManager component. The rationale behind having this new field is to allow the TxnManager to 'calibrate' its last seen txn_id, so it could make less trial-and-error iterations when reserving an identifier for a transaction. The CoordinatorOpResultPB::highest_seen_txn_id would not be needed if TxnStatusManager (or their federation) was able to reserve an identifier for a new transaction on their own. Of course, that would require some sort of interaction among TxnStatusManager instances (like request forwarding or alike). But without such a provision, the TxnManager needs the newly introduced field to succeed with its trial-and-error approach while finding the next available identifier for a newly initiated transaction. This is a follow-up to 95f4109518e7b81b9115a4e8bacbd157dcecad0c. Change-Id: Ifcb4d90bc10a5695c3f54229688ccdcaf56011d0 --- M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/integration-tests/txn_status_table-itest.cc M src/kudu/tablet/txn_coordinator.h M src/kudu/transactions/txn_status_manager-test.cc M src/kudu/transactions/txn_status_manager.cc M src/kudu/transactions/txn_status_manager.h M src/kudu/transactions/txn_system_client.cc M src/kudu/transactions/txn_system_client.h M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tserver_admin.proto 10 files changed, 177 insertions(+), 48 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/16526/6 -- To view, visit http://gerrit.cloudera.org:8080/16526 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifcb4d90bc10a5695c3f54229688ccdcaf56011d0 Gerrit-Change-Number: 16526 Gerrit-PatchSet: 6 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [master] introduce filter type for ListTables
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16561 ) Change subject: [master] introduce filter_type for ListTables .. Patch Set 2: > (1 comment) Thank you for putting together the patch! I'll take care of https://gerrit.cloudera.org/c/16560/ revving it further. -- To view, visit http://gerrit.cloudera.org:8080/16561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786 Gerrit-Change-Number: 16561 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 07 Oct 2020 23:36:10 + Gerrit-HasComments: No
[kudu-CR] [master] introduce filter type for ListTables
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16561 ) Change subject: [master] introduce filter_type for ListTables .. [master] introduce filter_type for ListTables This patch introduces a new 'filter_type' field for the ListTablesRequestPB message. With this patch, it's possible to explicitly filter tables to be returned from the system catalog. I added corresponding unit test as well. This patch doesn't update kudu CLI -- this is to be done in a separate patch. Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786 Reviewed-on: http://gerrit.cloudera.org:8080/16561 Reviewed-by: Andrew Wong Tested-by: Kudu Jenkins --- M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc M src/kudu/master/master.proto 3 files changed, 138 insertions(+), 10 deletions(-) Approvals: Andrew Wong: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/16561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786 Gerrit-Change-Number: 16561 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [master] introduce filter type for ListTables
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16561 ) Change subject: [master] introduce filter_type for ListTables .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/16561/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16561/1//COMMIT_MSG@7 PS1, Line 7: [master] introduce filter_type for ListTables > That's right -- that's a piece to be consistent with table type filtering. Thanks for the context. It actually clarified some things for me as well. I'd thought you were blocked on some ListTables-related behavior and I assumed you were working on this to unblock tests, so I took up what I assumed was the less important ListTablets behavior. If it's actually the ListTablets behavior you need to unbreak tests, feel free to take over the patch I have: https://gerrit.cloudera.org/c/16560/ -- To view, visit http://gerrit.cloudera.org:8080/16561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786 Gerrit-Change-Number: 16561 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 07 Oct 2020 23:05:43 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 p5 (b): add highest seen txn id into CoordinatorOpResultPB
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/16526 )
Change subject: KUDU-2612 p5 (b): add highest_seen_txn_id into
CoordinatorOpResultPB
..
Patch Set 5: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16526/3/src/kudu/transactions/txn_status_manager.cc
File src/kudu/transactions/txn_status_manager.cc:
http://gerrit.cloudera.org:8080/#/c/16526/3/src/kudu/transactions/txn_status_manager.cc@164
PS3, Line 164: // Always set the 'highest_seen_txn_id' on return.
: SCOPED_CLEANUP({
: if (highest_seen_txn_id) {
: std::lock_guard l(lock_);
: *highest_seen_txn_id = highest_txn_id_;
: }
: });
> OK sounds reasonable, particularly with concurrent activity. My main concer
Sounds good to me, would you mind adding more to the comment to explain the
motivation? Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/16526
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcb4d90bc10a5695c3f54229688ccdcaf56011d0
Gerrit-Change-Number: 16526
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin
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, 07 Oct 2020 23:02:03 +
Gerrit-HasComments: Yes
[kudu-CR] [master] introduce filter type for ListTables
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16561 ) Change subject: [master] introduce filter_type for ListTables .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/16561/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16561/1//COMMIT_MSG@7 PS1, Line 7: [master] introduce filter_type for ListTables > Just making sure I understand the context of this work: I was under the ass That's right -- that's a piece to be consistent with table type filtering. I haven't see breakages of current tests due to the absence of this piece in the scope of my TxnManager patch. Since we discussed this offline, I'd like to add some context here (it might be useful for other reviewers and for posterity at least). The story behind this change is that I was pondering about adding 'filter_type' for ListTablets since the current behavior of ListTablets leaves many tests (50+) broken once TxnManager creates txn status table upon initialization. However, adding that field only for ListTablets but leaving ListTables without corresponding functionality seemed not very consistent to me, given that I anticipated we were to add this flag anyways at least in sake of kudu CLI's functionality. We discussed this offline, and you were ready to help there with the corresponding piece for ListTablets, and I took care of ListTables in this patch. http://gerrit.cloudera.org:8080/#/c/16561/1/src/kudu/master/master-test.cc File src/kudu/master/master-test.cc: http://gerrit.cloudera.org:8080/#/c/16561/1/src/kudu/master/master-test.cc@696 PS1, Line 696: fitler > typo Done -- To view, visit http://gerrit.cloudera.org:8080/16561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786 Gerrit-Change-Number: 16561 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 07 Oct 2020 22:53:40 + Gerrit-HasComments: Yes
[kudu-CR] [master] introduce filter type for ListTables
Hello Kudu Jenkins, Andrew Wong, Bankim Bhavsar, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16561 to look at the new patch set (#2). Change subject: [master] introduce filter_type for ListTables .. [master] introduce filter_type for ListTables This patch introduces a new 'filter_type' field for the ListTablesRequestPB message. With this patch, it's possible to explicitly filter tables to be returned from the system catalog. I added corresponding unit test as well. This patch doesn't update kudu CLI -- this is to be done in a separate patch. Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786 --- M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc M src/kudu/master/master.proto 3 files changed, 138 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/16561/2 -- To view, visit http://gerrit.cloudera.org:8080/16561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786 Gerrit-Change-Number: 16561 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] tserver: only list tablets of user-defined tables by default
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16560 )
Change subject: tserver: only list tablets of user-defined tables by default
..
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/16560/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16560/1//COMMIT_MSG@16
PS1, Line 16: explicilty
explicitly
http://gerrit.cloudera.org:8080/#/c/16560/1/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/16560/1/src/kudu/integration-tests/txn_status_table-itest.cc@208
PS1, Line 208: TEST_F(TxnStatusTableITest, TestDontListTablets) {
I'm not sure I see the coverage for the following things:
* specifying TableTypePB::DEFAULT_TABLE for ListTabletsRequestPB::type_filter
gives the same result as if not specifying it at all for _both_ when user
tables are present and abset
* how the filter works in the presence and absence of user and system tables
Does it make sense to add coverage for that as well?
http://gerrit.cloudera.org:8080/#/c/16560/1/src/kudu/tserver/mini_tablet_server.cc
File src/kudu/tserver/mini_tablet_server.cc:
http://gerrit.cloudera.org:8080/#/c/16560/1/src/kudu/tserver/mini_tablet_server.cc@158
PS1, Line 158: vector MiniTabletServer::ListTablets(set
type_filter) const {
> warning: the parameter 'type_filter' is copied for each invocation but only
+1
Consider passing the 'type_filter' parameter by const reference?
http://gerrit.cloudera.org:8080/#/c/16560/1/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:
http://gerrit.cloudera.org:8080/#/c/16560/1/src/kudu/tserver/tablet_service.cc@2103
PS1, Line 2103: set types_to_return = { TableTypePB::DEFAULT_TABLE
};
What if request doesn't list DEFAULT_TABLE in its 'filter_type' field?
--
To view, visit http://gerrit.cloudera.org:8080/16560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f8d34a8c377fcdef0162eaf042498a6d41baa4a
Gerrit-Change-Number: 16560
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 07 Oct 2020 22:33:22 +
Gerrit-HasComments: Yes
[kudu-CR] [master] introduce filter type for ListTables
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16561 ) Change subject: [master] introduce filter_type for ListTables .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/16561/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16561/1//COMMIT_MSG@7 PS1, Line 7: [master] introduce filter_type for ListTables Just making sure I understand the context of this work: I was under the assumption this blocked some tests, but I don't think that's the case, given we already don't list system tables from the ListTables() endpoint. This is more about adding functionality to the CLI tool than unblocking tests -- is that right? -- To view, visit http://gerrit.cloudera.org:8080/16561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786 Gerrit-Change-Number: 16561 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 07 Oct 2020 22:27:30 + Gerrit-HasComments: Yes
[kudu-CR] [master] introduce filter type for ListTables
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16561 ) Change subject: [master] introduce filter_type for ListTables .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/16561/1/src/kudu/master/master-test.cc File src/kudu/master/master-test.cc: http://gerrit.cloudera.org:8080/#/c/16561/1/src/kudu/master/master-test.cc@696 PS1, Line 696: fitler typo -- To view, visit http://gerrit.cloudera.org:8080/16561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786 Gerrit-Change-Number: 16561 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 07 Oct 2020 22:20:05 + Gerrit-HasComments: Yes
[kudu-CR] [master] introduce filter type for ListTables
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16561 Change subject: [master] introduce filter_type for ListTables .. [master] introduce filter_type for ListTables This patch introduces a new 'filter_type' field for the ListTablesRequestPB message. With this patch, it's possible to explicitly filter tables to be returned from the system catalog. I added corresponding unit test as well. This patch doesn't update kudu CLI -- this is to be done in a separate patch. Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786 --- M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc M src/kudu/master/master.proto 3 files changed, 138 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/16561/1 -- To view, visit http://gerrit.cloudera.org:8080/16561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786 Gerrit-Change-Number: 16561 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] tserver: only list tablets of user-defined tables by default
Hello Alexey Serbin, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/16560 to review the following change. Change subject: tserver: only list tablets of user-defined tables by default .. tserver: only list tablets of user-defined tables by default Several tests depend on the fact that a cluster starts up with no tablets. This will become a problem if we start creating the transaction status table upon startup. So, in an effort to prevent such tests from failing, this patch avoids listing system tables by default. The optionality of the table_type() interface in TabletMetadata felt needlessly clunky in implementing this, so I made the field non-optional instead, opting to explicilty use DEFAULT_TABLE where appropriate. I left the public constructors alone to avoid requiring common.pb.h in every test that wanted to create a DEFAULT_TABLE tablet. Change-Id: I3f8d34a8c377fcdef0162eaf042498a6d41baa4a --- M src/kudu/integration-tests/txn_status_table-itest.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tablet/tablet_replica.cc M src/kudu/tserver/mini_tablet_server.cc M src/kudu/tserver/mini_tablet_server.h M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tserver.proto 9 files changed, 88 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/16560/1 -- To view, visit http://gerrit.cloudera.org:8080/16560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3f8d34a8c377fcdef0162eaf042498a6d41baa4a Gerrit-Change-Number: 16560 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin
[kudu-CR] KUDU-3187: Enhance the HMS plugin to check if synchronization is enabled
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16388 ) Change subject: KUDU-3187: Enhance the HMS plugin to check if synchronization is enabled .. KUDU-3187: Enhance the HMS plugin to check if synchronization is enabled This patch enhances the HMS plugin to check if synchronization is enabled on the Kudu cluster backing the table. This can simplify enabling HMS synchronization because the plugin jar can be added to the HMS classpath unconditionally. This also helps support cases where multiple Kudu clusters use the same HMS and one is synchronized and the other isn’t. This patch introduces a new environment property, `KUDU_HMS_SYNC_ENABLED`, to the plugin which allows for faster tests that do not need to setup a Kudu cluster. It could also be useful to disable this new functionality as a workaround if issues are seen. Additonally, this patch adjusts the mini HMS to add security properties configuration. This is required now that the Kudu client runs in the plugin and communicates with the cluster. See these commits for context: - https://github.com/apache/kudu/commit/0ee93e31a1febb987c72e7392a69b2584e6f38ed - https://github.com/apache/kudu/commit/3343144fefaad5a30e95e21297c64c78e308fa1f I also manually verified the following behavior on a real cluster: - `KUDU_HMS_SYNC_ENABLED` environment variable - CREATE/ALTER/DROP on non-sync cluster works - - With plugin jar used by the HMS - CREATE/ALTER/DROP on sync cluster fails with clear message - Authn via Kerberos Change-Id: Ib3588d72af1bb499202b47fca50a08876e13ea37 Reviewed-on: http://gerrit.cloudera.org:8080/16388 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong --- M java/kudu-hive/build.gradle M java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java M java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java M src/kudu/hms/hms_catalog-test.cc M src/kudu/hms/hms_client-test.cc M src/kudu/hms/mini_hms.cc M src/kudu/hms/mini_hms.h 7 files changed, 239 insertions(+), 43 deletions(-) Approvals: Kudu Jenkins: Verified Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ib3588d72af1bb499202b47fca50a08876e13ea37 Gerrit-Change-Number: 16388 Gerrit-PatchSet: 11 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Upgrade Bootstrap to 3.4.1 in web UI
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16556 ) Change subject: Upgrade Bootstrap to 3.4.1 in web UI .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16556/1/www/bootstrap/js/npm.js File www/bootstrap/js/npm.js: PS1: > I don't think this is necessary, it seems to work fine without it, no error I guess it's desirable to import as less as possible to keep it functional, unless it requires some non-trivial cherry-picking and patching. >From that standpoint I'd simply remove this file since it's not going to be >used by Kudu anyways. -- To view, visit http://gerrit.cloudera.org:8080/16556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b92241457af2997b3486781dc5e9e5f9e971471 Gerrit-Change-Number: 16556 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 07 Oct 2020 18:14:19 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3187: Enhance the HMS plugin to check if synchronization is enabled
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16388 ) Change subject: KUDU-3187: Enhance the HMS plugin to check if synchronization is enabled .. Patch Set 10: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib3588d72af1bb499202b47fca50a08876e13ea37 Gerrit-Change-Number: 16388 Gerrit-PatchSet: 10 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 07 Oct 2020 17:44:21 + Gerrit-HasComments: No
[kudu-CR] Bump version in examples to 1.13.0
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16486 ) Change subject: Bump version in examples to 1.13.0 .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/16486/1/examples/scala/sbt-int-test-example/src/it/resources/logback.xml File examples/scala/sbt-int-test-example/src/it/resources/logback.xml: http://gerrit.cloudera.org:8080/#/c/16486/1/examples/scala/sbt-int-test-example/src/it/resources/logback.xml@31 PS1, Line 31: > nit: extra change not needed. thanks, seems my sed script decided to add the missing newline -- To view, visit http://gerrit.cloudera.org:8080/16486 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I42ed941b5feaaa0d010b9fb1fbe82f77b66f6b23 Gerrit-Change-Number: 16486 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 07 Oct 2020 17:42:23 + Gerrit-HasComments: Yes
[kudu-CR] Bump version in examples to 1.13.0
Hello Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16486 to look at the new patch set (#2). Change subject: Bump version in examples to 1.13.0 .. Bump version in examples to 1.13.0 Change-Id: I42ed941b5feaaa0d010b9fb1fbe82f77b66f6b23 --- M examples/java/collectl/README.adoc M examples/java/collectl/pom.xml M examples/java/insert-loadgen/pom.xml M examples/java/java-example/pom.xml M examples/quickstart/nifi/README.adoc M examples/quickstart/spark/README.adoc M examples/quickstart/ycsb/README.adoc M examples/scala/sbt-int-test-example/README.adoc M examples/scala/sbt-int-test-example/build.sbt M examples/scala/spark-example/pom.xml 10 files changed, 15 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/16486/2 -- To view, visit http://gerrit.cloudera.org:8080/16486 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I42ed941b5feaaa0d010b9fb1fbe82f77b66f6b23 Gerrit-Change-Number: 16486 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Upgrade Bootstrap to 3.4.1 in web UI
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16556 ) Change subject: Upgrade Bootstrap to 3.4.1 in web UI .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16556/1/www/bootstrap/js/npm.js File www/bootstrap/js/npm.js: PS1: > Do we need this file imported as well? I don't think this is necessary, it seems to work fine without it, no errors logged to the console either. It was in the archive though, do you think I should remove it? -- To view, visit http://gerrit.cloudera.org:8080/16556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b92241457af2997b3486781dc5e9e5f9e971471 Gerrit-Change-Number: 16556 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 07 Oct 2020 17:38:11 + Gerrit-HasComments: Yes
[kudu-CR] Upgrade Bootstrap to 3.4.1 in web UI
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16556 ) Change subject: Upgrade Bootstrap to 3.4.1 in web UI .. Patch Set 1: Do we want/need to non `min` versions of the files? -- To view, visit http://gerrit.cloudera.org:8080/16556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b92241457af2997b3486781dc5e9e5f9e971471 Gerrit-Change-Number: 16556 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 07 Oct 2020 17:34:04 + Gerrit-HasComments: No
[kudu-CR] Upgrade Bootstrap to 3.4.1 in web UI
Grant Henke has removed a vote on this change. Change subject: Upgrade Bootstrap to 3.4.1 in web UI .. Removed Code-Review+2 by Grant Henke -- To view, visit http://gerrit.cloudera.org:8080/16556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I0b92241457af2997b3486781dc5e9e5f9e971471 Gerrit-Change-Number: 16556 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Upgrade Bootstrap to 3.4.1 in web UI
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16556 ) Change subject: Upgrade Bootstrap to 3.4.1 in web UI .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16556/1/www/bootstrap/js/npm.js File www/bootstrap/js/npm.js: PS1: Do we need this file imported as well? It lists many files as required, but they are missing in our clone of bootstrap. Or the plan is to add them later on? -- To view, visit http://gerrit.cloudera.org:8080/16556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b92241457af2997b3486781dc5e9e5f9e971471 Gerrit-Change-Number: 16556 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 07 Oct 2020 17:06:21 + Gerrit-HasComments: Yes
[kudu-CR] Bump version in examples to 1.13.0
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16486 ) Change subject: Bump version in examples to 1.13.0 .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/16486/1/examples/scala/sbt-int-test-example/src/it/resources/logback.xml File examples/scala/sbt-int-test-example/src/it/resources/logback.xml: http://gerrit.cloudera.org:8080/#/c/16486/1/examples/scala/sbt-int-test-example/src/it/resources/logback.xml@31 PS1, Line 31: nit: extra change not needed. -- To view, visit http://gerrit.cloudera.org:8080/16486 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I42ed941b5feaaa0d010b9fb1fbe82f77b66f6b23 Gerrit-Change-Number: 16486 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 07 Oct 2020 16:53:23 + Gerrit-HasComments: Yes
[kudu-CR] Upgrade Bootstrap to 3.4.1 in web UI
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16556 ) Change subject: Upgrade Bootstrap to 3.4.1 in web UI .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b92241457af2997b3486781dc5e9e5f9e971471 Gerrit-Change-Number: 16556 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 07 Oct 2020 16:52:24 + Gerrit-HasComments: No
[kudu-CR] Upgrade Bootstrap to 3.4.1 in web UI
Hello Grant Henke, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/16556 to review the following change. Change subject: Upgrade Bootstrap to 3.4.1 in web UI .. Upgrade Bootstrap to 3.4.1 in web UI The web UI has been using Bootstrap 3.3.7 which is affected by multiple XSS vulnerability CVEs: https://snyk.io/vuln/npm:bootstrap This patch upgrades Bootstrap to 3.4.1 which is the latest 3.x release of Bootstrap. Change-Id: I0b92241457af2997b3486781dc5e9e5f9e971471 --- A www/bootstrap/css/bootstrap-theme.css A www/bootstrap/css/bootstrap-theme.css.map A www/bootstrap/css/bootstrap-theme.min.css A www/bootstrap/css/bootstrap-theme.min.css.map A www/bootstrap/css/bootstrap.css A www/bootstrap/css/bootstrap.css.map M www/bootstrap/css/bootstrap.min.css A www/bootstrap/css/bootstrap.min.css.map A www/bootstrap/fonts/glyphicons-halflings-regular.eot A www/bootstrap/fonts/glyphicons-halflings-regular.svg A www/bootstrap/fonts/glyphicons-halflings-regular.ttf A www/bootstrap/fonts/glyphicons-halflings-regular.woff2 A www/bootstrap/js/bootstrap.js M www/bootstrap/js/bootstrap.min.js A www/bootstrap/js/npm.js 15 files changed, 10,318 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/16556/1 -- To view, visit http://gerrit.cloudera.org:8080/16556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I0b92241457af2997b3486781dc5e9e5f9e971471 Gerrit-Change-Number: 16556 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Grant Henke
[kudu-CR] KUDU-2612 p10: have timestamp assignment account for commit timestamps
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16470 ) Change subject: KUDU-2612 p10: have timestamp assignment account for commit timestamps .. KUDU-2612 p10: have timestamp assignment account for commit timestamps One of the hurdles to performing a transaction's commit on a participant is that the commit process must ensure repeatable reads. Without multi-op transactions, this is done via a dance between the TimeManager (the entity that tracks and assigns timestamps) and the MvccManager (the entity that tracks the lifecycle of ops). This patch extends the dance between the TimeManager and MvccManager to ensure that when a participant commits a transaction, all future ops will be assigned a higher timestamp. I found it time-consuming to peruse the existing codebase for how timestamp assignment ensured repeatable reads, so I added a block comment to time_manager.h describing how it does so. I also added a section about how this patch extends it to ensure repeatable reads in the context of transactions. Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72 Reviewed-on: http://gerrit.cloudera.org:8080/16470 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/time_manager-test.cc M src/kudu/consensus/time_manager.cc M src/kudu/consensus/time_manager.h M src/kudu/integration-tests/txn_participant-itest.cc M src/kudu/tablet/ops/participant_op.cc M src/kudu/tablet/ops/participant_op.h M src/kudu/tablet/ops/write_op.cc M src/kudu/tablet/ops/write_op.h M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/txn_participant.cc M src/kudu/tablet/txn_participant.h 14 files changed, 487 insertions(+), 83 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72 Gerrit-Change-Number: 16470 Gerrit-PatchSet: 12 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2612 p11: persist txn metadata in superblock
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16492 ) Change subject: KUDU-2612 p11: persist txn metadata in superblock .. KUDU-2612 p11: persist txn metadata in superblock Currently, transaction metadata is entirely stored in the WALs; once a transaction is begun, it anchors WALs indefinitely. This is untenable, as any tablet that participates in a transaction would never be able to GC its WALs. This patch addresses this by storing transaction metadata in the tablet superblock for the following participant ops: - BEGIN_TXN: an empty record is added to the superblock for the given transaction ID. In the future, this can be used to store the owner of the transaction. - FINALIZE_COMMIT: the commit timestamp is added to the superblock for the given transaction ID. The commit timestamp of a transaction will be used when scanning over mutations applied to a tablet instead of the apply timestamp. - ABORT_TXN: an abort bit is added to the superblock for the given transaction ID. Upon applying each of these ops, the op's OpId is anchored in the WALs until the tablet metadata is successfully flushed, ensuring that if we don't flush the tablet metadata before restarting, we can rebuild any ops that whose mutations had not been persisted by replaying the WALs. What about BEGIN_COMMIT ops? While these ops will still need to be anchored, BEGIN_COMMIT ops don't need to persist any long-term information. As such, their anchoring is slightly different -- rather than mutating the tablet metadata and unanchoring on a metadata flush, BEGIN_COMMIT ops will update the in-memory state for the in-flight Txn, and unanchor once the corresponding FINALIZE_COMMIT or ABORT_TXN begins applying. On bootstrap, the following rules are applied: - If we've persisted terminal information (i.e. abort or commit) about a transaction ID, we can skip replaying any participant op for that transaction. - If we otherwise have a persisted record for the transaction ID, we should start an open transaction and replay all participant ops for that transaction. - If we have no record of a transaction persisted, we must replay all participant ops for the given transaction. While storing things in the superblock is not ideal, there are further improvements that can be made to reduce its size, e.g. after finalizing a transaction, the transactions mutations may be merged in with the rest of the tablet; once all mutations of a given transaction have been merged, the transaction metadata may be removed from the metadata. Change-Id: I2f32808aef10484f4e0ad3942bb005f61fbdb34a Reviewed-on: http://gerrit.cloudera.org:8080/16492 Tested-by: Andrew Wong Reviewed-by: Alexey Serbin --- M src/kudu/integration-tests/txn_participant-itest.cc M src/kudu/tablet/metadata.proto M src/kudu/tablet/ops/participant_op.cc M src/kudu/tablet/ops/participant_op.h M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/tablet_metadata-test.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tablet/txn_participant-test.cc M src/kudu/tablet/txn_participant.cc M src/kudu/tablet/txn_participant.h 13 files changed, 739 insertions(+), 133 deletions(-) Approvals: Andrew Wong: Verified Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I2f32808aef10484f4e0ad3942bb005f61fbdb34a Gerrit-Change-Number: 16492 Gerrit-PatchSet: 12 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
