[kudu-CR] KUDU-2971 p2: add basic protobuf msg
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/14426 )
Change subject: KUDU-2971 p2: add basic protobuf msg
..
Patch Set 8:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14426/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14426/7//COMMIT_MSG@7
PS7, Line 7: KUDU-2971
> KUDU-2971
Done
http://gerrit.cloudera.org:8080/#/c/14426/1/src/kudu/subprocess/subprocess.proto
File src/kudu/subprocess/subprocess.proto:
http://gerrit.cloudera.org:8080/#/c/14426/1/src/kudu/subprocess/subprocess.proto@34
PS1, Line 34: }
> yeah but we use proto2 syntax, I thought proto3 would be necessary here. Ne
Ack
http://gerrit.cloudera.org:8080/#/c/14426/4/src/kudu/subprocess/subprocess.proto
File src/kudu/subprocess/subprocess.proto:
http://gerrit.cloudera.org:8080/#/c/14426/4/src/kudu/subprocess/subprocess.proto@17
PS4, Line 17:
> nit: does it make sense to add a small comment pointing to the fact that th
Done
http://gerrit.cloudera.org:8080/#/c/14426/4/src/kudu/subprocess/subprocess.proto@25
PS4, Line 25: import "google/protobuf/any.proto";
: import "kudu/common/wire_protocol.proto";
:
: // EchoResponsePB simply echoes the data in EchoRequestPB as a
response.
: message EchoRequestPB {
:
> Do you mind adding a small comment for these (something like in the commit
Done
--
To view, visit http://gerrit.cloudera.org:8080/14426
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If02ab0f9bc1bc73f9c1a46d96bdef6725b8f6954
Gerrit-Change-Number: 14426
Gerrit-PatchSet: 8
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 13 Jan 2020 07:03:13 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2971 p2: add basic protobuf msg
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14426 to look at the new patch set (#8). Change subject: KUDU-2971 p2: add basic protobuf msg .. KUDU-2971 p2: add basic protobuf msg This commit adds some basic protobuf messages for communicating with a subprocess. For example, EchoRequestPB and EchoResponsePB that simply echoes the request as a response. Change-Id: If02ab0f9bc1bc73f9c1a46d96bdef6725b8f6954 --- M src/kudu/subprocess/CMakeLists.txt A src/kudu/subprocess/subprocess.proto 2 files changed, 74 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/14426/8 -- To view, visit http://gerrit.cloudera.org:8080/14426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If02ab0f9bc1bc73f9c1a46d96bdef6725b8f6954 Gerrit-Change-Number: 14426 Gerrit-PatchSet: 8 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-2971: process communicates via protobuf-based protocol
Hello Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14329 to look at the new patch set (#8). Change subject: [java] KUDU-2971: process communicates via protobuf-based protocol .. [java] KUDU-2971: process communicates via protobuf-based protocol This commit adds a java tool that can communicate over a stdin/stdout pipe via protobuf-based protocol. It is useful in cases a Kudu process (e.g. master) needs to talk to third-party libraries written in Java. This tool has: 1) a single reader thread that continuously reads protobuf-based messages from the standard input and puts the messages to a FIFO blocking queue; 2) multiple writer threads that continuously retrieve the messages from the queue, process them and write the responses to the standard output. IOException is fatal and causes the program to exit, e.g. I/O errors when reading/writing to the pipe, and parsing malformed protobuf messages. To support a new protobuf message type, simply extend the 'ProtocolProcessor' interface and add the specific ProcessorMain class (similar to 'EchoProcessorMain') for the message type. Change-Id: Iaf9ad24dbc9acc681284b6433836271b5b4c7982 --- A java/kudu-subprocess/build.gradle A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/EchoProcessorMain.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/EchoProtocolProcessor.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/KuduSubprocessException.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageProcessor.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageReader.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageWriter.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ProtocolProcessor.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessCommandLineParser.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/Subprocessor.java A java/kudu-subprocess/src/main/resources/log4j2.properties A java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/MessageTestUtil.java A java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/TestMessageProcessor.java A java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/TestMessageReaderWriter.java M java/settings.gradle 16 files changed, 1,355 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/14329/8 -- To view, visit http://gerrit.cloudera.org:8080/14329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iaf9ad24dbc9acc681284b6433836271b5b4c7982 Gerrit-Change-Number: 14329 Gerrit-PatchSet: 8 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-2971: process communicates via protobuf-based protocol
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/14329 )
Change subject: [java] KUDU-2971: process communicates via protobuf-based
protocol
..
Patch Set 8:
(37 comments)
http://gerrit.cloudera.org:8080/#/c/14329/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14329/7//COMMIT_MSG@7
PS7, Line 7: KUDU-2971
> KUDU-2971
Done
http://gerrit.cloudera.org:8080/#/c/14329/7//COMMIT_MSG@9
PS7, Line 9: a std
> Should clarify here that because communication takes place over a stdin/std
Done
http://gerrit.cloudera.org:8080/#/c/14329/7//COMMIT_MSG@18
PS7, Line 18: from the queue, process them and write the responses to the
standard
: output.
:
> Curious why you didn't go with writing the messages to another blocking que
Makes sense. I am thinking to address it in a follow up patch as this one is
big enough?
http://gerrit.cloudera.org:8080/#/c/14329/6/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/BasicSubprocessor.java
File
java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/BasicSubprocessor.java:
http://gerrit.cloudera.org:8080/#/c/14329/6/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/BasicSubprocessor.java@96
PS6, Line 96:
:
:
> Ah, I was asking how this interacts, if at all, with ERROR_HANDLER, but bas
Ah, that is not only for output stream, updated. Thanks!
http://gerrit.cloudera.org:8080/#/c/14329/6/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageProcessor.java
File
java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageProcessor.java:
http://gerrit.cloudera.org:8080/#/c/14329/6/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageProcessor.java@44
PS6, Line 44:
: MessageProcessor(long maxMessageBytes,
:BufferedInputStream in,
:BufferedOutputStream out) {
: this.maxMessageBytes = maxMessageBytes;
: this.in = in;
: this.out = out;
: }
:
: /**
:* Read a protobuf message, if any, from the underlying
buffered input
:* stream. The read is not atomic (partial read can happen if
any
:* exceptions occur) and blocking (waits until the input is
available).
:*
:* @return the message in a byte array.
:* @throws EOFException if the end of the stream has been
reached
:* @throws IOException if this input stream has been closed, an
I/O
:
> I just requested constructors on the latest patch. I think one constructor
Done
http://gerrit.cloudera.org:8080/#/c/14329/6/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageProcessor.java@92
PS6, Line 92: private void doRead(byte bytes[], int size) throws EOFExcept
> IllegalStateException is a RuntimeException which is an "Unchecked" excepti
Ack
http://gerrit.cloudera.org:8080/#/c/14329/7/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageProcessor.java
File
java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageProcessor.java:
http://gerrit.cloudera.org:8080/#/c/14329/7/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageProcessor.java@49
PS7, Line 49: this.in = in;
> Can this be in the constructor instead? Mutable state like this can get tri
Done
http://gerrit.cloudera.org:8080/#/c/14329/7/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageProcessor.java@72
PS7, Line 72: String.format("message size (%d) exceeds maximum
message size (%d)",
: size, maxMessageBytes));
: }
> Why do we do this? Don't we want to block waiting for additional input if n
Makes sense, updated.
http://gerrit.cloudera.org:8080/#/c/14329/7/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageProcessor.java@83
PS7, Line 83:* input stream into the specified byte array, starting at the
offset
:* 0. If fail to r
> Nit: reformat using String.format
Done
http://gerrit.cloudera.org:8080/#/c/14329/7/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageProcessor.java@90
PS7, Line 90:* error occurs, or fail to read the
specified size
> Maybe include how much was actually read?
Done
http://gerrit.cloudera.org:8080/#/c/14329/7/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageProcessor.java@91
PS7, Line 91:*/
> Why are we converting this into a UTF-8 String? It's a serialized PB messag
Done
http://gerrit.cloudera.org:8080/#/c/14329/7/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageProcessor.java@122
PS7, Line 122:
> Why is this a String? Shouldn't it be a byte array?
Done
[kudu-CR] WIP [util] utilities to get info on cloud instance
Volodymyr Verovkin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14866 ) Change subject: WIP [util] utilities to get info on cloud instance .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14866/1/src/kudu/util/cloud/instance_metadata.cc File src/kudu/util/cloud/instance_metadata.cc: http://gerrit.cloudera.org:8080/#/c/14866/1/src/kudu/util/cloud/instance_metadata.cc@86 PS1, Line 86: "169.254.169.123" > I'm not sure I follow. Configuration file for what? This is a pre-defined Sorry, I never saw IP address hardcoded. IMHO, it should be placed in textual configuration file. -- To view, visit http://gerrit.cloudera.org:8080/14866 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I083f4803decaef5d6e7d44184bbd98b074d2578b Gerrit-Change-Number: 14866 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Mon, 13 Jan 2020 05:52:31 + Gerrit-HasComments: Yes
[kudu-CR] [tools] Support running the master and tablet server via the kudu binary
Grant Henke has posted comments on this change. (
http://gerrit.cloudera.org:8080/12517 )
Change subject: [tools] Support running the master and tablet server via the
kudu binary
..
Patch Set 23:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12517/23/src/kudu/master/master_runner.cc
File src/kudu/master/master_runner.cc:
http://gerrit.cloudera.org:8080/#/c/12517/23/src/kudu/master/master_runner.cc@85
PS23, Line 85: }
> The return statement is missing in the end of this function.
It would be an unreachable statement. We just run until terminated.
http://gerrit.cloudera.org:8080/#/c/12517/22/src/kudu/tools/tool_action.cc
File src/kudu/tools/tool_action.cc:
http://gerrit.cloudera.org:8080/#/c/12517/22/src/kudu/tools/tool_action.cc@271
PS22, Line 271: if (program_name_) {
: CHECK_NE("",
google::SetCommandLineOptionWithMode("log_filename",
: program_name_->c_str(),
: google::FlagSettingMode::SET_FLAGS_DEFAULT));
: }
> I thought since only tool_action_tserver and tool_action_master were using
I don't think we can. The log file name needs to be set before we call
InitGoogleLoggingSafe to properly affect the log file name.
http://gerrit.cloudera.org:8080/#/c/12517/23/src/kudu/tserver/tablet_server_runner.cc
File src/kudu/tserver/tablet_server_runner.cc:
http://gerrit.cloudera.org:8080/#/c/12517/23/src/kudu/tserver/tablet_server_runner.cc@81
PS23, Line 81: }
> The return statement is missing in the end of this function.
It would be an unreachable statement. We just run until terminated.
--
To view, visit http://gerrit.cloudera.org:8080/12517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3717cbac930b3506a76f7a51388c64afbcbb480e
Gerrit-Change-Number: 12517
Gerrit-PatchSet: 23
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Greg Solovyev
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Mon, 13 Jan 2020 03:34:02 +
Gerrit-HasComments: Yes
[kudu-CR] fs: move file cache to server
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/15011 )
Change subject: fs: move file cache to server
..
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/15011/2/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
http://gerrit.cloudera.org:8080/#/c/15011/2/src/kudu/fs/log_block_manager.cc@727
PS2, Line 727: if (PREDICT_TRUE(block_manager_->file_cache_)) {
:
CONTAINER_DISK_FAILURE(block_manager_->file_cache_->DeleteFile(data_file_name),
: data_failure_msg);
:
CONTAINER_DISK_FAILURE(block_manager_->file_cache_->DeleteFile(metadata_file_name),
: metadata_failure_msg);
: } else {
:
CONTAINER_DISK_FAILURE(block_manager_->env_->DeleteFile(data_file_name),
: data_failure_msg);
:
CONTAINER_DISK_FAILURE(block_manager_->env_->DeleteFile(metadata_file_name),
: metadata_failure_msg);
: }
Maybe, encapsulate this into the block manager itself? Or maybe have different
types of block manager (caching and non-caching one)?
http://gerrit.cloudera.org:8080/#/c/15011/2/src/kudu/server/server_base.h
File src/kudu/server/server_base.h:
http://gerrit.cloudera.org:8080/#/c/15011/2/src/kudu/server/server_base.h@108
PS2, Line 108: FileCache* file_cache() { return file_cache_.get(); }
nit: add const specifier for the method?
http://gerrit.cloudera.org:8080/#/c/15011/2/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/15011/2/src/kudu/server/server_base.cc@224
PS2, Line 224: TAG_FLAG(server_max_open_files, evolving);
This flag is tagged 'evolving' by default, no?
http://gerrit.cloudera.org:8080/#/c/15011/2/src/kudu/server/server_base.cc@227
PS2, Line 227: value < -1
>From the description of the flag, it seems to be that '-1' is a valid value
>meaning 'auto-calculate the proper setting', no?
http://gerrit.cloudera.org:8080/#/c/15011/2/src/kudu/server/server_base.cc@228
PS2, Line 228: Invalid max open files
Maybe, include the name of the flag in the 'flagname' parameter to make the
message more actionable?
--
To view, visit http://gerrit.cloudera.org:8080/15011
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice92c3622c954b06b773c58d51f08082010d7de3
Gerrit-Change-Number: 15011
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 13 Jan 2020 02:49:38 +
Gerrit-HasComments: Yes
[kudu-CR] fs: remove test-only constructor
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15010 ) Change subject: fs: remove test-only constructor .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I11024a2f28123bba3e3989ddd5e68f581481d5fb Gerrit-Change-Number: 15010 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 13 Jan 2020 02:01:58 + Gerrit-HasComments: No
[kudu-CR] [java] KUDU-3035: Pass last propagated timestamp in Batch
Hello Adar Dembo, Hao Hao, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15017 to review the following change. Change subject: [java] KUDU-3035: Pass last propagated timestamp in Batch .. [java] KUDU-3035: Pass last propagated timestamp in Batch Change-Id: I056d19130bf8cb6a2dbc72a02af4c4ae8d7181e9 --- M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/15017/1 -- To view, visit http://gerrit.cloudera.org:8080/15017 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I056d19130bf8cb6a2dbc72a02af4c4ae8d7181e9 Gerrit-Change-Number: 15017 Gerrit-PatchSet: 1 Gerrit-Owner: xqrzd Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao
[kudu-CR] [java] KUDU-3035: Pass last propagated timestamp in Batch
xqrzd has abandoned this change. ( http://gerrit.cloudera.org:8080/15016 ) Change subject: [java] KUDU-3035: Pass last propagated timestamp in Batch .. Abandoned -- To view, visit http://gerrit.cloudera.org:8080/15016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I1c0dc4786269b6705802711c354fe4bec27365d7 Gerrit-Change-Number: 15016 Gerrit-PatchSet: 1 Gerrit-Owner: xqrzd Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-3035: Pass last propagated timestamp in Batch
Hello Adar Dembo, Hao Hao, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15016 to review the following change. Change subject: [java] KUDU-3035: Pass last propagated timestamp in Batch .. [java] KUDU-3035: Pass last propagated timestamp in Batch Change-Id: I1c0dc4786269b6705802711c354fe4bec27365d7 --- M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/15016/1 -- To view, visit http://gerrit.cloudera.org:8080/15016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1c0dc4786269b6705802711c354fe4bec27365d7 Gerrit-Change-Number: 15016 Gerrit-PatchSet: 1 Gerrit-Owner: xqrzd Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao
[kudu-CR] [java] KUDU-3035: Pass last propagated timestamp in Batch
xqrzd has abandoned this change. ( http://gerrit.cloudera.org:8080/15015 ) Change subject: [java] KUDU-3035: Pass last propagated timestamp in Batch .. Abandoned -- To view, visit http://gerrit.cloudera.org:8080/15015 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I3cc4b3424e537236f686730ca43189cbcb1dcc47 Gerrit-Change-Number: 15015 Gerrit-PatchSet: 1 Gerrit-Owner: xqrzd Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-3035: Pass last propagated timestamp in Batch
Hello Adar Dembo, Hao Hao, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15015 to review the following change. Change subject: [java] KUDU-3035: Pass last propagated timestamp in Batch .. [java] KUDU-3035: Pass last propagated timestamp in Batch Change-Id: I3cc4b3424e537236f686730ca43189cbcb1dcc47 --- M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/15015/1 -- To view, visit http://gerrit.cloudera.org:8080/15015 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3cc4b3424e537236f686730ca43189cbcb1dcc47 Gerrit-Change-Number: 15015 Gerrit-PatchSet: 1 Gerrit-Owner: xqrzd Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao
[kudu-CR] [metrics] Fix bugs when metrics do merge
Yingchun Lai has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15014 Change subject: [metrics] Fix bugs when metrics do merge .. [metrics] Fix bugs when metrics do merge 1. METRIC_merged_entities_count_of_tablet/table/server should never retire because their value will never change. 2. Metric's modification epoch should be updated when AtomicGauge set_value() or FunctionGauge DetachToConstant(). 3. Merge should abort if they are invalid for merge, FunctionGauge included. Change-Id: I36ee6244964820e3326742c6902a578bf23041d1 --- M src/kudu/master/catalog_manager.cc M src/kudu/server/server_base.cc M src/kudu/tablet/tablet.cc M src/kudu/util/metrics.h 4 files changed, 12 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/15014/1 -- To view, visit http://gerrit.cloudera.org:8080/15014 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I36ee6244964820e3326742c6902a578bf23041d1 Gerrit-Change-Number: 15014 Gerrit-PatchSet: 1 Gerrit-Owner: Yingchun Lai <405403...@qq.com>
[kudu-CR] [tools] Add 'run cleanup' option for 'kudu perf loadgen'
Yingchun Lai has posted comments on this change. (
http://gerrit.cloudera.org:8080/14958 )
Change subject: [tools] Add 'run_cleanup' option for 'kudu perf loadgen'
..
Patch Set 3:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/14958/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14958/2//COMMIT_MSG@9
PS2, Line 9: Add 'run_cleanup' option to provide a way to cleanup test data
written to
: the table, especially an existing user table.
> I guess if cleanup isn't on by default it's OK with me.
Done
http://gerrit.cloudera.org:8080/#/c/14958/2/src/kudu/tools/tool_action_perf.cc
File src/kudu/tools/tool_action_perf.cc:
http://gerrit.cloudera.org:8080/#/c/14958/2/src/kudu/tools/tool_action_perf.cc@307
PS2, Line 307: DEFINE_bool(run_cleanup, true,
> +1 to Adar's comment: I also think this should be `false` by default at lea
Done
http://gerrit.cloudera.org:8080/#/c/14958/3/src/kudu/tools/tool_action_perf.cc
File src/kudu/tools/tool_action_perf.cc:
http://gerrit.cloudera.org:8080/#/c/14958/3/src/kudu/tools/tool_action_perf.cc@464
PS3, Line 464: value_gen.reset(new Generator(*key_gen));
> This is somewhat expensive, as it's being created per-row rather than per-t
Done
http://gerrit.cloudera.org:8080/#/c/14958/3/src/kudu/tools/tool_action_perf.cc@498
PS3, Line 498:
kMaxUnscaledDecimal32)));
> Nit: fix the indentation here?
Done
http://gerrit.cloudera.org:8080/#/c/14958/3/src/kudu/tools/tool_action_perf.cc@650
PS3, Line 650: operate rows
> Nit: maybe "rows total"?
Done
http://gerrit.cloudera.org:8080/#/c/14958/3/src/kudu/tools/tool_action_perf.cc@665
PS3, Line 665: if (!status.ok() || total_err_count != 0) {
> Indentation in this block is off.
Done
http://gerrit.cloudera.org:8080/#/c/14958/3/src/kudu/tools/tool_action_perf.cc@730
PS3, Line 730: >=
> Why this change?
In an unit test, I want to create a single tablet by setting
table_num_hash_partitions and table_num_range_partitions to 1, then we can
easily check sequential rows in the tablet.
And I think we don't need to keep this restriction.
--
To view, visit http://gerrit.cloudera.org:8080/14958
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e75adde434bac5e88151361655526b91f327b4c
Gerrit-Change-Number: 14958
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <405403...@qq.com>
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Comment-Date: Sun, 12 Jan 2020 08:24:41 +
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add 'run cleanup' option for 'kudu perf loadgen'
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14958 to look at the new patch set (#4). Change subject: [tools] Add 'run_cleanup' option for 'kudu perf loadgen' .. [tools] Add 'run_cleanup' option for 'kudu perf loadgen' Add 'run_cleanup' option to provide a way to cleanup test data written to the table, especially an existing user table. Change-Id: I1e75adde434bac5e88151361655526b91f327b4c --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_perf.cc 2 files changed, 163 insertions(+), 73 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/14958/4 -- To view, visit http://gerrit.cloudera.org:8080/14958 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1e75adde434bac5e88151361655526b91f327b4c Gerrit-Change-Number: 14958 Gerrit-PatchSet: 4 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
