[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18330 ) Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty .. Patch Set 19: (13 comments) I just took another quick look. I'm not sure current split of fs.proto makes a lot of sense. Are you sure the change in the instance metadata files size is due adding this new attribute, but not other fields recently added in the context of multi-tenancy? http://gerrit.cloudera.org:8080/#/c/18330/19//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18330/19//COMMIT_MSG@7 PS19, Line 7: Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty Maybe, for clarity update this to be fix UUID format in 'kudu dump instance' output http://gerrit.cloudera.org:8080/#/c/18330/19//COMMIT_MSG@9 PS19, Line 9: The output of two commands below about the 'uuid' field are not the same Maybe, replace with UUID fields were output in different formats by the following commands: http://gerrit.cloudera.org:8080/#/c/18330/19//COMMIT_MSG@24 PS19, Line 24: '--debug/default --json/json_pretty ? http://gerrit.cloudera.org:8080/#/c/18330/19/src/kudu/fs/fs_with_extend.proto File src/kudu/fs/fs_with_extend.proto: http://gerrit.cloudera.org:8080/#/c/18330/19/src/kudu/fs/fs_with_extend.proto@22 PS19, Line 22: LBM 10 times larger than before or so (7200 bytes vs 836 bytes). Are you sure that's because of the new attribute, not because of the new 'tenant' field that has been added recently? http://gerrit.cloudera.org:8080/#/c/18330/19/src/kudu/util/easy_json.h File src/kudu/util/easy_json.h: http://gerrit.cloudera.org:8080/#/c/18330/19/src/kudu/util/easy_json.h@27 PS19, Line 27: pretty json or json I'm not sure this comment adds any useful information. Consider updating this: the idea is to http://gerrit.cloudera.org:8080/#/c/18330/19/src/kudu/util/easy_json.h@72 PS19, Line 72: Pass external Json object. ? What are the ownership rules for the pointer passed? Who owns the memory pointed by the 'value' parameter? http://gerrit.cloudera.org:8080/#/c/18330/19/src/kudu/util/pb_util.h File src/kudu/util/pb_util.h: http://gerrit.cloudera.org:8080/#/c/18330/19/src/kudu/util/pb_util.h@422 PS19, Line 422: bytes fields byte fields http://gerrit.cloudera.org:8080/#/c/18330/19/src/kudu/util/pb_util.h@423 PS19, Line 423: json JSON http://gerrit.cloudera.org:8080/#/c/18330/19/src/kudu/util/pb_util.h@423 PS19, Line 423: protobuf container What's "protobuf container" in this context? http://gerrit.cloudera.org:8080/#/c/18330/19/src/kudu/util/pb_util.h@428 PS19, Line 428: plain format What's plain format in this context? I'm not sure I understand. http://gerrit.cloudera.org:8080/#/c/18330/19/src/kudu/util/pb_util.h@430 PS19, Line 430: protobuf container What's 'protobuf container'? http://gerrit.cloudera.org:8080/#/c/18330/19/src/kudu/util/pb_util.proto File src/kudu/util/pb_util.proto: http://gerrit.cloudera.org:8080/#/c/18330/19/src/kudu/util/pb_util.proto@46 PS19, Line 46: // This is for 'pbc dump' when using 'json/json-pretty' format, some bytes fields : // will output its base64-encoded text, set this option, we can output base64-decoded : // text the same as using plain format. : // This option is only used for bytes fields, if some fields with other types set this option, : // The command 'pbc dump' will ignore it. Consider rewriting this -- it reads a bit convoluted as of now. http://gerrit.cloudera.org:8080/#/c/18330/19/src/kudu/util/pb_util.proto@51 PS19, Line 51: PBC_ It doesn't have to be just the 'kudu pbc dump' tool -- that's about a way to represent byte PB fields in various human and machine-readable output. So, drop the 'PBC_' prefix. Instead, I'd suggest to name this as RAW or NO_ENCODING as opposed to any encoding, such as base64 or alike when outputting byte fields. -- To view, visit http://gerrit.cloudera.org:8080/18330 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf Gerrit-Change-Number: 18330 Gerrit-PatchSet: 19 Gerrit-Owner: Yuqi Du Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du Gerrit-Comment-Date: Wed, 19 Jul 2023 02:28:07 + Gerrit-HasComments: Yes
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Yuqi Du has removed a vote on this change. Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/18330 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf Gerrit-Change-Number: 18330 Gerrit-PatchSet: 19 Gerrit-Owner: Yuqi Du Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18330 ) Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty .. Patch Set 19: > Patch Set 19: Verified+1 > > > Patch Set 19: Verified-1 > > > > Build Failed > > > > http://jenkins.kudu.apache.org/job/kudu-gerrit/28112/ : FAILURE > > > There are 2 failure. > > 1. DEBUG, rebalance_tool a test kudu is coredump. > > 2. ASAN. There was 1 failure: > 1) testFlushBySize(org.apache.kudu.client.TestAsyncKuduSession) > com.stumbleupon.async.TimeoutException: Timed out after 5ms when joining > Deferred@398562602(state=PENDING, result=null, callback=wakeup thread > Time-limited test, errback=wakeup thread Time-limited test) > at com.stumbleupon.async.Deferred.doJoin(Deferred.java:1183) > at com.stumbleupon.async.Deferred.join(Deferred.java:1042) > at > org.apache.kudu.client.TestAsyncKuduSession.testFlushBySize(TestAsyncKuduSession.java:409) > > FAILURES!!! > Tests run: 12, Failures: 1 > > > I'll review the 2 unsteady tests later. The two fails do not matter with this patch. -- To view, visit http://gerrit.cloudera.org:8080/18330 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf Gerrit-Change-Number: 18330 Gerrit-PatchSet: 19 Gerrit-Owner: Yuqi Du Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du Gerrit-Comment-Date: Fri, 30 Jun 2023 07:06:25 + Gerrit-HasComments: No
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18330 ) Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty .. Patch Set 19: (1 comment) http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc File src/kudu/util/pb_util.cc: http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc@989 PS14, Line 989: // when users use '--json or --json_pr > I should study about this Thanks for your advices. I have added a new extend. You can review it again. And this fix will cause metadata file's header becomes a litter large. So I extract a fs_with_extends.proto file to avoid this disadvantage changes. -- To view, visit http://gerrit.cloudera.org:8080/18330 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf Gerrit-Change-Number: 18330 Gerrit-PatchSet: 19 Gerrit-Owner: Yuqi Du Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du Gerrit-Comment-Date: Fri, 30 Jun 2023 07:04:52 + Gerrit-HasComments: Yes
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18330 ) Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty .. Patch Set 19: Verified+1 > Patch Set 19: Verified-1 > > Build Failed > > http://jenkins.kudu.apache.org/job/kudu-gerrit/28112/ : FAILURE There are 2 failure. 1. DEBUG, rebalance_tool a test kudu is coredump. 2. ASAN. There was 1 failure: 1) testFlushBySize(org.apache.kudu.client.TestAsyncKuduSession) com.stumbleupon.async.TimeoutException: Timed out after 5ms when joining Deferred@398562602(state=PENDING, result=null, callback=wakeup thread Time-limited test, errback=wakeup thread Time-limited test) at com.stumbleupon.async.Deferred.doJoin(Deferred.java:1183) at com.stumbleupon.async.Deferred.join(Deferred.java:1042) at org.apache.kudu.client.TestAsyncKuduSession.testFlushBySize(TestAsyncKuduSession.java:409) FAILURES!!! Tests run: 12, Failures: 1 I'll review the 2 unsteady tests later. -- To view, visit http://gerrit.cloudera.org:8080/18330 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf Gerrit-Change-Number: 18330 Gerrit-PatchSet: 19 Gerrit-Owner: Yuqi Du Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du Gerrit-Comment-Date: Fri, 30 Jun 2023 06:48:59 + Gerrit-HasComments: No
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Hello Alexey Serbin, Ashwani Raina, Yingchun Lai, Yifan Zhang, Kudu Jenkins,
Abhishek Chennaka, KeDeng, Wang Xixu,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18330
to look at the new patch set (#19).
Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using
-json/-json_pretty
..
[tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty
The output of two commands below about the 'uuid' field are not the same
1. kudu pbc dump xxx/instance -debug/default
2. kudu pbc dump xxx/instance -json/-json_pretty
For example:
1. kudu pbc dump instance, which is a plain output format.
Message 0
---
uuid: "ac6b3e392cb34b4681e54a33c95fdf9c"
format_stamp: "Formatted at 2023-03-10 09:16:22 on 2a43db713d56"
2. kudu pbc dump instance --json, which is a json output format
{"uuid":"YWM2YjNlMzkyY2IzNGI0NjgxZTU0YTMzYzk1ZmRmOWM=","formatStamp":"Formatted
at 2023-03-10 09:16:22 on 2a43db713d56"}
The reason is the type of 'uuid' is bytes and protobuf's bytes type is
base64-encoded.
'--debug/default' mode shows uuid's plain text while '--debug/default'
mode uses protobuf's API and shows base64-encoded text.
This patch fixes this problem.
Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf
---
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/fs.proto
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
A src/kudu/fs/fs_with_extend.proto
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/util/easy_json.cc
M src/kudu/util/easy_json.h
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
M src/kudu/util/pb_util.proto
14 files changed, 197 insertions(+), 80 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/18330/19
--
To view, visit http://gerrit.cloudera.org:8080/18330
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf
Gerrit-Change-Number: 18330
Gerrit-PatchSet: 19
Gerrit-Owner: Yuqi Du
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Ashwani Raina
Gerrit-Reviewer: KeDeng
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai
Gerrit-Reviewer: Yuqi Du
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Hello Alexey Serbin, Ashwani Raina, Yingchun Lai, Yifan Zhang, Kudu Jenkins,
Abhishek Chennaka, KeDeng, Wang Xixu,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18330
to look at the new patch set (#17).
Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using
-json/-json_pretty
..
[tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty
The output of two commands below about the 'uuid' field are not the same
1. kudu pbc dump xxx/instance -debug/default
2. kudu pbc dump xxx/instance -json/-json_pretty
For example:
1. kudu pbc dump instance, which is a plain output format.
Message 0
---
uuid: "ac6b3e392cb34b4681e54a33c95fdf9c"
format_stamp: "Formatted at 2023-03-10 09:16:22 on 2a43db713d56"
2. kudu pbc dump instance --json, which is a json output format
{"uuid":"YWM2YjNlMzkyY2IzNGI0NjgxZTU0YTMzYzk1ZmRmOWM=","formatStamp":"Formatted
at 2023-03-10 09:16:22 on 2a43db713d56"}
The reason is the type of 'uuid' is bytes and protobuf's bytes type is
base64-encoded.
'--debug/default' mode shows uuid's plain text while '--debug/default'
mode uses protobuf's API and shows base64-encoded text.
This patch fixes this problem.
Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf
---
M src/kudu/fs/fs.proto
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/util/easy_json.cc
M src/kudu/util/easy_json.h
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
M src/kudu/util/pb_util.proto
8 files changed, 103 insertions(+), 24 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/18330/17
--
To view, visit http://gerrit.cloudera.org:8080/18330
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf
Gerrit-Change-Number: 18330
Gerrit-PatchSet: 17
Gerrit-Owner: Yuqi Du
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Ashwani Raina
Gerrit-Reviewer: KeDeng
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai
Gerrit-Reviewer: Yuqi Du
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Yuqi Du has posted comments on this change. (
http://gerrit.cloudera.org:8080/18330 )
Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using
-json/-json_pretty
..
Patch Set 16:
(13 comments)
Thanks for your advices and I fixed them. You can review it again.
http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG@12
PS14, Line 12:
> It is better to describe the difference of the first command and the second
Done
http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG@13
PS14, Line 13:
> base64-encoded
Done
http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG@14
PS14, Line 14: . kudu pbc dump instance, which is a plain output format.
> Here you need to compare '--debug/default' mode with '-json/-json_pretty' m
I guess you mean I should paste the two commands output at this. So I do this.
http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG@15
PS14, Line 15:
> base64-encoded
Done
http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG@15
PS14, Line 15:
> API
Done
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.h
File src/kudu/util/pb_util.h:
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.h@422
PS14, Line 422: // 'BytesTextDumpFlag
> Should the name of this enum contain some reference on the context where it
Done
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.h@423
PS14, Line 423: when u
> That's not the best name for the enum tag. Maybe, name this as BASE64_ENCO
ok
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.h@427
PS14, Line 427: enum class BytesFieldDumpFlag {
> Use DECODE_UUID as the default value is a little confusing. There is a Dump
Yes. I fixed the names.
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc
File src/kudu/util/pb_util.cc:
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc@989
PS14, Line 989: const std::string kUuidField = "uuid";
> Instead of having this field name hard-coded here, consider something simil
I should study about this
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc@990
PS14, Line 990: decode it from base6
> decode it from base64 format
Done
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc@994
PS14, Line 994: // Maybe it's necessary to consider other 'bytes' fields
which can be dumped.
> If the whole message is encoded in base64 format, can we decode whole messa
No. Only bytes fields, in 'kudu pbc dump instance' is the uuid. Other field is
normal , not based64-encoded.
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc@1027
PS14, Line 1027: JsonFormat json_format = JsonFormat::JSON;
> nit: is it possible to move this to line 1024, to be after Format::JSON but
It's hard to make the two 'case' using a common variable except moving this
statement before switch.
L1029 has an example.
So, I think this statement at here it ok.
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc@1044
PS14, Line 1044: RETURN_N
> Here and below: why not to use RETURN_NOT_OK() here? I.e., why to crash if
Yes. This CHECK is not necessary. I fix them.
--
To view, visit http://gerrit.cloudera.org:8080/18330
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf
Gerrit-Change-Number: 18330
Gerrit-PatchSet: 16
Gerrit-Owner: Yuqi Du
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Ashwani Raina
Gerrit-Reviewer: KeDeng
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai
Gerrit-Reviewer: Yuqi Du
Gerrit-Comment-Date: Tue, 13 Jun 2023 16:37:06 +
Gerrit-HasComments: Yes
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Hello Alexey Serbin, Ashwani Raina, Yingchun Lai, Yifan Zhang, Kudu Jenkins,
Abhishek Chennaka, KeDeng, Wang Xixu,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18330
to look at the new patch set (#16).
Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using
-json/-json_pretty
..
[tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty
The output of two commands below about the 'uuid' field are not the same
1. kudu pbc dump xxx/instance -debug/default
2. kudu pbc dump xxx/instance -json/-json_pretty
For example:
1. kudu pbc dump instance, which is a plain output format.
Message 0
---
uuid: "ac6b3e392cb34b4681e54a33c95fdf9c"
format_stamp: "Formatted at 2023-03-10 09:16:22 on 2a43db713d56"
2. kudu pbc dump instance --json, which is a json output format
{"uuid":"YWM2YjNlMzkyY2IzNGI0NjgxZTU0YTMzYzk1ZmRmOWM=","formatStamp":"Formatted
at 2023-03-10 09:16:22 on 2a43db713d56"}
The reason is the type of 'uuid' is bytes and protobuf's bytes type is
base64-encoded.
'--debug/default' mode shows uuid's plain text while '--debug/default'
mode uses protobuf's API and shows base64-encoded text.
This patch fixes this problem.
Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/util/easy_json.cc
M src/kudu/util/easy_json.h
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
6 files changed, 87 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/18330/16
--
To view, visit http://gerrit.cloudera.org:8080/18330
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf
Gerrit-Change-Number: 18330
Gerrit-PatchSet: 16
Gerrit-Owner: Yuqi Du
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Ashwani Raina
Gerrit-Reviewer: KeDeng
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai
Gerrit-Reviewer: Yuqi Du
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Hello Alexey Serbin, Ashwani Raina, Yingchun Lai, Yifan Zhang, Kudu Jenkins,
Abhishek Chennaka, KeDeng, Wang Xixu,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18330
to look at the new patch set (#15).
Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using
-json/-json_pretty
..
[tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty
The output of two commands below about the 'uuid' field are not the same
1. kudu pbc dump xxx/instance -debug/default
2. kudu pbc dump xxx/instance -json/-json_pretty
For example:
1. kudu pbc dump instance, which is a plain output format.
Message 0
---
uuid: "ac6b3e392cb34b4681e54a33c95fdf9c"
format_stamp: "Formatted at 2023-03-10 09:16:22 on 2a43db713d56"
2. kudu pbc dump instance --json, which is a json output format
{"uuid":"YWM2YjNlMzkyY2IzNGI0NjgxZTU0YTMzYzk1ZmRmOWM=","formatStamp":"Formatted
at 2023-03-10 09:16:22 on 2a43db713d56"}
The reason is the type of 'uuid' is bytes and protobuf's bytes type is
base64-encoded.
'--debug/default' mode shows uuid's plain text while '--debug/default'
mode uses protobuf's API and shows base64-encoded text.
This patch fixes this problem.
Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/util/easy_json.cc
M src/kudu/util/easy_json.h
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
6 files changed, 87 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/18330/15
--
To view, visit http://gerrit.cloudera.org:8080/18330
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf
Gerrit-Change-Number: 18330
Gerrit-PatchSet: 15
Gerrit-Owner: Yuqi Du
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Ashwani Raina
Gerrit-Reviewer: KeDeng
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai
Gerrit-Reviewer: Yuqi Du
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/18330 )
Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using
-json/-json_pretty
..
Patch Set 14:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG@13
PS14, Line 13: base64 encoded
base64-encoded
http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG@15
PS14, Line 15: apis
API
http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG@15
PS14, Line 15: base64 encoded
base64-encoded
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.h
File src/kudu/util/pb_util.h:
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.h@422
PS14, Line 422: enum class DumpFlag {
Should the name of this enum contain some reference on the context where it's
used, i.e. something related to UUID, e.g. UUIDTextDump?
It would be great to have a comment to describe this enumeration: the context
where can be used in general and where it's used at least now when its have
just been introduced.
Also, please document every enumeration tag as well.
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.h@423
PS14, Line 423: DEFAULT
That's not the best name for the enum tag. Maybe, name this as BASE64_ENCODED?
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.h@424
PS14, Line 424: DECODE_UUID
Maybe, name this as UUID_HEX?
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc
File src/kudu/util/pb_util.cc:
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc@989
PS14, Line 989: const std::string kUuidField = "uuid";
Instead of having this field name hard-coded here, consider something similar
to the KUDU.redact option: that way a special custom protobuf option could be
added to any protobuf field.
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc@1027
PS14, Line 1027: JsonFormat json_format = JsonFormat::JSON;
nit: is it possible to move this to line 1024, to be after Format::JSON but
before Format::JSON_PRETTY?
http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc@1044
PS14, Line 1044: CHECK_OK
Here and below: why not to use RETURN_NOT_OK() here? I.e., why to crash if the
input isn't what it expected to be?
--
To view, visit http://gerrit.cloudera.org:8080/18330
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf
Gerrit-Change-Number: 18330
Gerrit-PatchSet: 14
Gerrit-Owner: Yuqi Du
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Ashwani Raina
Gerrit-Reviewer: KeDeng
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai
Gerrit-Reviewer: Yuqi Du
Gerrit-Comment-Date: Mon, 12 Jun 2023 18:25:27 +
Gerrit-HasComments: Yes
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/18330 ) Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty .. Patch Set 14: (5 comments) http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG@12 PS14, Line 12: These two commands' outputs about the 'uuid' field are not the same It is better to describe the difference of the first command and the second command. http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG@14 PS14, Line 14: --debug/default' mode shows uuid's plain text while '--debug/default' Here you need to compare '--debug/default' mode with '-json/-json_pretty' mode. http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.h File src/kudu/util/pb_util.h: http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.h@427 PS14, Line 427: Status Dump(std::ostream* os, Format format, DumpFlag flag = DumpFlag::DECODE_UUID); Use DECODE_UUID as the default value is a little confusing. There is a DumpFlag naming DEFAULT. Maybe you can rename DEFAULT as ENCODED_UUID. http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc File src/kudu/util/pb_util.cc: http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc@990 PS14, Line 990: use base64 decode it decode it from base64 format http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc@994 PS14, Line 994: // Maybe it's necessary to consider other 'bytes' fields which can be dumped. If the whole message is encoded in base64 format, can we decode whole message from base64 format not only decode one field? -- To view, visit http://gerrit.cloudera.org:8080/18330 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf Gerrit-Change-Number: 18330 Gerrit-PatchSet: 14 Gerrit-Owner: Yuqi Du Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du Gerrit-Comment-Date: Mon, 05 Jun 2023 06:59:20 + Gerrit-HasComments: Yes
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Hello Alexey Serbin, Ashwani Raina, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Abhishek Chennaka, KeDeng, Wang Xixu, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/18330 to look at the new patch set (#14). Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty .. [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty The two commands below: 1. kudu pbc dump xxx/instance -debug/default 2. kudu pbc dump xxx/instance -json/-json_pretty These two commands' outputs about the 'uuid' field are not the same, because the type of 'uuid' is bytes and protobuf's bytes type is base64 encoded. '--debug/default' mode shows uuid's plain text while '--debug/default' mode uses protobuf's apis and shows base64 encoded text. This patch fixes this problem. Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_pbc.cc M src/kudu/util/easy_json.cc M src/kudu/util/easy_json.h M src/kudu/util/pb_util.cc M src/kudu/util/pb_util.h 6 files changed, 75 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/18330/14 -- To view, visit http://gerrit.cloudera.org:8080/18330 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf Gerrit-Change-Number: 18330 Gerrit-PatchSet: 14 Gerrit-Owner: Yuqi Du Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Hello Alexey Serbin, Ashwani Raina, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Abhishek Chennaka, KeDeng, Wang Xixu, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/18330 to look at the new patch set (#13). Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty .. [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty The two commands below: 1. kudu pbc dump xxx/instance -debug/default 2. kudu pbc dump xxx/instance -json/-json_pretty These two commands' outputs about the 'uuid' field are not the same, because the type of 'uuid' is bytes and protobuf's bytes type is base64 encoded. '--debug/default' mode shows uuid's plain text while '--debug/default' mode uses protobuf's apis and shows base64 encoded text. This patch fixes this problem. Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_pbc.cc M src/kudu/util/easy_json.cc M src/kudu/util/easy_json.h M src/kudu/util/pb_util.cc M src/kudu/util/pb_util.h 6 files changed, 75 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/18330/13 -- To view, visit http://gerrit.cloudera.org:8080/18330 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf Gerrit-Change-Number: 18330 Gerrit-PatchSet: 13 Gerrit-Owner: Yuqi Du Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Hello Yingchun Lai, Yifan Zhang, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/18330 to look at the new patch set (#11). Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty .. [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty The two commands below: 1. kudu pbc dump xxx/instance -debug/default 2. kudu pbc dump xxx/instance -json/-json_pretty These two commands' outputs about the 'uuid' field are not the same, because the type of 'uuid' is bytes and protobuf's bytes type is base64 encoded. '--debug/default' mode shows uuid's plain text while '--debug/default' mode uses protobuf's apis and shows base64 encoded text. This patch fixes this problem. Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_pbc.cc M src/kudu/util/easy_json.cc M src/kudu/util/easy_json.h M src/kudu/util/pb_util.cc M src/kudu/util/pb_util.h 6 files changed, 75 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/18330/11 -- To view, visit http://gerrit.cloudera.org:8080/18330 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf Gerrit-Change-Number: 18330 Gerrit-PatchSet: 11 Gerrit-Owner: Yuqi Du Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Hello Yingchun Lai, Yifan Zhang, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/18330 to look at the new patch set (#9). Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty .. [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty The two commands below: 1. kudu pbc dump xxx/instance -debug/default 2. kudu pbc dump xxx/instance -json/-json_pretty These two commands' outputs about the 'uuid' field are not the same, because the type of 'uuid' is bytes and protobuf's bytes type is base64 encoded. '--debug/default' mode shows uuid's plain text while '--debug/default' mode uses protobuf's apis and shows base64 encoded text. This patch fixes this problem. Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_pbc.cc M src/kudu/util/easy_json.cc M src/kudu/util/easy_json.h M src/kudu/util/pb_util.cc M src/kudu/util/pb_util.h 6 files changed, 77 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/18330/9 -- To view, visit http://gerrit.cloudera.org:8080/18330 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf Gerrit-Change-Number: 18330 Gerrit-PatchSet: 9 Gerrit-Owner: Yuqi Du Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du
[kudu-CR] [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json pretty
Hello Yingchun Lai, Yifan Zhang, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/18330 to look at the new patch set (#8). Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty .. [tool] Fix outputs 'kudu pbc dump instance' when using -json/-json_pretty The two commands below: kudu pbc dump xxx/instance -debug kudu pbc dump xxx/instance -json/-json_pretty These two commands' outputs, the uuid is not the same, because uuid is bytes and pb' bytes is base64 encoded. This patch fixes this problem. Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_pbc.cc M src/kudu/util/easy_json.cc M src/kudu/util/easy_json.h M src/kudu/util/pb_util.cc M src/kudu/util/pb_util.h 6 files changed, 77 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/18330/8 -- To view, visit http://gerrit.cloudera.org:8080/18330 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf Gerrit-Change-Number: 18330 Gerrit-PatchSet: 8 Gerrit-Owner: Yuqi Du Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du
