[kudu-CR] [Client] Add query id to trace the whole query process
Wang Xixu has posted comments on this change. (
http://gerrit.cloudera.org:8080/18846 )
Change subject: [Client] Add query id to trace the whole query process
..
Patch Set 28:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18846/27/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:
http://gerrit.cloudera.org:8080/#/c/18846/27/src/kudu/tserver/tablet_service.cc@3039
PS27, Line 3039: LOG(INFO) << Substitute("Query id: $0, scanner id: $1.",
: req->query_id(), req->scanner_id());
> Right, so isn't it enough to have this query id output by in the trace logs
OK. Do you have any other questions about this patch: post query id from Impala
to Kudu.
--
To view, visit http://gerrit.cloudera.org:8080/18846
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9dbae801596726fec1c85ee547128da3179345d9
Gerrit-Change-Number: 18846
Gerrit-PatchSet: 28
Gerrit-Owner: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Jian Zhang
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai
Gerrit-Reviewer: Yuqi Du
Gerrit-Comment-Date: Wed, 18 Jan 2023 06:13:13 +
Gerrit-HasComments: Yes
[kudu-CR] [Client] Add query id to trace the whole query process
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/18846 ) Change subject: [Client] Add query id to trace the whole query process .. Patch Set 28: > Patch Set 28: > > (1 comment) ok -- To view, visit http://gerrit.cloudera.org:8080/18846 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9dbae801596726fec1c85ee547128da3179345d9 Gerrit-Change-Number: 18846 Gerrit-PatchSet: 28 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Jian Zhang Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du Gerrit-Comment-Date: Wed, 18 Jan 2023 06:10:48 + Gerrit-HasComments: No
[kudu-CR] [tools] Add test to generate heavy rowset compaction
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/19278 ) Change subject: [tools] Add test to generate heavy rowset compaction .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/19278/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19278/5//COMMIT_MSG@13 PS5, Line 13: The test may require a follow-up change to accomodate recently : added change for 'memory budgeting of CompactRowSetsOp' > If so, then I'm not sure that I understand the purpose of this particular p The reason behind having this patch is to document steps to reproduce high memory usage by compaction during heavy rowset updates. As this is long running test (~15 minutes) and ends up consuming heavy memory, the test may behave differently on different test nodes (depending on existing load running on those systems), so it makes sense to have it disabled by default. It can be enabled and used as and when required to reproduce such scenarios. Even if new flags used in memory budgeting patch is included in this test, I think we may still need to keep it disabled because step 1 stated above will anyway take time and consume memory. -- To view, visit http://gerrit.cloudera.org:8080/19278 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ief4ec03f1bf0f7936f8fb054948f87e71333f824 Gerrit-Change-Number: 19278 Gerrit-PatchSet: 5 Gerrit-Owner: Ashwani Raina Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 18 Jan 2023 05:58:13 + Gerrit-HasComments: Yes
[kudu-CR] [tools] Add test to generate heavy rowset compaction
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19278 ) Change subject: [tools] Add test to generate heavy rowset compaction .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/19278/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19278/5//COMMIT_MSG@13 PS5, Line 13: The test may require a follow-up change to accomodate recently : added change for 'memory budgeting of CompactRowSetsOp' > With new flags introduced in memory budgeting patch (https://github.com/apa If so, then I'm not sure that I understand the purpose of this particular patch if the newly introduced test is disabled anyways. Why to commit this patch as-is then? -- To view, visit http://gerrit.cloudera.org:8080/19278 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ief4ec03f1bf0f7936f8fb054948f87e71333f824 Gerrit-Change-Number: 19278 Gerrit-PatchSet: 5 Gerrit-Owner: Ashwani Raina Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 18 Jan 2023 05:39:58 + Gerrit-HasComments: Yes
[kudu-CR] [Client] Add query id to trace the whole query process
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/18846 )
Change subject: [Client] Add query id to trace the whole query process
..
Patch Set 28:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18846/27/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:
http://gerrit.cloudera.org:8080/#/c/18846/27/src/kudu/tserver/tablet_service.cc@3039
PS27, Line 3039: LOG(INFO) << Substitute("Query id: $0, scanner id: $1.",
: req->query_id(), req->scanner_id());
> Here binding query id with scanner id for troubleshooting and debuging, alt
Right, so isn't it enough to have this query id output by in the trace logs
then?
--
To view, visit http://gerrit.cloudera.org:8080/18846
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9dbae801596726fec1c85ee547128da3179345d9
Gerrit-Change-Number: 18846
Gerrit-PatchSet: 28
Gerrit-Owner: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Jian Zhang
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai
Gerrit-Reviewer: Yuqi Du
Gerrit-Comment-Date: Wed, 18 Jan 2023 05:35:58 +
Gerrit-HasComments: Yes
[kudu-CR] [web] add maintenance op statistics information at web pages for data retained bytes
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19407 ) Change subject: [web] add maintenance op statistics information at web pages for data_retained_bytes .. Patch Set 4: Verified+1 (6 comments) +1 for adding a link to a screenshot of the maintenance manager page, as suggested by Yifan http://gerrit.cloudera.org:8080/#/c/19407/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19407/4//COMMIT_MSG@11 PS4, Line 11: no the information no such information http://gerrit.cloudera.org:8080/#/c/19407/4//COMMIT_MSG@11 PS4, Line 11: showed shown http://gerrit.cloudera.org:8080/#/c/19407/4//COMMIT_MSG@13 PS4, Line 13: protobuf file the protobuf file http://gerrit.cloudera.org:8080/#/c/19407/4//COMMIT_MSG@13 PS4, Line 13: show it at web pages ... updates the embedded web server to show the retained bytes for a maintenance operation http://gerrit.cloudera.org:8080/#/c/19407/4//COMMIT_MSG@13 PS4, Line 13: variable field http://gerrit.cloudera.org:8080/#/c/19407/4/src/kudu/integration-tests/webserver-stress-itest.cc File src/kudu/integration-tests/webserver-stress-itest.cc: http://gerrit.cloudera.org:8080/#/c/19407/4/src/kudu/integration-tests/webserver-stress-itest.cc@101 PS4, Line 101: #ifdef __linux__ Would be great to add a comment to explain why having this extra check under this macro. -- To view, visit http://gerrit.cloudera.org:8080/19407 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iac8f0307705d57cea48901102b170c88d73d8c2e Gerrit-Change-Number: 19407 Gerrit-PatchSet: 4 Gerrit-Owner: Yuqi Du Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du Gerrit-Comment-Date: Wed, 18 Jan 2023 05:16:18 + Gerrit-HasComments: Yes
[kudu-CR] [examples] Make java-example up-to-date
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19418 ) Change subject: [examples] Make java-example up-to-date .. Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/19418/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19418/2//COMMIT_MSG@11 PS2, Line 11: Osx nit: since some 10.x version they started officially calling it macOS $ sw_vers ProductName:macOS ProductVersion: 11.7 BuildVersion: 20G817 http://gerrit.cloudera.org:8080/#/c/19418/2/examples/java/java-example/README.adoc File examples/java/java-example/README.adoc: http://gerrit.cloudera.org:8080/#/c/19418/2/examples/java/java-example/README.adoc@39 PS2, Line 39: This means that the exact version and architecture jar file can not : be found in the maven repo It means the maven repository does not contain a JAR file with the exact architecture and version. http://gerrit.cloudera.org:8080/#/c/19418/2/examples/java/java-example/README.adoc@40 PS2, Line 40: be worked around by I guess here the essence of the workaround should be outlined, i.e. building the required JAR locally and running with local the local repository? The required practical steps could be presented right below in a form of bulleted/numbered list or something like that, but having them in a sentence, separated by commas is also OK. http://gerrit.cloudera.org:8080/#/c/19418/2/examples/java/java-example/README.adoc@41 PS2, Line 41: dir directory http://gerrit.cloudera.org:8080/#/c/19418/2/examples/java/java-example/README.adoc@41 PS2, Line 41: set setting http://gerrit.cloudera.org:8080/#/c/19418/2/examples/java/java-example/pom.xml File examples/java/java-example/pom.xml: http://gerrit.cloudera.org:8080/#/c/19418/2/examples/java/java-example/pom.xml@31 PS2, Line 31: 1.16.0 nit: you could separate this into its own changelist since the up-to-date version in the examples and arch-related workaround for Apple M1 chips are orthogonal. -- To view, visit http://gerrit.cloudera.org:8080/19418 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf0eaa4799287adb2b8c90f62e9d90f7f7105de1 Gerrit-Change-Number: 19418 Gerrit-PatchSet: 2 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Ádám Bakai Gerrit-Comment-Date: Wed, 18 Jan 2023 04:56:05 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 ) Change subject: KUDU-1945: Support non unique primary key for Java client .. Patch Set 18: (16 comments) http://gerrit.cloudera.org:8080/#/c/19384/18//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19384/18//COMMIT_MSG@11 PS18, Line 11: auto_increment_id auto_incrementing_id http://gerrit.cloudera.org:8080/#/c/19384/18//COMMIT_MSG@27 PS18, Line 27: end-end nit: end-to-end ? http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java File java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java: http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@159 PS18, Line 159: Type type = Type.getTypeForDataType(pb.getType()); : ColumnTypeAttributes typeAttributes = pb.hasTypeAttributes() ? : pbToColumnTypeAttributes(pb.getTypeAttributes()) : null; : Object defaultValue = pb.hasWriteDefaultValue() ? : byteStringToObject(type, typeAttributes, pb.getWriteDefaultValue()) : null; nit: could move these lines down to be after line 175? http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@167 PS18, Line 167: int desiredBlockSize = pb.getCfileBlockSize(); nit: this could be moved down to be after line 175 as well? http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@170 PS18, Line 170: default encoding Why default? It seems they are coming from the 'pb' parameter. http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@171 PS18, Line 171: return new ColumnSchema.AutoIncrementingColumnSchemaBuilder() : .encoding(encoding) : .compressionAlgorithm(compressionAlgorithm) : .build(); After looking at this one more time I realized I'm confused :) It seems in this case we are discarding whatever is set in the other fields of the 'pb' parameter. Why is so? I'd think this helper method takes all the fields in the 'pb' as the input, treating those as the source of truth, but only one extra artificial field is set: nonUniqueKey(). It seems now the semantics of this helper method has changed. It would be great to clarify why we need to change it up to such extent. Thanks! http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@179 PS18, Line 179: !isKeyUnique nit: remove the negation and swap lines 180 & 182? http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java: http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@792 PS18, Line 792: schema nit: a schema http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@798 PS18, Line 798: assertEquals(1, schema.getPrimaryKeyColumnCount()); This now looks a bit different from we have in the C++ client: https://gerrit.cloudera.org/#/c/19272/19/src/kudu/client/client-unittest.cc@233 Should we reconcile? http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@799 PS18, Line 799: table nit: a table http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@810 PS18, Line 810: assertEquals(3, schema.getColumnCount()); : assertEquals(2, schema.getPrimaryKeyColumnCount()); Isn't this some sort of a contradiction to the POLA principle (https://en.wikipedia.org/wiki/Principle_of_least_astonishment)? At lines 797 & 798 the schema shows different numbers. I'd expect those to be the same. http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java: http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2570 PS18, Line 2570: schema nit for here and elsewhere in the code below: a schema http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2579 PS18, Line 2579: assertFalse(schema.isPrimaryKeyUnique()); Could you also add an assert to verify the number of columns in the resulting table's schema?
[kudu-CR] KUDU-1945 Auto-Incrementing Column, C++ client
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/19272 ) Change subject: KUDU-1945 Auto-Incrementing Column, C++ client .. Patch Set 19: (2 comments) http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.h File src/kudu/client/schema.h: http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.h@798 PS19, Line 798: Utility function to return the actual name of the auto incrementing column > Yep, that makes sense. Do we want to restrict DDL operations with the auto Yes, there will be a followup patch on the server side to restrict alter table operations involving auto-incrementing column. For the C++ client side changes, I think Marton was planning to include those changes in the followup changelist along with the server side bits. http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.cc File src/kudu/client/schema.cc: http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.cc@589 PS19, Line 589: std::string Name; : KuduColumnSchema::DataType Type; > style nit: the name of these fields should start with non-capital letters +1 -- To view, visit http://gerrit.cloudera.org:8080/19272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic133e3d44cc56c8351e33d95b523ed7b6b13617b Gerrit-Change-Number: 19272 Gerrit-PatchSet: 19 Gerrit-Owner: Marton Greber Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Wed, 18 Jan 2023 04:30:30 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945 Auto-Incrementing Column, C++ client
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19272 ) Change subject: KUDU-1945 Auto-Incrementing Column, C++ client .. Patch Set 19: (1 comment) http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/client-unittest.cc File src/kudu/client/client-unittest.cc: http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/client-unittest.cc@267 PS19, Line 267: > Could you also add a scenario where a schema with a INT64 column named "aut I think Column name 'auto_incrementing_id' should be reserved column name for Kudu engine. To avoid confusion, we should not allow user to add a column named as 'auto_incrementing_id' when creating table or alter table. Java client throw exception if user try to add a column named as 'auto_incrementing_id'. -- To view, visit http://gerrit.cloudera.org:8080/19272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic133e3d44cc56c8351e33d95b523ed7b6b13617b Gerrit-Change-Number: 19272 Gerrit-PatchSet: 19 Gerrit-Owner: Marton Greber Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Wed, 18 Jan 2023 04:21:21 + Gerrit-HasComments: Yes
[kudu-CR] [row operations] minor updates
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19424 ) Change subject: [row_operations] minor updates .. [row_operations] minor updates This patch updates the code in row_operations.cc * simplified handling of RowOperationsPB::Type in DecodeOp * use std::make_shared where appropriate * use move semantics in a few places * unsorted style-related changes Change-Id: Ib1d7c6c28078447638a246a08f137b374cc9dac7 Reviewed-on: http://gerrit.cloudera.org:8080/19424 Tested-by: Alexey Serbin Reviewed-by: Yifan Zhang --- M src/kudu/common/row_operations.cc M src/kudu/tablet/tablet_auto_incrementing-test.cc M src/kudu/tserver/tablet_server-test.cc 3 files changed, 77 insertions(+), 73 deletions(-) Approvals: Alexey Serbin: Verified Yifan Zhang: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/19424 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ib1d7c6c28078447638a246a08f137b374cc9dac7 Gerrit-Change-Number: 19424 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai
[kudu-CR] [row operations] minor updates
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19424 ) Change subject: [row_operations] minor updates .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/19424 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib1d7c6c28078447638a246a08f137b374cc9dac7 Gerrit-Change-Number: 19424 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Comment-Date: Wed, 18 Jan 2023 02:33:31 + Gerrit-HasComments: No
[kudu-CR] KUDU-1945 Auto-Incrementing Column, C++ client
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19272 ) Change subject: KUDU-1945 Auto-Incrementing Column, C++ client .. Patch Set 19: (2 comments) http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/client-unittest.cc File src/kudu/client/client-unittest.cc: http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/client-unittest.cc@267 PS19, Line 267: Could you also add a scenario where a schema with a INT64 column named "auto_incrementing_id" is created and added into the list of non-unique primary key columns? What's the expected behavior of the client library in that case? http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.h File src/kudu/client/schema.h: http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.h@798 PS19, Line 798: Utility function to return the actual name of the auto incrementing column > Java client don't allow to use Alter Table API to rename or drop the auto-i Yep, that makes sense. Do we want to restrict DDL operations with the auto-incrementing column at the server side as well? Or we rather want to have this only as the client-side restriction since there isn't any inconsistencies from the server-side point of view if renaming and the auto-incrementing column? I guess we should at least restrict dropping the auto-incrementing column at the server side since the column is a part of the primary key, and Kudu doesn't properly handle update of the primary key columns as well as columns participating in partition schema. -- To view, visit http://gerrit.cloudera.org:8080/19272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic133e3d44cc56c8351e33d95b523ed7b6b13617b Gerrit-Change-Number: 19272 Gerrit-PatchSet: 19 Gerrit-Owner: Marton Greber Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Wed, 18 Jan 2023 02:16:17 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945 Auto-Incrementing Column, C++ client
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19272 ) Change subject: KUDU-1945 Auto-Incrementing Column, C++ client .. Patch Set 19: (1 comment) http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.h File src/kudu/client/schema.h: http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.h@798 PS19, Line 798: Utility function to return the actual name of the auto incrementing column > I guess this is only for the time when a table with implicitly added column Java client don't allow to use Alter Table API to rename or drop the auto-incrementing column. c++ client should also add such restrictions. -- To view, visit http://gerrit.cloudera.org:8080/19272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic133e3d44cc56c8351e33d95b523ed7b6b13617b Gerrit-Change-Number: 19272 Gerrit-PatchSet: 19 Gerrit-Owner: Marton Greber Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Wed, 18 Jan 2023 01:46:00 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945 Auto-Incrementing Column, C++ client
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19272 ) Change subject: KUDU-1945 Auto-Incrementing Column, C++ client .. Patch Set 19: Code-Review+1 (10 comments) Just a few nits, overall looks good to me. http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/scan_token-test.cc File src/kudu/client/scan_token-test.cc: http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/scan_token-test.cc@338 PS19, Line 338: const KuduScanToken* style nit: consider using const reference here instead -- in most cases in Kudu codebase, constant parameters are passed by reference, not by pointer http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/scan_token-test.cc@341 PS19, Line 341: shared_ptr client; : CHECK_OK(cluster_->CreateClient(nullptr, )); Why not to use already existing 'client_' member field here? Would be great to add a short blurb to explain the reasoning behind. http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.h File src/kudu/client/schema.h: http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.h@798 PS19, Line 798: Utility function to return the actual name of the auto incrementing column I guess this is only for the time when a table with implicitly added column is created? In other words, that's the default name for the auto-incrementing column, IIUC. We don't have any restriction in place to prohibit changing the name of the auto-incrementing column later on using AlterTable, right? http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.h@867 PS19, Line 867: auto_incrementing_col_name style nit: this should either follow the naming convention for regular field (like 'schema_' below) and end with underscore or the convention of naming static constant/constexpr static fields, where it would be something like kAutoIncColName or something similar. http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.cc File src/kudu/client/schema.cc: http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.cc@575 PS19, Line 575: vector& style nit: in most of the Kudu code, the "out" and "inout" parameters are usually passed as a pointer http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.cc@589 PS19, Line 589: std::string Name; : KuduColumnSchema::DataType Type; style nit: the name of these fields should start with non-capital letters Shouldn't they be static constexpr ones? Also, make this private section explicit (i.e. add the 'private:' tag) and move it to be after all the methods and fields in the 'public:' section. http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.cc@594 PS19, Line 594: style nit: add two extra spaces before the column http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.cc@609 PS19, Line 609: bool key_cols_unique; Should this field be initialized in the constructor? http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/client/schema.cc@682 PS19, Line 682: nit: remove this extra line? http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/common/schema.cc File src/kudu/common/schema.cc: http://gerrit.cloudera.org:8080/#/c/19272/19/src/kudu/common/schema.cc@a293 PS19, Line 293: : : : : : : : : : : : Does it make sense to keep the logic to enforce the invariants for auto-incrementing column in the form of corresponding DCHECK() macros to spot programming mistakes? -- To view, visit http://gerrit.cloudera.org:8080/19272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic133e3d44cc56c8351e33d95b523ed7b6b13617b Gerrit-Change-Number: 19272 Gerrit-PatchSet: 19 Gerrit-Owner: Marton Greber Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Wed, 18 Jan 2023 00:55:00 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 ) Change subject: KUDU-1945: Support non unique primary key for Java client .. Patch Set 18: Code-Review+1 Thanks for adding the alter table side of work as well. -- To view, visit http://gerrit.cloudera.org:8080/19384 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c Gerrit-Change-Number: 19384 Gerrit-PatchSet: 18 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 17 Jan 2023 19:03:15 + Gerrit-HasComments: No
[kudu-CR] plumb JWT authentication into clients
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/18471 ) Change subject: plumb JWT authentication into clients .. Patch Set 18: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/18471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibdfc2640c047a2e3bb5ea03aea4439cea2532e23 Gerrit-Change-Number: 18471 Gerrit-PatchSet: 18 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Tue, 17 Jan 2023 18:48:15 + Gerrit-HasComments: No
[kudu-CR] plumb JWT authentication into clients
Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/18471 ) Change subject: plumb JWT authentication into clients .. Patch Set 18: (1 comment) http://gerrit.cloudera.org:8080/#/c/18471/17//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18471/17//COMMIT_MSG@19 PS17, Line 19: > nit: line is too long to exceed the limit Done -- To view, visit http://gerrit.cloudera.org:8080/18471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibdfc2640c047a2e3bb5ea03aea4439cea2532e23 Gerrit-Change-Number: 18471 Gerrit-PatchSet: 18 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Tue, 17 Jan 2023 18:47:33 + Gerrit-HasComments: Yes
[kudu-CR] plumb JWT authentication into clients
Zoltan Chovan has uploaded a new patch set (#18) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18471 ) Change subject: plumb JWT authentication into clients .. plumb JWT authentication into clients This change plumbs the JWT authentication into the client and into the client negotiation (the JWTVerifier is set when building the Messenger). There following new flags are added: * enable_jwt_token_auth * jwt_validate_signature (unsafe) * jwt_allow_without_tls (unsafe) * jwks_file_path * jwks_url If 'enable_jwt_token_auth' is set to true, then either 'jwks_file_path' or 'jwks_url' has to be set, also both cannot be set at the same time. Co-authored-by: Andrew Wong Change-Id: Ibdfc2640c047a2e3bb5ea03aea4439cea2532e23 --- M src/kudu/client/client.cc M src/kudu/rpc/client_negotiation.cc M src/kudu/security/tls_handshake.cc M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc A src/kudu/util/jwt_test_certs.h 6 files changed, 460 insertions(+), 365 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/18471/18 -- To view, visit http://gerrit.cloudera.org:8080/18471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibdfc2640c047a2e3bb5ea03aea4439cea2532e23 Gerrit-Change-Number: 18471 Gerrit-PatchSet: 18 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] plumb JWT authentication into clients
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/18471 ) Change subject: plumb JWT authentication into clients .. Patch Set 17: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/18471/17//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18471/17//COMMIT_MSG@19 PS17, Line 19: or nit: line is too long to exceed the limit -- To view, visit http://gerrit.cloudera.org:8080/18471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibdfc2640c047a2e3bb5ea03aea4439cea2532e23 Gerrit-Change-Number: 18471 Gerrit-PatchSet: 17 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Tue, 17 Jan 2023 18:35:30 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945 Fix TSAN warnings in client-test.cc
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19427 ) Change subject: KUDU-1945 Fix TSAN warnings in client-test.cc .. KUDU-1945 Fix TSAN warnings in client-test.cc This patch addresses the TSAN warnings seen due to race conditions in the test ClientTestAutoIncrementingColumn.ConcurrentWrites in client-test.cc. Without any modification 5/1024 failed: http://dist-test.cloudera.org/job?job_id=root.1673953260.499709 With the proposed changes: 0/1024 failed: http://dist-test.cloudera.org/job?job_id=root.1673952868.476619 Thanks to Marton for helping out with the dist-test. Change-Id: I24315c24ed7424e641837cf2d681262584f3e4d5 Reviewed-on: http://gerrit.cloudera.org:8080/19427 Reviewed-by: Marton Greber Reviewed-by: Attila Bukor Tested-by: Attila Bukor --- M src/kudu/client/client-test.cc 1 file changed, 0 insertions(+), 2 deletions(-) Approvals: Marton Greber: Looks good to me, but someone else must approve Attila Bukor: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/19427 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I24315c24ed7424e641837cf2d681262584f3e4d5 Gerrit-Change-Number: 19427 Gerrit-PatchSet: 2 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber
[kudu-CR] KUDU-1945 Fix TSAN warnings in client-test.cc
Attila Bukor has removed a vote on this change. Change subject: KUDU-1945 Fix TSAN warnings in client-test.cc .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/19427 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I24315c24ed7424e641837cf2d681262584f3e4d5 Gerrit-Change-Number: 19427 Gerrit-PatchSet: 1 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber
[kudu-CR] KUDU-1945 Fix TSAN warnings in client-test.cc
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/19427 ) Change subject: KUDU-1945 Fix TSAN warnings in client-test.cc .. Patch Set 1: Verified+1 Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/19427 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I24315c24ed7424e641837cf2d681262584f3e4d5 Gerrit-Change-Number: 19427 Gerrit-PatchSet: 1 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Comment-Date: Tue, 17 Jan 2023 17:46:42 + Gerrit-HasComments: No
[kudu-CR] [examples] Make java-example up-to-date
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/19418 ) Change subject: [examples] Make java-example up-to-date .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/19418/2/examples/java/java-example/README.adoc File examples/java/java-example/README.adoc: http://gerrit.cloudera.org:8080/#/c/19418/2/examples/java/java-example/README.adoc@39 PS2, Line 39: error occurs. This means that the exact version and architecture jar file can not nit: maybe specifically call out aarch64 in parentheses? -- To view, visit http://gerrit.cloudera.org:8080/19418 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf0eaa4799287adb2b8c90f62e9d90f7f7105de1 Gerrit-Change-Number: 19418 Gerrit-PatchSet: 2 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Ádám Bakai Gerrit-Comment-Date: Tue, 17 Jan 2023 17:40:14 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945 Fix TSAN warnings in client-test.cc
Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/19427 ) Change subject: KUDU-1945 Fix TSAN warnings in client-test.cc .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/19427 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I24315c24ed7424e641837cf2d681262584f3e4d5 Gerrit-Change-Number: 19427 Gerrit-PatchSet: 1 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Comment-Date: Tue, 17 Jan 2023 17:32:39 + Gerrit-HasComments: No
[kudu-CR] KUDU-1945 Fix TSAN warnings in client-test.cc
Abhishek Chennaka has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19427 Change subject: KUDU-1945 Fix TSAN warnings in client-test.cc .. KUDU-1945 Fix TSAN warnings in client-test.cc This patch addresses the TSAN warnings seen due to race conditions in the test ClientTestAutoIncrementingColumn.ConcurrentWrites in client-test.cc. Without any modification 5/1024 failed: http://dist-test.cloudera.org/job?job_id=root.1673953260.499709 With the proposed changes: 0/1024 failed: http://dist-test.cloudera.org/job?job_id=root.1673952868.476619 Thanks to Marton for helping out with the dist-test. Change-Id: I24315c24ed7424e641837cf2d681262584f3e4d5 --- M src/kudu/client/client-test.cc 1 file changed, 0 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/19427/1 -- To view, visit http://gerrit.cloudera.org:8080/19427 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I24315c24ed7424e641837cf2d681262584f3e4d5 Gerrit-Change-Number: 19427 Gerrit-PatchSet: 1 Gerrit-Owner: Abhishek Chennaka
[kudu-CR] [row operations] minor updates
Alexey Serbin has removed a vote on this change. Change subject: [row_operations] minor updates .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/19424 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ib1d7c6c28078447638a246a08f137b374cc9dac7 Gerrit-Change-Number: 19424 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai
[kudu-CR] [row operations] minor updates
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19424 ) Change subject: [row_operations] minor updates .. Patch Set 1: Verified+1 unrelated flake in the newly introduced ClientTestAutoIncrementingColumn.ConcurrentWrites (TSAN warnings) -- To view, visit http://gerrit.cloudera.org:8080/19424 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib1d7c6c28078447638a246a08f137b374cc9dac7 Gerrit-Change-Number: 19424 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Comment-Date: Tue, 17 Jan 2023 16:15:22 + Gerrit-HasComments: No
[kudu-CR] KUDU-3406 corrected estimate for ancient UNDO delta size
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19413 ) Change subject: KUDU-3406 corrected estimate for ancient UNDO delta size .. KUDU-3406 corrected estimate for ancient UNDO delta size When looking into micro-benchmark results produced by the $KUDU_HOME/src/kudu/scripts/benchmarks.sh script, I noticed that dense_node-itest showed 9-fold increase in the number of blocks under management. Even if the test disables GC of the ancient UNDO deltas (i.e. it runs with --enable_undo_delta_block_gc=false), that's not the expected behavior. It turned out the issue was in the way how DeltaTracker::EstimateBytesInPotentiallyAncientUndoDeltas() operated: it always treated a delta to be ancient if no stats were present. So, if a delta file was lazily loaded without stats being read, DeltaTracker assumed all its deltas were ancient. With the new behavior introduced in 1556a353e, it led to rowset merge compactions skipping the newly generated UNDO deltas since the estimate reported 100% of those deltas being ancient. While this was not detected by prior testing using various real-world scenarios involving some tangible amount of data written, tracking the history of stats emitted by dense_node-itest allowed to spot the issue. This patch addresses the issue, introducing different estimate modes for the method mentioned above and using proper modes in various contexts. I verified that with this patch added, the benchmark based on dense_node-itest now reports the number of blocks as it has been reporting for the longest span of its history. So I'm not adding any new tests with this patch. This is a follow-up to 1556a353e60c5d555996347cbd46d5e5a6661266. Change-Id: I17bddae86f84792caf14fb1e11a6e1c0d7a92b56 Reviewed-on: http://gerrit.cloudera.org:8080/19413 Tested-by: Kudu Jenkins Reviewed-by: Attila Bukor --- M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/memrowset.h M src/kudu/tablet/mock-rowsets.h M src/kudu/tablet/rowset.h M src/kudu/tablet/tablet.cc 8 files changed, 73 insertions(+), 29 deletions(-) Approvals: Kudu Jenkins: Verified Attila Bukor: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/19413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I17bddae86f84792caf14fb1e11a6e1c0d7a92b56 Gerrit-Change-Number: 19413 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang
[kudu-CR] plumb JWT authentication into clients
Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/18471 ) Change subject: plumb JWT authentication into clients .. Patch Set 16: (5 comments) http://gerrit.cloudera.org:8080/#/c/18471/16/src/kudu/integration-tests/security-itest.cc File src/kudu/integration-tests/security-itest.cc: PS16: > What are these changes for? Should they be in another patch? yeah, I think I messed up a rebase, thanks! http://gerrit.cloudera.org:8080/#/c/18471/16/src/kudu/rpc/client_negotiation.cc File src/kudu/rpc/client_negotiation.cc: http://gerrit.cloudera.org:8080/#/c/18471/16/src/kudu/rpc/client_negotiation.cc@200 PS16, Line 200: // When using SASL authentication, verifying the server's certificate is > Please update this comment Done http://gerrit.cloudera.org:8080/#/c/18471/16/src/kudu/rpc/negotiation.cc File src/kudu/rpc/negotiation.cc: PS16: > Again, no need to reorder includes if the file is untouched otherwise. Done http://gerrit.cloudera.org:8080/#/c/18471/16/src/kudu/security/tls_handshake.h File src/kudu/security/tls_handshake.h: PS16: > Same as in negotiation.cc Done http://gerrit.cloudera.org:8080/#/c/18471/16/src/kudu/server/server_base.cc File src/kudu/server/server_base.cc: http://gerrit.cloudera.org:8080/#/c/18471/16/src/kudu/server/server_base.cc@249 PS16, Line 249: "When true, read the JWT token out of the RPC and extract user " > Maybe expand on this comment to indicate that this enables JWT authenticati Done -- To view, visit http://gerrit.cloudera.org:8080/18471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibdfc2640c047a2e3bb5ea03aea4439cea2532e23 Gerrit-Change-Number: 18471 Gerrit-PatchSet: 16 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Tue, 17 Jan 2023 14:24:30 + Gerrit-HasComments: Yes
[kudu-CR] plumb JWT authentication into clients
Zoltan Chovan has uploaded a new patch set (#17) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18471 ) Change subject: plumb JWT authentication into clients .. plumb JWT authentication into clients This change plumbs the JWT authentication into the client and into the client negotiation (the JWTVerifier is set when building the Messenger). There following new flags are added: * enable_jwt_token_auth * jwt_validate_signature (unsafe) * jwt_allow_without_tls (unsafe) * jwks_file_path * jwks_url If 'enable_jwt_token_auth' is set to true, then either 'jwks_file_path' or 'jwks_url' has to be set, also both cannot be set at the same time. Co-authored-by: Andrew Wong Change-Id: Ibdfc2640c047a2e3bb5ea03aea4439cea2532e23 --- M src/kudu/client/client.cc M src/kudu/rpc/client_negotiation.cc M src/kudu/security/tls_handshake.cc M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc A src/kudu/util/jwt_test_certs.h 6 files changed, 460 insertions(+), 365 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/18471/17 -- To view, visit http://gerrit.cloudera.org:8080/18471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibdfc2640c047a2e3bb5ea03aea4439cea2532e23 Gerrit-Change-Number: 18471 Gerrit-PatchSet: 17 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] KUDU-3406 corrected estimate for ancient UNDO delta size
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/19413 ) Change subject: KUDU-3406 corrected estimate for ancient UNDO delta size .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/19413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17bddae86f84792caf14fb1e11a6e1c0d7a92b56 Gerrit-Change-Number: 19413 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Comment-Date: Tue, 17 Jan 2023 11:07:17 + Gerrit-HasComments: No
