[kudu-CR] Upgrade C++ to protobuf 3.12

2020-07-07 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/16135 )

Change subject: Upgrade C++ to protobuf 3.12
..

Upgrade C++ to protobuf 3.12

This has various performance improvements, and also enables Arena
support by default (used in a follow-on commit)

Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
Reviewed-on: http://gerrit.cloudera.org:8080/16135
Reviewed-by: Alexey Serbin 
Tested-by: Kudu Jenkins
---
M build-support/ubsan-blacklist.txt
M src/kudu/cfile/bloomfile.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_writer.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_cache.h
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/log_util.cc
M src/kudu/rpc/exactly_once_rpc-test.cc
M src/kudu/rpc/inbound_call.cc
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/rpc/serialization.cc
M src/kudu/rpc/serialization.h
M src/kudu/tablet/ops/op_tracker.cc
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_action_table.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/protobuf_util.h
M thirdparty/vars.sh
28 files changed, 125 insertions(+), 87 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Kudu Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/16135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
Gerrit-Change-Number: 16135
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Upgrade C++ to protobuf 3.12

2020-07-07 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16135 )

Change subject: Upgrade C++ to protobuf 3.12
..


Patch Set 5: Code-Review+2

(2 comments)

Thank you for this patch!

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/consensus_queue.cc
File src/kudu/consensus/consensus_queue.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/consensus_queue.cc@696
PS4, Line 696: LAGS_consensus_max_batch_size_bytes - request->ByteSizeLong();
> I'm afraid that's a behavioral change and don't want to let this "upgrade P
Yep, this makes sense to me.  I guess we can do that in a separate changelist.


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/rpc/result_tracker.h
File src/kudu/rpc/result_tracker.h:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/rpc/result_tracker.h@283
PS4, Line 283: int64_t
> perhaps but I didn't want to make more changes than necessary here lest som
Yup, that makes sense to me.  We might address that in a separate changelist.



--
To view, visit http://gerrit.cloudera.org:8080/16135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
Gerrit-Change-Number: 16135
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 07 Jul 2020 23:24:26 +
Gerrit-HasComments: Yes


[kudu-CR] Upgrade C++ to protobuf 3.12

2020-07-07 Thread Todd Lipcon (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/16135

to look at the new patch set (#5).

Change subject: Upgrade C++ to protobuf 3.12
..

Upgrade C++ to protobuf 3.12

This has various performance improvements, and also enables Arena
support by default (used in a follow-on commit)

Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
---
M build-support/ubsan-blacklist.txt
M src/kudu/cfile/bloomfile.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_writer.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_cache.h
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/log_util.cc
M src/kudu/rpc/exactly_once_rpc-test.cc
M src/kudu/rpc/inbound_call.cc
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/rpc/serialization.cc
M src/kudu/rpc/serialization.h
M src/kudu/tablet/ops/op_tracker.cc
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_action_table.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/protobuf_util.h
M thirdparty/vars.sh
28 files changed, 125 insertions(+), 87 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/16135/5
--
To view, visit http://gerrit.cloudera.org:8080/16135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
Gerrit-Change-Number: 16135
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Upgrade C++ to protobuf 3.12

2020-07-07 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16135 )

Change subject: Upgrade C++ to protobuf 3.12
..


Patch Set 4:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/cfile/bloomfile.cc
File src/kudu/cfile/bloomfile.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/cfile/bloomfile.cc@173
PS4, Line 173: hdr.ByteSizeLong()
> Maybe, wrap into static_cast() to be explicit on what's expected
Done


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/consensus_queue.cc
File src/kudu/consensus/consensus_queue.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/consensus_queue.cc@696
PS4, Line 696: LAGS_consensus_max_batch_size_bytes - request->ByteSizeLong();
> While you are here, do you mind adding a few lines to verify that FLAGS_con
I'm afraid that's a behavioral change and don't want to let this "upgrade PB" 
patch creep into semantic differences


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/log.cc
File src/kudu/consensus/log.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/log.cc@1240
PS4, Line 1240:   total_size_bytes_(
  :   PREDICT_FALSE(count == 1 && 
entry_batch_pb_.entry(0).type() == FLUSH_MARKER)
  :   ? 0 : entry_batch_pb_.ByteSizeLong())
> Does it make sense to change type of LogEntryBatch::total_size_bytes_ to si
Done


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/log_cache.h
File src/kudu/consensus/log_cache.h:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/log_cache.h@76
PS4, Line 76: ByteSize
> ByteSizeLong()
Done


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/log_util.cc
File src/kudu/consensus/log_util.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/log_util.cc@785
PS4, Line 785: new_header.ByteSizeLong()
> nit: wrap into static_cast() ?
Done


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/log_util.cc@807
PS4, Line 807: footer.ByteSizeLong()
> ditto
Done


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/rpc/inbound_call.cc
File src/kudu/rpc/inbound_call.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/rpc/inbound_call.cc@181
PS4, Line 181: response.ByteSizeLong();
> What if response.ByteSizeLong() > 2^32 ?  Does it make sense to add CHECK()
done


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/rpc/outbound_call.cc
File src/kudu/rpc/outbound_call.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/rpc/outbound_call.cc@148
PS4, Line 148: req.ByteSizeLong();
> Add DCHECK() on ByteSizeLong() to be less that 2^32 ?
Done


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/rpc/result_tracker.h
File src/kudu/rpc/result_tracker.h:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/rpc/result_tracker.h@283
PS4, Line 283: int64_t
> Does it make sense to switch this to size_t along with kudu_malloc_usable_s
perhaps but I didn't want to make more changes than necessary here lest some 
unintended behavioral change leak in


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/tools/tool_action_pbc.cc
File src/kudu/tools/tool_action_pbc.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/tools/tool_action_pbc.cc@205
PS4, Line 205:   JsonParseOptions opts;
 :   opts.case_insensitive_enum_parsing = true;
> nit: move this outside of the cycle?
Done


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/util/pb_util.cc
File src/kudu/util/pb_util.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/util/pb_util.cc@793
PS4, Line 793: DCHECK_GE(data_len_long, 0) << "Error computing ByteSize";
> Is this effective anymore?
Done


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/util/pb_util.cc@808
PS4, Line 808: static_cast
> drop?
Done


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/util/protobuf_util.h
File src/kudu/util/protobuf_util.h:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/util/protobuf_util.h@37
PS4, Line 37: ByteSize
> ByteSizeLong
Done



--
To view, visit http://gerrit.cloudera.org:8080/16135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
Gerrit-Change-Number: 16135
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 07 Jul 2020 22:04:07 +
Gerrit-HasComments: Yes


[kudu-CR] Upgrade C++ to protobuf 3.12

2020-07-07 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16135 )

Change subject: Upgrade C++ to protobuf 3.12
..


Patch Set 4:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/cfile/bloomfile.cc
File src/kudu/cfile/bloomfile.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/cfile/bloomfile.cc@173
PS4, Line 173: hdr.ByteSizeLong()
Maybe, wrap into static_cast() to be explicit on what's expected from 
BloomBlockHeaderPB here?


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/consensus_queue.cc
File src/kudu/consensus/consensus_queue.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/consensus_queue.cc@696
PS4, Line 696: LAGS_consensus_max_batch_size_bytes - request->ByteSizeLong();
While you are here, do you mind adding a few lines to verify that 
FLAGS_consensus_max_batch_size_bytes >= request->ByteSizeLong() and handle the 
unexpected case appropriately (I guess returning Status::InvalidArgument() 
might be a good option)?


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/log.cc
File src/kudu/consensus/log.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/log.cc@1240
PS4, Line 1240:   total_size_bytes_(
  :   PREDICT_FALSE(count == 1 && 
entry_batch_pb_.entry(0).type() == FLUSH_MARKER)
  :   ? 0 : entry_batch_pb_.ByteSizeLong())
Does it make sense to change type of LogEntryBatch::total_size_bytes_ to size_t 
as well or wrap entry_batch_pb_.ByteSizeLong() into static_cast()  ?


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/log_cache.h
File src/kudu/consensus/log_cache.h:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/log_cache.h@76
PS4, Line 76: ByteSize
ByteSizeLong()


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/log_util.cc
File src/kudu/consensus/log_util.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/log_util.cc@785
PS4, Line 785: new_header.ByteSizeLong()
nit: wrap into static_cast() ?


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/consensus/log_util.cc@807
PS4, Line 807: footer.ByteSizeLong()
ditto


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/rpc/inbound_call.cc
File src/kudu/rpc/inbound_call.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/rpc/inbound_call.cc@181
PS4, Line 181: response.ByteSizeLong();
What if response.ByteSizeLong() > 2^32 ?  Does it make sense to add CHECK() or 
something like that here?


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/rpc/outbound_call.cc
File src/kudu/rpc/outbound_call.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/rpc/outbound_call.cc@148
PS4, Line 148: req.ByteSizeLong();
Add DCHECK() on ByteSizeLong() to be less that 2^32 ?


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/rpc/result_tracker.h
File src/kudu/rpc/result_tracker.h:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/rpc/result_tracker.h@283
PS4, Line 283: int64_t
Does it make sense to switch this to size_t along with kudu_malloc_usable_size ?


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/tools/tool_action_pbc.cc
File src/kudu/tools/tool_action_pbc.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/tools/tool_action_pbc.cc@205
PS4, Line 205:   JsonParseOptions opts;
 :   opts.case_insensitive_enum_parsing = true;
nit: move this outside of the cycle?


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/util/pb_util.cc
File src/kudu/util/pb_util.cc:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/util/pb_util.cc@793
PS4, Line 793: DCHECK_GE(data_len_long, 0) << "Error computing ByteSize";
Is this effective anymore?


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/util/pb_util.cc@808
PS4, Line 808: static_cast
drop?


http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/util/protobuf_util.h
File src/kudu/util/protobuf_util.h:

http://gerrit.cloudera.org:8080/#/c/16135/4/src/kudu/util/protobuf_util.h@37
PS4, Line 37: ByteSize
ByteSizeLong



--
To view, visit http://gerrit.cloudera.org:8080/16135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
Gerrit-Change-Number: 16135
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 07 Jul 2020 06:31:24 +
Gerrit-HasComments: Yes


[kudu-CR] Upgrade C++ to protobuf 3.12

2020-07-06 Thread Todd Lipcon (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/16135

to look at the new patch set (#4).

Change subject: Upgrade C++ to protobuf 3.12
..

Upgrade C++ to protobuf 3.12

This has various performance improvements, and also enables Arena
support by default (used in a follow-on commit)

Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
---
M build-support/ubsan-blacklist.txt
M src/kudu/cfile/bloomfile.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_writer.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_cache.h
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/log_util.cc
M src/kudu/rpc/exactly_once_rpc-test.cc
M src/kudu/rpc/inbound_call.cc
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/rpc/serialization.cc
M src/kudu/rpc/serialization.h
M src/kudu/tablet/ops/op_tracker.cc
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_action_table.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/protobuf_util.h
M thirdparty/vars.sh
27 files changed, 117 insertions(+), 82 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/16135/4
--
To view, visit http://gerrit.cloudera.org:8080/16135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
Gerrit-Change-Number: 16135
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Upgrade C++ to protobuf 3.12

2020-07-06 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16135 )

Change subject: Upgrade C++ to protobuf 3.12
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16135/3/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/16135/3/thirdparty/vars.sh@57
PS3, Line 57: PROTOBUF_VERSION=3.12.3
> In this change (unmerged) I also changed the build to use the protobuf-cpp
ah, I don't have access to upload to the thirdparty bucket anymore, so will 
keep as is for now, but if you want to do a follow-on, go for it.



-- 
To view, visit http://gerrit.cloudera.org:8080/16135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
Gerrit-Change-Number: 16135
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 07 Jul 2020 04:46:59 +
Gerrit-HasComments: Yes


[kudu-CR] Upgrade C++ to protobuf 3.12

2020-07-05 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16135 )

Change subject: Upgrade C++ to protobuf 3.12
..


Patch Set 3:

(1 comment)

Looks like there is still an UBSAN failure that might need to be ignored too?

http://gerrit.cloudera.org:8080/#/c/16135/3/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/16135/3/thirdparty/vars.sh@57
PS3, Line 57: PROTOBUF_VERSION=3.12.3
In this change (unmerged) I also changed the build to use the protobuf-cpp 
source instead of the full protobuf source to make thirdparty smaller.
https://gerrit.cloudera.org/#/c/15939/

Think thats worth doing?



-- 
To view, visit http://gerrit.cloudera.org:8080/16135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
Gerrit-Change-Number: 16135
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 06 Jul 2020 00:33:00 +
Gerrit-HasComments: Yes


[kudu-CR] Upgrade C++ to protobuf 3.12

2020-07-02 Thread Todd Lipcon (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/16135

to look at the new patch set (#3).

Change subject: Upgrade C++ to protobuf 3.12
..

Upgrade C++ to protobuf 3.12

This has various performance improvements, and also enables Arena
support by default (used in a follow-on commit)

Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
---
M build-support/ubsan-blacklist.txt
M src/kudu/cfile/bloomfile.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_writer.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_cache.h
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/log_util.cc
M src/kudu/rpc/exactly_once_rpc-test.cc
M src/kudu/rpc/inbound_call.cc
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/rpc/serialization.cc
M src/kudu/rpc/serialization.h
M src/kudu/tablet/ops/op_tracker.cc
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_action_table.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/protobuf_util.h
M thirdparty/vars.sh
27 files changed, 114 insertions(+), 82 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/16135/3
--
To view, visit http://gerrit.cloudera.org:8080/16135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
Gerrit-Change-Number: 16135
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)


[kudu-CR] Upgrade C++ to protobuf 3.12

2020-07-02 Thread Todd Lipcon (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/16135

to look at the new patch set (#2).

Change subject: Upgrade C++ to protobuf 3.12
..

Upgrade C++ to protobuf 3.12

This has various performance improvements, and also enables Arena
support by default (used in a follow-on commit)

Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
---
M src/kudu/cfile/bloomfile.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_writer.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_cache.h
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/log_util.cc
M src/kudu/rpc/exactly_once_rpc-test.cc
M src/kudu/rpc/inbound_call.cc
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/rpc/serialization.cc
M src/kudu/rpc/serialization.h
M src/kudu/tablet/ops/op_tracker.cc
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_action_table.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/protobuf_util.h
M thirdparty/vars.sh
26 files changed, 111 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/16135/2
--
To view, visit http://gerrit.cloudera.org:8080/16135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
Gerrit-Change-Number: 16135
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)


[kudu-CR] Upgrade C++ to protobuf 3.12

2020-07-01 Thread Todd Lipcon (Code Review)
Hello Alexey Serbin,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/16135

to review the following change.


Change subject: Upgrade C++ to protobuf 3.12
..

Upgrade C++ to protobuf 3.12

This has various performance improvements, and also enables Arena
support by default (used in a follow-on commit)

Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
---
M src/kudu/cfile/bloomfile.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_writer.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_cache.h
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/log_util.cc
M src/kudu/rpc/exactly_once_rpc-test.cc
M src/kudu/rpc/inbound_call.cc
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/rpc/serialization.cc
M src/kudu/rpc/serialization.h
M src/kudu/tablet/ops/op_tracker.cc
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/protobuf_util.h
M thirdparty/vars.sh
24 files changed, 93 insertions(+), 76 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/16135/1
--
To view, visit http://gerrit.cloudera.org:8080/16135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifd5e6bbdd2c517b1af2ff833701983315111bba4
Gerrit-Change-Number: 16135
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin