[kudu-CR] [c++ client] re-acquire authn token if expired
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6648 to look at the new patch set (#7). Change subject: [c++ client] re-acquire authn token if expired .. [c++ client] re-acquire authn token if expired Updated C++ client to re-acquire authn token if server responds with FATAL_INVALID_AUTHENTICATION_TOKEN error on connection negotiation. If that happens while negotiating an connection while performing an RPC call, the connection being negotiated is closed and the client re-connects to the cluster to get new authn token. After successfully re-acquiring authn token the client retries the RPC call again. Added corresponding integration test to cover authn token re-acquisition for various scenarios. Change-Id: I418497ac59cfd4e476e9bfc6abe6b10b487712f5 --- M docs/known_issues.adoc M docs/security.adoc M src/kudu/client/batcher.cc M src/kudu/client/client-internal.cc M src/kudu/client/meta_cache.cc M src/kudu/client/scanner-internal.cc M src/kudu/client/scanner-internal.h M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/authn_token_expire-itest.cc M src/kudu/rpc/client_negotiation.cc M src/kudu/rpc/client_negotiation.h M src/kudu/rpc/connection.cc M src/kudu/rpc/connection.h M src/kudu/rpc/messenger.h M src/kudu/rpc/negotiation.cc M src/kudu/rpc/outbound_call.cc M src/kudu/rpc/reactor.cc M src/kudu/rpc/reactor.h M src/kudu/rpc/retriable_rpc.h M src/kudu/rpc/rpc.h M src/kudu/rpc/server_negotiation.cc 21 files changed, 557 insertions(+), 81 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/6648/7 -- To view, visit http://gerrit.cloudera.org:8080/6648 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I418497ac59cfd4e476e9bfc6abe6b10b487712f5 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [c++ client] re-acquire authn token if expired
Alexey Serbin has posted comments on this change. Change subject: [c++ client] re-acquire authn token if expired .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/6648/5/src/kudu/rpc/connection.cc File src/kudu/rpc/connection.cc: Line 648: rthread->CompleteConnectionNegotiation(conn_, negotiation_status_, > Done I decided to do that in a separate changelist -- I think it would be cleaner that way. -- To view, visit http://gerrit.cloudera.org:8080/6648 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I418497ac59cfd4e476e9bfc6abe6b10b487712f5 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++ client] re-acquire authn token if expired
Alexey Serbin has posted comments on this change.
Change subject: [c++ client] re-acquire authn token if expired
..
Patch Set 5:
(15 comments)
http://gerrit.cloudera.org:8080/#/c/6648/5/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:
Line 223: const Status& connect_status = ConnectToCluster(client,
deadline);
> ConnectToCluster returns an owned Status.
Done
http://gerrit.cloudera.org:8080/#/c/6648/5/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:
Line 677: table_->client()->data_->messenger_->reset_authn_token();
> Not every caller of this should be resetting the authn token. I think it's
That's a good catch. Indeed, this method is called from other places as well.
Line 706: const rpc::RpcController& controller(retrier().controller());
> this should be owned
Done
Line 708: const Status& controller_status = controller.status();
> this should be owned
Done
Line 709: if (controller_status.IsRemoteError()) {
> Would it be possible to build this logic into the RpcRetrier instead of imp
Good observation! I also was thinking about that. I think we can use
RetriableRpc instead of this ad-hoc implementation and the implementation of
KuduScanner. I think it's worth addressing that in a separate changelist.
I'll file a Jira item for that.
Line 769: if (new_status.IsNotAuthorized()) {
> I don't think this is necessarily retriable. For instance it doesn't seem
woops, that's a mistake -- it should contain all those check for RPC error code
(I need to move that code from the RemoveError clause above).
http://gerrit.cloudera.org:8080/#/c/6648/5/src/kudu/client/scanner-internal.cc
File src/kudu/client/scanner-internal.cc:
Line 134: c->data_->messenger_->reset_authn_token();
> Is there a race here between calling reset_authn_token() and another thread
There would be a race if any pair of methods like
KuduScanner::{OpenTable(),NextRows()} were called on the same scanner object
concurrently. Since the KuduScanner interface is explicitly declared
non-thread-safe in client.h, those methods are not supposed to be called
concurrently.
http://gerrit.cloudera.org:8080/#/c/6648/5/src/kudu/integration-tests/authn_token_expire-itest.cc
File src/kudu/integration-tests/authn_token_expire-itest.cc:
Line 105: std::copy(common_flags.begin(), common_flags.end(),
> This seems more difficult than directly inserting the flags into opts.extra
Done
Line 186: TEST_F(AuthnTokenExpireITest, FetchNewAuthnTokenOnInsertAndScan) {
> Is this testing something above and beyond InvalidTokenDuringWorkload? Per
You are right. This test is obsolete once the Workload test appeared. I'll
remove it. Sorry for this litter.
Line 214: // Since the authn token is verified upon connection establishment,
it's
> Should this be handled by FLAGS_rpc_reopen_outbound_connections ?
Done
Line 234: TEST_F(AuthnTokenExpireITest, RestartTabletServers) {
> I think this is a good test case, but I think FLAGS_rpc_reopen_outbound_con
Absolutely. Good catch!
Line 263: TEST_F(AuthnTokenExpireITest, RestartCluster) {
> Likewise, I think FLAGS_rpc_reopen_outbound_connections shouldn't be set du
I think you are absolutely right. These tests were written first, and then I
added that re-open feature and the workload test, setting the flag everywhere.
I'll fix this.
http://gerrit.cloudera.org:8080/#/c/6648/5/src/kudu/rpc/client_negotiation.cc
File src/kudu/rpc/client_negotiation.cc:
Line 97: const string& code_name =
ErrorStatusPB::RpcErrorCodePB_Name(error.code());
> RpcErrorCodePB_Name returns an owned string.
Done
http://gerrit.cloudera.org:8080/#/c/6648/5/src/kudu/rpc/connection.cc
File src/kudu/rpc/connection.cc:
Line 148: LOG(WARNING) << "Shutting down connection " << ToString()
> the ToString already includes "connection".
Done
Line 648: rthread->CompleteConnectionNegotiation(conn_, negotiation_status_,
> while you are here, the negotiation_status_ should probably be moved as wel
Done
--
To view, visit http://gerrit.cloudera.org:8080/6648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I418497ac59cfd4e476e9bfc6abe6b10b487712f5
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] [c++ client] re-acquire authn token if expired
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6648 to look at the new patch set (#6). Change subject: [c++ client] re-acquire authn token if expired .. [c++ client] re-acquire authn token if expired Updated C++ client to re-acquire authn token if server responds with FATAL_INVALID_AUTHENTICATION_TOKEN error on connection negotiation. If that happens while negotiating an connection while performing an RPC call, the connection being negotiated is closed and the client re-connects to the cluster to get new authn token. After successfully re-acquiring authn token the client retries the RPC call again. Added corresponding integration test to cover authn token re-acquisition for various scenarios. Change-Id: I418497ac59cfd4e476e9bfc6abe6b10b487712f5 --- M docs/known_issues.adoc M docs/security.adoc M src/kudu/client/batcher.cc M src/kudu/client/client-internal.cc M src/kudu/client/meta_cache.cc M src/kudu/client/scanner-internal.cc M src/kudu/client/scanner-internal.h M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/authn_token_expire-itest.cc M src/kudu/rpc/client_negotiation.cc M src/kudu/rpc/client_negotiation.h M src/kudu/rpc/connection.cc M src/kudu/rpc/connection.h M src/kudu/rpc/messenger.h M src/kudu/rpc/negotiation.cc M src/kudu/rpc/outbound_call.cc M src/kudu/rpc/reactor.cc M src/kudu/rpc/reactor.h M src/kudu/rpc/retriable_rpc.h M src/kudu/rpc/rpc.h M src/kudu/rpc/server_negotiation.cc 21 files changed, 557 insertions(+), 81 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/6648/6 -- To view, visit http://gerrit.cloudera.org:8080/6648 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I418497ac59cfd4e476e9bfc6abe6b10b487712f5 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error
Alexey Serbin has submitted this change and it was merged.
Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error
..
[rpc] handling ERROR_UNAVAILABLE RPC error
This patch adds handling of the newly introduced ERROR_UNAVAILABLE RPC
error code. The ERROR_UNAVAILABLE error code is a broader counterpart
of the ERROR_SERVER_TOO_BUSY.
>From the client side, both ERROR_UNAVAILABLE and ERROR_SERVER_TOO_BUSY
codes mean it's worth retrying the call at a later time. To reflect
that, the internal codes {RetriableRpcStatus,ScanRpcStatus}::SERVER_BUSY
are replaced with more generic SERVICE_UNAVAILABLE.
Added an integration test to cover the behavior of the server components
and the Kudu C++ client when client sends authn token signed by a TSK
unknown to master and tablet servers.
Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959
Reviewed-on: http://gerrit.cloudera.org:8080/6640
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert
---
M src/kudu/client/batcher.cc
M src/kudu/client/client-internal.cc
M src/kudu/client/client.h
M src/kudu/client/scanner-internal.cc
M src/kudu/client/scanner-internal.h
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/security-unknown-tsk-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/exactly_once_rpc-test.cc
M src/kudu/rpc/outbound_call.h
M src/kudu/rpc/retriable_rpc.h
M src/kudu/rpc/rpc.cc
M src/kudu/rpc/rpc.h
M src/kudu/rpc/rpc_context.h
16 files changed, 529 insertions(+), 35 deletions(-)
Approvals:
Dan Burkert: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/6640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error
Alexey Serbin has posted comments on this change. Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error .. Patch Set 12: > > hmm ok. You may want to do a disttest loop to make sure it's not > > flaky, if you haven't already. > > I did that for the release configuration already, mainly because > that was the most flaky config for the xxxDuringWorkload test. > Currently it passes 1024 out of 1024 runs. > > Frankly, I didn't expect that the other (simpler) test would be > flaky. > > I'll do more testing for the SAN configs, though. All right, 1024 out of 1024 runs for both ASAN and TSAN passed. -- To view, visit http://gerrit.cloudera.org:8080/6640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959 Gerrit-PatchSet: 12 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error
Alexey Serbin has posted comments on this change. Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error .. Patch Set 12: > hmm ok. You may want to do a disttest loop to make sure it's not > flaky, if you haven't already. I did that for the release configuration already, mainly because that was the most flaky config for the xxxDuringWorkload test. Currently it passes 1024 out of 1024 runs. Frankly, I didn't expect that the other (simpler) test would be flaky. I'll do more testing for the SAN configs, though. -- To view, visit http://gerrit.cloudera.org:8080/6640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959 Gerrit-PatchSet: 12 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error
Alexey Serbin has posted comments on this change. Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/6640/12/src/kudu/integration-tests/security-unknown-tsk-itest.cc File src/kudu/integration-tests/security-unknown-tsk-itest.cc: Line 295: ASSERT_STR_CONTAINS(err_msg, "Service unavailable"); > As I understand, the error was at the client side, but in different phase: It seems the call has been timed out prior to sending it: it was on the outbound queue and the RPC detected the user timeout has happened even before sending it to the server. -- To view, visit http://gerrit.cloudera.org:8080/6640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959 Gerrit-PatchSet: 12 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error
Dan Burkert has posted comments on this change. Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error .. Patch Set 13: Code-Review+2 hmm ok. You may want to do a disttest loop to make sure it's not flaky, if you haven't already. -- To view, visit http://gerrit.cloudera.org:8080/6640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959 Gerrit-PatchSet: 13 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] log block manager: detect and repair unpunched holes
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6717 to look at the new patch set (#6). Change subject: log block manager: detect and repair unpunched holes .. log block manager: detect and repair unpunched holes This patch adds detection and repair of "unpunched holes" to the LBM. These are deleted blocks whose space in the container data files is still "live", either because hole punching failed, or because the server crashed before the holes could be punched. The newly added container accounting is used as a heuristic to decide when to repunch holes: if a container's data size exceeds the size we think it should have (after alignment), we'll repunch all holes and truncate any preallocated space off the end. The heuristic uses some (configurable) slop to work around various filesystem accounting issues. An alternative is to use the container's extent map to figure this out (and to provide greater precision on where to punch), but testing on el6.6 showed that calling the FS_IOC_FIEMAP ioctl() on every full container increased LBM startup time by about 50%. That's bad enough that we shouldn't do it willy nilly. It could be gated on the above heuristic and used to drive more precise hole repunching, but given the complexity involved and given the amount of tilting I've done at this particular windmill, it'll remain a problem for another day. Testing is done in three ways: - A new unit test that exercises new LBMCorruptor functionality. - Inclusion in block_manager-stress-test via the change to LBMCorruptor::InjectRandomNonFatalInconsistency(). - Inclusion in BlockManagerTest.TestMetadataOkayDespiteFailedWrites via the generalization of the env_inject_io_error gflag. Change-Id: I016ea401380f4c8c7b1fd907ff67cb595f377dd1 --- M src/kudu/fs/block_manager-stress-test.cc M src/kudu/fs/block_manager-test.cc M src/kudu/fs/log_block_manager-test-util.cc M src/kudu/fs/log_block_manager-test-util.h M src/kudu/fs/log_block_manager-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/fs/log_block_manager.h M src/kudu/util/env_posix.cc 8 files changed, 292 insertions(+), 82 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/6717/6 -- To view, visit http://gerrit.cloudera.org:8080/6717 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I016ea401380f4c8c7b1fd907ff67cb595f377dd1 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] log block manager: detect and repair unpunched holes
Adar Dembo has posted comments on this change.
Change subject: log block manager: detect and repair unpunched holes
..
Patch Set 5:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/6717/5/src/kudu/fs/log_block_manager-test-util.cc
File src/kudu/fs/log_block_manager-test-util.cc:
PS5, Line 156: initial_data_size
> does it make sense to add assertions on size?
Not sure what you mean; do you mean we should assert that initial_data_size is
larger or equal to a certain value? What value would that add?
PS5, Line 460: int64_t* old_data_file_size
> maybe set a nullptr default arg so that you don't have to pass it if you do
All three callers make use of old_data_file_size though.
http://gerrit.cloudera.org:8080/#/c/6717/5/src/kudu/fs/log_block_manager-test.cc
File src/kudu/fs/log_block_manager-test.cc:
Line 50:
> extra line, likely also clean the other one below
Done
http://gerrit.cloudera.org:8080/#/c/6717/5/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
PS5, Line 79: 0.10
> this is what's gating the heavy startup penalty, right? How easy is it to r
Will change the name.
If the penalty you're alluding to was the 50% from the extent map approach,
that's totally gone.
Without any slop (meaning, any full container with even one unaccounted-for
byte will repunch its holes and truncate), I observed a 6% penalty on el6.6
where my ~10,000 containers of size 16-32k had about 2k extra space in them.
We still don't really know why these discrepancies exist. Todd's theory is that
older versions of ext4 include blocks occupied by interior nodes in the extent
tree in their accounting, but I wasn't able to find a slam dunk explanation in
a bug report or similar resource. And I don't have time to do a thorough
investigation in different environments. So, this knob will have to suffice.
Note: I didn't see any space discrepancies at all in Ubuntu 16; that is, at
some point, ext4 fixed its space accounting issues. Just not on el6.
PS5, Line 80: Additional fraction of a log container's calculated size that "
: "must be consumed on disk before the container
considered to be "
: "inconsistent and subject to excess space cleanup.
> explain when/how the cleanup happens (at startup or with a tool, right?)
Done
PS5, Line 1833: actual_size
> you're basically saying in the comments above that this number is unstrustw
Yes, it's untrustworthy. I'll change the name.
PS5, Line 1834: s = env_->GetFileSizeOnDisk(data_filename, &actual_size);
: if (!s.ok()) {
: *result_status = s.CloneAndPrepend(Substitute(
: "Could not get on-disk file size of container $0",
container->ToString()));
: return;
: }
> use RETURN_NOT_OK_PREPEND
Can't; this function is run asynchronously out of a thread pool so it doesn't
return its status.
PS5, Line 1840: measured_size
> _this_ is the actual size, right?
This is the size according to our own accounting. It's not the size according
to the filesystem.
PS5, Line 2054: TODO(adar): can be more efficient (less fs work and more space
reclamation
: // in case of misaligned blocks) via hole coalescing first,
but this is easy.
> file a jira? seems like we might have to revisit this if it turns out that
I'm going to punt. I think the TODO() is enough for now, and my observations
show that even repunching every hole in the container isn't that bad (see
above).
http://gerrit.cloudera.org:8080/#/c/6717/5/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
PS5, Line 116: certain
> which ones? can it be any operation?
The ones that use it. :)
It's a little ridiculous to maintain an exhaustive list that must be kept
in-sync, especially since this is a testing-only gflag. It'd also be ridiculous
to say "search for all references to this flag in order to find which
operations are affected."
--
To view, visit http://gerrit.cloudera.org:8080/6717
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I016ea401380f4c8c7b1fd907ff67cb595f377dd1
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error
Alexey Serbin has posted comments on this change.
Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error
..
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6640/12/src/kudu/integration-tests/security-unknown-tsk-itest.cc
File src/kudu/integration-tests/security-unknown-tsk-itest.cc:
Line 295: ASSERT_STR_CONTAINS(err_msg, "Service unavailable");
> Hmm, so the error you hit was that the RPC got timed out by the server inst
As I understand, the error was at the client side, but in different phase:
W0428 23:52:22.339293 461 meta_cache.cc:765] Timed out: GetTableLocations {
table: 'security-unkno
wn-tsk-itest', partition-key: (), attempt: 35 } failed: timed out after
deadline expired: Get
TableLocations RPC to 127.0.0.1:44953 timed out after 0.001s (ON_OUTBOUND_QUEUE)
--
To view, visit http://gerrit.cloudera.org:8080/6640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error
Dan Burkert has posted comments on this change. Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error .. Patch Set 13: (1 comment) http://gerrit.cloudera.org:8080/#/c/6640/12/src/kudu/integration-tests/security-unknown-tsk-itest.cc File src/kudu/integration-tests/security-unknown-tsk-itest.cc: Line 295: } Hmm, so the error you hit was that the RPC got timed out by the server instead of being timed out by the client, thus the error string didn't include the latest failure about mismatched key versions? -- To view, visit http://gerrit.cloudera.org:8080/6640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959 Gerrit-PatchSet: 13 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] subprocess: add KillAndWait() and allow customization of exit signal
Adar Dembo has submitted this change and it was merged. Change subject: subprocess: add KillAndWait() and allow customization of exit signal .. subprocess: add KillAndWait() and allow customization of exit signal This patch does two things: - It introduces a KillAndWait() method that terminates and fully reaps a process. - It allows one to customize the exit signal delivered to a subprocess when it goes out of scope. The default signal is SIGKILL which doesn't let subprocesses clean up after themselves. Change-Id: Iaf31bd4ea6de2917521a9852714fb33cfbec1f61 Reviewed-on: http://gerrit.cloudera.org:8080/6741 Tested-by: Adar Dembo Reviewed-by: David Ribeiro Alves --- M src/kudu/util/subprocess-test.cc M src/kudu/util/subprocess.cc M src/kudu/util/subprocess.h 3 files changed, 113 insertions(+), 8 deletions(-) Approvals: David Ribeiro Alves: Looks good to me, approved Adar Dembo: Verified -- To view, visit http://gerrit.cloudera.org:8080/6741 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iaf31bd4ea6de2917521a9852714fb33cfbec1f61 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] env: add RWFile::GetExtentMap for analyzing file extents
Adar Dembo has submitted this change and it was merged. Change subject: env: add RWFile::GetExtentMap for analyzing file extents .. env: add RWFile::GetExtentMap for analyzing file extents This patch introduces a method to get the extent metadata of a file provided it resides on an extent-based filesystem (such as ext4 or xfs). Each extent is an offset and length into the file, and represents a chunk of filesystem that has been allocated for the file. Gaps between extents are expected to be unallocated and may represent punched out holes. On Linux, the extent listing is retrieved via repeated calls to the FS_IOC_FIEMAP ioctl, though only some of the information returned is actually used. Originally I intended to use this in the log block manager for finding extra allocated space in container files. I ended up using a coarser heuristic, but I'd like to merge this anyway as I think it'll be useful in future as a more precise way of repairing extra allocated space. Change-Id: I35bd1bdb9e1a839af2ab95ea73b79217c1f4a2b3 Reviewed-on: http://gerrit.cloudera.org:8080/6583 Tested-by: Kudu Jenkins Reviewed-by: David Ribeiro Alves --- M src/kudu/util/env-test.cc M src/kudu/util/env.h M src/kudu/util/env_posix.cc M src/kudu/util/file_cache.cc 4 files changed, 145 insertions(+), 3 deletions(-) Approvals: David Ribeiro Alves: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/6583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I35bd1bdb9e1a839af2ab95ea73b79217c1f4a2b3 Gerrit-PatchSet: 12 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] subprocess: add KillAndWait() and allow customization of exit signal
David Ribeiro Alves has posted comments on this change. Change subject: subprocess: add KillAndWait() and allow customization of exit signal .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6741 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaf31bd4ea6de2917521a9852714fb33cfbec1f61 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] env: add RWFile::GetExtentMap for analyzing file extents
David Ribeiro Alves has posted comments on this change. Change subject: env: add RWFile::GetExtentMap for analyzing file extents .. Patch Set 11: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35bd1bdb9e1a839af2ab95ea73b79217c1f4a2b3 Gerrit-PatchSet: 11 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] log block manager: detect and repair unpunched holes
David Ribeiro Alves has posted comments on this change.
Change subject: log block manager: detect and repair unpunched holes
..
Patch Set 5:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/6717/5/src/kudu/fs/log_block_manager-test-util.cc
File src/kudu/fs/log_block_manager-test-util.cc:
PS5, Line 156: initial_data_size
does it make sense to add assertions on size?
PS5, Line 460: int64_t* old_data_file_size
maybe set a nullptr default arg so that you don't have to pass it if you don't
care (twice in this file iirc)
http://gerrit.cloudera.org:8080/#/c/6717/5/src/kudu/fs/log_block_manager-test.cc
File src/kudu/fs/log_block_manager-test.cc:
Line 50:
extra line, likely also clean the other one below
http://gerrit.cloudera.org:8080/#/c/6717/5/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
PS5, Line 79: 0.10
this is what's gating the heavy startup penalty, right? How easy is it to reach
10% wasted space? afraid that if it's too easy then we might as well not gate
it at all
PS5, Line 79: log_container_extra_space_heuristic_slop_fraction
this flag name is not very informative. maybe:
log_container_wasted_space_before_cleanup_fraction or somesuch
PS5, Line 80: Additional fraction of a log container's calculated size that "
: "must be consumed on disk before the container
considered to be "
: "inconsistent and subject to excess space cleanup.
explain when/how the cleanup happens (at startup or with a tool, right?)
PS5, Line 1833: actual_size
you're basically saying in the comments above that this number is
unstrustworthy, or did I get that wrong?
maybe "reported_size" or "size_reported_by_os" would be a better name
PS5, Line 1834: s = env_->GetFileSizeOnDisk(data_filename, &actual_size);
: if (!s.ok()) {
: *result_status = s.CloneAndPrepend(Substitute(
: "Could not get on-disk file size of container $0",
container->ToString()));
: return;
: }
use RETURN_NOT_OK_PREPEND
PS5, Line 1840: measured_size
_this_ is the actual size, right?
PS5, Line 2054: TODO(adar): can be more efficient (less fs work and more space
reclamation
: // in case of misaligned blocks) via hole coalescing first,
but this is easy.
file a jira? seems like we might have to revisit this if it turns out that hole
puching is easily triggered and too heavy
http://gerrit.cloudera.org:8080/#/c/6717/5/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
PS5, Line 116: certain
which ones? can it be any operation?
--
To view, visit http://gerrit.cloudera.org:8080/6717
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I016ea401380f4c8c7b1fd907ff67cb595f377dd1
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error
Alexey Serbin has posted comments on this change. Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error .. Patch Set 13: > woops, you're right. Meant to leave that comment on the reacquire > patch. Thanks for confirmation. I'll do that in that patch then. -- To view, visit http://gerrit.cloudera.org:8080/6640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959 Gerrit-PatchSet: 13 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1981 Kudu should run at hosts len(FQDN) > 64
Alexey Serbin has submitted this change and it was merged. Change subject: KUDU-1981 Kudu should run at hosts len(FQDN) > 64 .. KUDU-1981 Kudu should run at hosts len(FQDN) > 64 This is a fix for KUDU-1981: with security enabled, Kudu servers cannot start at machines with len(FQDN) > 64. Prior to this fix, the host FQDN was put into the CSR's CN (common name) field while generating self-signed certificate for server RPC messenger. Per RFC5280, the CN field cannot contain strings longer than 64 characters long, and it seems OpenSSL enforces that limit as required. The idea is to put FQDNs into the SAN X509v3 extension field as 'DNS' fields. That makes it possible to have names in the SAN which are even longer than 255 characters. This patch returns back a part of the SAN-related functionality which had been implemented initially in cert_management.cc and then removed since it was not used back then. This patch also adds a couple of unit tests to cover the new functionality and to make sure it's possible to set CN field of CSR to 64-chars length value and have corresponding X509 certificate generated with no issues. Change-Id: Ie142e76e9b2dcef3e07dd33d82b6758c746ced19 Reviewed-on: http://gerrit.cloudera.org:8080/6734 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert --- M src/kudu/integration-tests/master_cert_authority-itest.cc M src/kudu/security/ca/cert_management-test.cc M src/kudu/security/ca/cert_management.cc M src/kudu/security/ca/cert_management.h M src/kudu/security/cert-test.cc M src/kudu/security/cert.cc M src/kudu/security/cert.h M src/kudu/security/test/test_certs.cc M src/kudu/security/test/test_certs.h M src/kudu/security/tls_context.cc 10 files changed, 232 insertions(+), 74 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/6734 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie142e76e9b2dcef3e07dd33d82b6758c746ced19 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1981 Kudu should run at hosts len(FQDN) > 64
Dan Burkert has posted comments on this change. Change subject: KUDU-1981 Kudu should run at hosts len(FQDN) > 64 .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6734 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie142e76e9b2dcef3e07dd33d82b6758c746ced19 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6640
to look at the new patch set (#13).
Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error
..
[rpc] handling ERROR_UNAVAILABLE RPC error
This patch adds handling of the newly introduced ERROR_UNAVAILABLE RPC
error code. The ERROR_UNAVAILABLE error code is a broader counterpart
of the ERROR_SERVER_TOO_BUSY.
>From the client side, both ERROR_UNAVAILABLE and ERROR_SERVER_TOO_BUSY
codes mean it's worth retrying the call at a later time. To reflect
that, the internal codes {RetriableRpcStatus,ScanRpcStatus}::SERVER_BUSY
are replaced with more generic SERVICE_UNAVAILABLE.
Added an integration test to cover the behavior of the server components
and the Kudu C++ client when client sends authn token signed by a TSK
unknown to master and tablet servers.
Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959
---
M src/kudu/client/batcher.cc
M src/kudu/client/client-internal.cc
M src/kudu/client/client.h
M src/kudu/client/scanner-internal.cc
M src/kudu/client/scanner-internal.h
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/security-unknown-tsk-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/exactly_once_rpc-test.cc
M src/kudu/rpc/outbound_call.h
M src/kudu/rpc/retriable_rpc.h
M src/kudu/rpc/rpc.cc
M src/kudu/rpc/rpc.h
M src/kudu/rpc/rpc_context.h
16 files changed, 529 insertions(+), 35 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/6640/13
--
To view, visit http://gerrit.cloudera.org:8080/6640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error
Dan Burkert has posted comments on this change. Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error .. Patch Set 12: woops, you're right. Meant to leave that comment on the reacquire patch. -- To view, visit http://gerrit.cloudera.org:8080/6640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959 Gerrit-PatchSet: 12 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] log block manager: detect and repair unpunched holes
David Ribeiro Alves has posted comments on this change. Change subject: log block manager: detect and repair unpunched holes .. Patch Set 5: Adding Andrew to the review since this touches stuff he might also be touching -- To view, visit http://gerrit.cloudera.org:8080/6717 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I016ea401380f4c8c7b1fd907ff67cb595f377dd1 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] log block manager: more container accounting
Adar Dembo has submitted this change and it was merged. Change subject: log block manager: more container accounting .. log block manager: more container accounting This patch moves some stats accounting from the FsReport into containers. The accounting changes: - total_bytes_written_ is now next_block_offset_, which I think describes its purpose more accurately. - total_bytes_ and total_blocks_ reflect the total number of bytes/blocks ever written into the container. - live_bytes_, live_bytes_aligned, and live_blocks_ only reflect the effects of live blocks. live_bytes_aligned_ and live_blocks_ will be used in future work (the former as part of a "does this container have extra space?" heuristic, the latter for determining when to GC a container). UpdateBytesWrittenAndTotalBlocks() is generalized into BlockCreated() to make room for a symmetric counterpart, called on the block deletion path. Note: BlockDeleted() has different synchronization requirements! Change-Id: I1907db2a74bcc074ecf882f8a758c3989431e267 Reviewed-on: http://gerrit.cloudera.org:8080/6716 Tested-by: Kudu Jenkins Reviewed-by: David Ribeiro Alves --- M src/kudu/fs/log_block_manager.cc 1 file changed, 90 insertions(+), 53 deletions(-) Approvals: David Ribeiro Alves: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/6716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I1907db2a74bcc074ecf882f8a758c3989431e267 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] log block manager: more container accounting
David Ribeiro Alves has posted comments on this change. Change subject: log block manager: more container accounting .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1907db2a74bcc074ecf882f8a758c3989431e267 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error
Alexey Serbin has posted comments on this change. Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error .. Patch Set 12: > Also, could you update the known limitations section here: > https://github.com/apache/kudu/blob/master/docs/security.adoc#known-limitations If that about the long-lived tickets, then that would be more relevant in the context of https://gerrit.cloudera.org/#/c/6648/ It seems we didn't mention the possibility of issuing an authn token which might be unknown to the rest of the system (which this patch addresses). -- To view, visit http://gerrit.cloudera.org:8080/6640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959 Gerrit-PatchSet: 12 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1978: avoid corruption when deleting misaligned blocks
Adar Dembo has submitted this change and it was merged. Change subject: KUDU-1978: avoid corruption when deleting misaligned blocks .. KUDU-1978: avoid corruption when deleting misaligned blocks The gist of the problem: when punching out blocks, we don't actually know where they end. The existing code assumes that proper alignment is always maintained, but when faced with a repeated sequence of blocks misaligned on both start and end offsets, this may not be true. It is conceivable (though not certain) that KUDU-1793 can cause such sequences. To fix, we could maintain per-block end offsets, but that'd be expensive for such a rare case (remember: we have millions of blocks). Instead, we'll look for blocks misaligned on their start offsets and skip the align up step when punching them out. This means we won't reclaim all of their space, but it's better than corrupting data belonging to the next block. A black box reproduction of KUDU-1793 is virtually impossible, so here's a white box implementation instead, via augmentation of the LBMCorruptor's misaligned block generator. Without the fix (though with the removal of the DCHECK_EQ), the new test fails 100% of the time. With the fix, it hasn't failed in 1000 runs. Change-Id: I6ed6e349ab10d8d04722cd7ef99e7a06554f51f1 Reviewed-on: http://gerrit.cloudera.org:8080/6715 Tested-by: Kudu Jenkins Reviewed-by: David Ribeiro Alves --- M src/kudu/fs/log_block_manager-test-util.cc M src/kudu/fs/log_block_manager-test-util.h M src/kudu/fs/log_block_manager-test.cc M src/kudu/fs/log_block_manager.cc 4 files changed, 180 insertions(+), 42 deletions(-) Approvals: David Ribeiro Alves: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/6715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I6ed6e349ab10d8d04722cd7ef99e7a06554f51f1 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1978: avoid corruption when deleting misaligned blocks
David Ribeiro Alves has posted comments on this change. Change subject: KUDU-1978: avoid corruption when deleting misaligned blocks .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6ed6e349ab10d8d04722cd7ef99e7a06554f51f1 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6640
to look at the new patch set (#12).
Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error
..
[rpc] handling ERROR_UNAVAILABLE RPC error
This patch adds handling of the newly introduced ERROR_UNAVAILABLE RPC
error code. The ERROR_UNAVAILABLE error code is a broader counterpart
of the ERROR_SERVER_TOO_BUSY.
>From the client side, both ERROR_UNAVAILABLE and ERROR_SERVER_TOO_BUSY
codes mean it's worth retrying the call at a later time. To reflect
that, the internal codes {RetriableRpcStatus,ScanRpcStatus}::SERVER_BUSY
are replaced with more generic SERVICE_UNAVAILABLE.
Added an integration test to cover the behavior of the server components
and the Kudu C++ client when client sends authn token signed by a TSK
unknown to master and tablet servers.
Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959
---
M src/kudu/client/batcher.cc
M src/kudu/client/client-internal.cc
M src/kudu/client/client.h
M src/kudu/client/scanner-internal.cc
M src/kudu/client/scanner-internal.h
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/security-unknown-tsk-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/exactly_once_rpc-test.cc
M src/kudu/rpc/outbound_call.h
M src/kudu/rpc/retriable_rpc.h
M src/kudu/rpc/rpc.cc
M src/kudu/rpc/rpc.h
M src/kudu/rpc/rpc_context.h
16 files changed, 535 insertions(+), 35 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/6640/12
--
To view, visit http://gerrit.cloudera.org:8080/6640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1970: node density integration test
Hello Dan Burkert, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6662 to look at the new patch set (#6). Change subject: KUDU-1970: node density integration test .. KUDU-1970: node density integration test This patch introduces a new itest that simulates a storage-dense Kudu deployment. The idea is simple: rather than actually generating and storing lots of data (which is both time intensive and developer unfriendly), let's run a workload that produces a lot of metadata with a minimal amount of data. This is cheaper, and the metadata can proxy for data in areas we care about (such as start up time, thread count, memory usage, etc.). The test itself isn't that interesting; most of the challenge was in running it repeatedly to determine which flag values yielded the most metadata. In a run of the test on a 48 core el6.6 machine (max_blocks_per_container=8, num_tablets=1000, num_seconds=240), I produced ~110K blocks across ~21k LBM containers, which yielded a subsequent ~100s LBM startup time. I made the following modifications elsewhere to make this work: - TestWorkload now supports arbitrary schemas. - EMC-based tests can configure the amount of time they wait on each daemon process to start as the server info file isn't dumped until after FS startup is complete (maybe that should be changed?) - The benchmarks.sh script runs the test with some customized parameters. I also snuck in changes to remove an unused variable from random.h and to switch TestWorkload from kudu::Thread to std::thread. Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d --- M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/dense_node-itest.cc M src/kudu/integration-tests/external_mini_cluster-itest-base.cc M src/kudu/integration-tests/external_mini_cluster-itest-base.h M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/scripts/benchmarks.sh M src/kudu/tools/data_gen_util.cc M src/kudu/tools/data_gen_util.h M src/kudu/tools/kudu-ts-cli-test.cc M src/kudu/util/random.h 13 files changed, 337 insertions(+), 47 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/6662/6 -- To view, visit http://gerrit.cloudera.org:8080/6662 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1970: node density integration test
Adar Dembo has posted comments on this change. Change subject: KUDU-1970: node density integration test .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/6662/4//COMMIT_MSG Commit Message: Line 11: lots of data (which is both time intensive and developer unfriendly), let's > This sentence makes it sounds like you are somehow writing only the metadat Will clarify. -- To view, visit http://gerrit.cloudera.org:8080/6662 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] subprocess: add KillAndWait() and allow customization of exit signal
Adar Dembo has posted comments on this change. Change subject: subprocess: add KillAndWait() and allow customization of exit signal .. Patch Set 3: Verified+1 Unrelated failure (minidump file left behind) in a Java test, I think. -- To view, visit http://gerrit.cloudera.org:8080/6741 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaf31bd4ea6de2917521a9852714fb33cfbec1f61 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1970: node density integration test
Dan Burkert has posted comments on this change. Change subject: KUDU-1970: node density integration test .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/6662 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1970: node density integration test
Dan Burkert has posted comments on this change. Change subject: KUDU-1970: node density integration test .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/6662/4//COMMIT_MSG Commit Message: Line 11: lots of data (which is both time intensive and developer unfriendly), let's This sentence makes it sounds like you are somehow writing only the metadata. -- To view, visit http://gerrit.cloudera.org:8080/6662 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1970: node density integration test
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6662 to look at the new patch set (#5). Change subject: KUDU-1970: node density integration test .. KUDU-1970: node density integration test This patch introduces a new itest that models a storage-dense Kudu deployment. The idea is simple: rather than actually generating and storing lots of data (which is both time intensive and developer unfriendly), let's produce a lot of metadata instead, as that's cheaper and can proxy for data. The test itself isn't that interesting; most of the challenge was in running it repeatedly to determine which flag values yielded the most metadata. In a run of the test on a 48 core el6.6 machine (max_blocks_per_container=8, num_tablets=1000, num_seconds=240), I produced ~110K blocks across ~21k LBM containers, which yielded a subsequent ~100s LBM startup time. I made the following modifications elsewhere to make this work: - TestWorkload now supports arbitrary schemas. - EMC-based tests can configure the amount of time they wait on each daemon process to start as the server info file isn't dumped until after FS startup is complete (maybe that should be changed?) - The benchmarks.sh script runs the test with some customized parameters. I also snuck in changes to remove an unused variable from random.h and to switch TestWorkload from kudu::Thread to std::thread. Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d --- M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/dense_node-itest.cc M src/kudu/integration-tests/external_mini_cluster-itest-base.cc M src/kudu/integration-tests/external_mini_cluster-itest-base.h M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/scripts/benchmarks.sh M src/kudu/tools/data_gen_util.cc M src/kudu/tools/data_gen_util.h M src/kudu/tools/kudu-ts-cli-test.cc M src/kudu/util/random.h 13 files changed, 337 insertions(+), 47 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/6662/5 -- To view, visit http://gerrit.cloudera.org:8080/6662 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tablet copy-itest: Reduce flakiness
Adar Dembo has posted comments on this change. Change subject: tablet_copy-itest: Reduce flakiness .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idd10f2fefb67634031f5c08e2adddc695193afb7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] external mini cluster: spawn perf record for each daemon during Start()
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6742 to look at the new patch set (#3). Change subject: external mini cluster: spawn perf record for each daemon during Start() .. external mini cluster: spawn perf record for each daemon during Start() I want this for the dense node test, but it's not really possible to do from outside ExternalMiniCluster without missing out on time spent in Start(), which I was interested in measuring. So here's a generic approach that can be used by any itest. I wasn't sure whether this should be configured via EMC option or gflag. I ended up with the latter because it's not really something a test needs programmatic access to; it's just something the dev running the test might want to enable manually. I also changed the existing perf calls in full_stack-insert-scan-test to use the new subprocess custom destroy signal. While there I fixed the handling of "--call-graph"; passing it without "fp" is a syntax error on both el6.6 and Ubuntu 16.04. Change-Id: I92079f616648788b461d550057b8e23bc9174b71 --- M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h M src/kudu/integration-tests/full_stack-insert-scan-test.cc 3 files changed, 61 insertions(+), 32 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/6742/3 -- To view, visit http://gerrit.cloudera.org:8080/6742 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I92079f616648788b461d550057b8e23bc9174b71 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] log block manager: detect and repair unpunched holes
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6717 to look at the new patch set (#5). Change subject: log block manager: detect and repair unpunched holes .. log block manager: detect and repair unpunched holes This patch adds detection and repair of "unpunched holes" to the LBM. These are deleted blocks whose space in the container data files is still "live", either because hole punching failed, or because the server crashed before the holes could be punched. The newly added container accounting is used as a heuristic to decide when to repunch holes: if a container's data size exceeds the size we think it should have (after alignment), we'll repunch all holes and truncate any preallocated space off the end. The heuristic uses some (configurable) slop to work around various filesystem accounting issues. An alternative is to use the container's extent map to figure this out (and to provide greater precision on where to punch), but testing on el6.6 showed that calling the FS_IOC_FIEMAP ioctl() on every full container increased LBM startup time by about 50%. That's bad enough that we shouldn't do it willy nilly. It could be gated on the above heuristic and used to drive more precise hole repunching, but given the complexity involved and given the amount of tilting I've done at this particular windmill, it'll remain a problem for another day. Testing is done in three ways: - A new unit test that exercises new LBMCorruptor functionality. - Inclusion in block_manager-stress-test via the change to LBMCorruptor::InjectRandomNonFatalInconsistency(). - Inclusion in BlockManagerTest.TestMetadataOkayDespiteFailedWrites via the generalization of the env_inject_io_error gflag. Change-Id: I016ea401380f4c8c7b1fd907ff67cb595f377dd1 --- M src/kudu/fs/block_manager-stress-test.cc M src/kudu/fs/block_manager-test.cc M src/kudu/fs/log_block_manager-test-util.cc M src/kudu/fs/log_block_manager-test-util.h M src/kudu/fs/log_block_manager-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/fs/log_block_manager.h M src/kudu/util/env_posix.cc 8 files changed, 291 insertions(+), 76 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/6717/5 -- To view, visit http://gerrit.cloudera.org:8080/6717 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I016ea401380f4c8c7b1fd907ff67cb595f377dd1 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tablet copy-itest: Reduce flakiness
Mike Percy has uploaded a new change for review. http://gerrit.cloudera.org:8080/6764 Change subject: tablet_copy-itest: Reduce flakiness .. tablet_copy-itest: Reduce flakiness This patch reduces the flakiness under heavy CPU load of TabletCopyITest.TestDisableTabletCopy_NoTightLoopWhenTabletDeleted from 37/100 failures [1] to 0/100 failures [2] on dist-test by simply tuning up the # of log messages per second we tolerate from 30 to 60. This patch also fixes a minor bug where we attempt to send a tablet copy request with invalid parameters even if the PrepareTabletCopyRequest() method returns an error, indicating that it wasn't able to fill in the request. This resulted in warning messages in the logs that looked like the following: [libprotobuf ERROR /home/mpercy/src/kudu/thirdparty/src/protobuf-2.6.1/src/google/protobuf/message_lite.cc:123] Can't parse message of type "kudu.consensus.StartTabletCopyRequestPB" because it is missing required fields: tablet_id, copy_peer_uuid, copy_peer_addr W0428 19:46:21.691408 10560 service_if.cc:62] invalid parameter for call kudu.consensus.ConsensusService.StartTabletCopy: missing fields: tablet_id, copy_peer_uuid, copy_peer_addr [1] http://dist-test.cloudera.org/job?job_id=mpercy.1493408511.5800 [2] http://dist-test.cloudera.org/job?job_id=mpercy.1493408241.3403 Change-Id: Idd10f2fefb67634031f5c08e2adddc695193afb7 --- M src/kudu/consensus/consensus_peers.cc M src/kudu/consensus/consensus_queue.cc M src/kudu/integration-tests/tablet_copy-itest.cc 3 files changed, 16 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/6764/1 -- To view, visit http://gerrit.cloudera.org:8080/6764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Idd10f2fefb67634031f5c08e2adddc695193afb7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy
[kudu-CR] KUDU-1970: node density integration test
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6662 to look at the new patch set (#4). Change subject: KUDU-1970: node density integration test .. KUDU-1970: node density integration test This patch introduces a new itest that models a storage-dense Kudu deployment. The idea is simple: rather than actually generating and storing lots of data (which is both time intensive and developer unfriendly), let's produce a lot of metadata instead, as that's cheaper and can proxy for data. The test itself isn't that interesting; most of the challenge was in running it repeatedly to determine which flag values yielded the most metadata. In a run of the test on a 48 core el6.6 machine (max_blocks_per_container=8, num_tablets=1000, num_seconds=240), I produced ~110K blocks across ~21k LBM containers, which yielded a subsequent ~100s LBM startup time. I made the following modifications elsewhere to make this work: - TestWorkload now supports arbitrary schemas. - EMC-based tests can configure the amount of time they wait on each daemon process to start as the server info file isn't dumped until after FS startup is complete (maybe that should be changed?) - The benchmarks.sh script runs the test with some customized parameters. I also snuck in changes to remove an unused variable from random.h and to switch TestWorkload from kudu::Thread to std::thread. Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d --- M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/dense_node-itest.cc M src/kudu/integration-tests/external_mini_cluster-itest-base.cc M src/kudu/integration-tests/external_mini_cluster-itest-base.h M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/scripts/benchmarks.sh M src/kudu/tools/data_gen_util.cc M src/kudu/tools/data_gen_util.h M src/kudu/tools/kudu-ts-cli-test.cc M src/kudu/util/random.h 13 files changed, 337 insertions(+), 47 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/6662/4 -- To view, visit http://gerrit.cloudera.org:8080/6662 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] external mini cluster: pass options by value and move where appropriate
Dan Burkert has posted comments on this change. Change subject: external mini cluster: pass options by value and move where appropriate .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6760 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I75cb8fb7e1c38ffba0ac4894a732c3d54f59da05 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: No
[kudu-CR] external mini cluster: pass options by value and move where appropriate
Adar Dembo has submitted this change and it was merged. Change subject: external mini cluster: pass options by value and move where appropriate .. external mini cluster: pass options by value and move where appropriate I also added a default constructor for the few cases where no option customization is needed (webserver-stress-itest and master_migration-itest). Change-Id: I75cb8fb7e1c38ffba0ac4894a732c3d54f59da05 Reviewed-on: http://gerrit.cloudera.org:8080/6760 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert --- M src/kudu/benchmarks/tpch/tpch_real_world.cc M src/kudu/integration-tests/all_types-itest.cc M src/kudu/integration-tests/alter_table-randomized-test.cc M src/kudu/integration-tests/client-stress-test.cc M src/kudu/integration-tests/external_mini_cluster-test.cc M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h M src/kudu/integration-tests/flex_partitioning-itest.cc M src/kudu/integration-tests/log-rolling-itest.cc M src/kudu/integration-tests/master-stress-test.cc M src/kudu/integration-tests/master_migration-itest.cc M src/kudu/integration-tests/open-readonly-fs-itest.cc M src/kudu/integration-tests/ts_itest-base.h M src/kudu/integration-tests/ts_recovery-itest.cc M src/kudu/integration-tests/version_migration-test.cc M src/kudu/integration-tests/webserver-stress-itest.cc 16 files changed, 35 insertions(+), 25 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/6760 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I75cb8fb7e1c38ffba0ac4894a732c3d54f59da05 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] log block manager: detect and repair unpunched holes
Adar Dembo has posted comments on this change.
Change subject: log block manager: detect and repair unpunched holes
..
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6717/4/src/kudu/fs/log_block_manager-test-util.cc
File src/kudu/fs/log_block_manager-test-util.cc:
Line 393: if (r == 0) {
> would this sequence of if()s read better as a 'case'?
Yeah, not sure why I didn't do it as a switch to begin with.
http://gerrit.cloudera.org:8080/#/c/6717/4/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
PS4, Line 2059: // triggers asynchronous hole punching
> not following this comment
Clearing needs_repunching drops the last references to the deleted LogBlocks.
Their destructors run and will queue up hole punch closures on the data dir's
thread pool.
The hole punching takes place asynchronously, but LBM::Open() will wait on the
thread pools anyway, so I guess it's not really all that asynchronous.
I'll clarify the comment.
http://gerrit.cloudera.org:8080/#/c/6717/4/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
Line 116: "Fraction of the time that write or preallocate
operations will fail");
> need to update this?
Done
--
To view, visit http://gerrit.cloudera.org:8080/6717
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I016ea401380f4c8c7b1fd907ff67cb595f377dd1
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error
Dan Burkert has posted comments on this change. Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error .. Patch Set 11: Also, could you update the known limitations section here: https://github.com/apache/kudu/blob/master/docs/security.adoc#known-limitations -- To view, visit http://gerrit.cloudera.org:8080/6640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959 Gerrit-PatchSet: 11 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [c++-client] clear non-covered entries from meta cache on table open
Dan Burkert has submitted this change and it was merged. Change subject: [c++-client] clear non-covered entries from meta cache on table open .. [c++-client] clear non-covered entries from meta cache on table open Clearing non-covered range entries from the meta cache on table open makes it easier to share client instances among different contexts without having to worry about polluting the non-covering range cache. Change-Id: I195eac8caa5efca9ad95284ab707c38340ba47f0 Reviewed-on: http://gerrit.cloudera.org:8080/6713 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h 5 files changed, 96 insertions(+), 0 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/6713 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I195eac8caa5efca9ad95284ab707c38340ba47f0 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon
[kudu-CR] subprocess: add KillAndWait() and allow customization of exit signal
Todd Lipcon has posted comments on this change.
Change subject: subprocess: add KillAndWait() and allow customization of exit
signal
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6741/1/src/kudu/integration-tests/full_stack-insert-scan-test.cc
File src/kudu/integration-tests/full_stack-insert-scan-test.cc:
PS1, Line 232: string cmd = Substitute("perf record --pid=$0", getpid());
: if (FLAGS_perf_fp_flag) cmd += " --call-graph fp";
> What systems are those? On the two that I tested (Ubuntu 16 and el6.6) "per
Going back to the original code review which introduced this, Vlad said "The
only issue is that adding "fp" will break on el6 since that parameter doesn't
take an argument in older perf versions." I'm guessing this refers to http://gerrit.cloudera.org:8080/6741
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf31bd4ea6de2917521a9852714fb33cfbec1f61
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] log block manager: detect and repair unpunched holes
Todd Lipcon has posted comments on this change.
Change subject: log block manager: detect and repair unpunched holes
..
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6717/4/src/kudu/fs/log_block_manager-test-util.cc
File src/kudu/fs/log_block_manager-test-util.cc:
Line 393: if (r == 0) {
would this sequence of if()s read better as a 'case'?
http://gerrit.cloudera.org:8080/#/c/6717/4/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
PS4, Line 2059: // triggers asynchronous hole punching
not following this comment
http://gerrit.cloudera.org:8080/#/c/6717/4/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
Line 116: "Fraction of the time that write or preallocate
operations will fail");
need to update this?
--
To view, visit http://gerrit.cloudera.org:8080/6717
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I016ea401380f4c8c7b1fd907ff67cb595f377dd1
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] log block manager: more container accounting
Todd Lipcon has posted comments on this change. Change subject: log block manager: more container accounting .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1907db2a74bcc074ecf882f8a758c3989431e267 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1978: avoid corruption when deleting misaligned blocks
Todd Lipcon has posted comments on this change. Change subject: KUDU-1978: avoid corruption when deleting misaligned blocks .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/6715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6ed6e349ab10d8d04722cd7ef99e7a06554f51f1 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] WIP: annotations to make TSAN aware of RWCLock semantics
Adar Dembo has abandoned this change. Change subject: WIP: annotations to make TSAN aware of RWCLock semantics .. Abandoned It's been a year since I published this change, and since I wasn't able to get it to work, no need to keep it open. -- To view, visit http://gerrit.cloudera.org:8080/2852 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Id09b3c0fdd93f4f46be6b767266c8be8889b95b6 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] external mini cluster: pass options by value and move where appropriate
Hello Dan Burkert, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/6760 to review the following change. Change subject: external mini cluster: pass options by value and move where appropriate .. external mini cluster: pass options by value and move where appropriate I also added a default constructor for the few cases where no option customization is needed (webserver-stress-itest and master_migration-itest). Change-Id: I75cb8fb7e1c38ffba0ac4894a732c3d54f59da05 --- M src/kudu/benchmarks/tpch/tpch_real_world.cc M src/kudu/integration-tests/all_types-itest.cc M src/kudu/integration-tests/alter_table-randomized-test.cc M src/kudu/integration-tests/client-stress-test.cc M src/kudu/integration-tests/external_mini_cluster-test.cc M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h M src/kudu/integration-tests/flex_partitioning-itest.cc M src/kudu/integration-tests/log-rolling-itest.cc M src/kudu/integration-tests/master-stress-test.cc M src/kudu/integration-tests/master_migration-itest.cc M src/kudu/integration-tests/open-readonly-fs-itest.cc M src/kudu/integration-tests/ts_itest-base.h M src/kudu/integration-tests/ts_recovery-itest.cc M src/kudu/integration-tests/version_migration-test.cc M src/kudu/integration-tests/webserver-stress-itest.cc 16 files changed, 35 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/6760/1 -- To view, visit http://gerrit.cloudera.org:8080/6760 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I75cb8fb7e1c38ffba0ac4894a732c3d54f59da05 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert
[kudu-CR] env: Always read fully when reading files
Todd Lipcon has posted comments on this change. Change subject: env: Always read fully when reading files .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6758 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] env: Always read fully when reading files
Adar Dembo has posted comments on this change. Change subject: env: Always read fully when reading files .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6758 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: No
[kudu-CR] env: Always read fully when reading files
Grant Henke has posted comments on this change.
Change subject: env: Always read fully when reading files
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6758/4/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
Line 1457: void InitDefaultEnv() { default_env = new PosixEnv; }
> These aren't in the anonymous namespace, so they should either remain stati
My bad, a bad find and replace.
--
To view, visit http://gerrit.cloudera.org:8080/6758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] env: Always read fully when reading files
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6758 to look at the new patch set (#5). Change subject: env: Always read fully when reading files .. env: Always read fully when reading files In KUDU-9 env_util::ReadFully was added to ensure short reads were retried until all data was read. Later RWFile was implemented with “read fully” behavior by default. (see a15c795360e32885c00442efacd2a345f993f425) Given we almost always use ReadFully or expect the data to be fully read regardless, this patch moves the “read fully” behavior into the Read function so it is always used. Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 --- M src/kudu/consensus/log_util.cc M src/kudu/fs/file_block_manager.cc M src/kudu/fs/log_block_manager-test.cc M src/kudu/tserver/tablet_copy_client-test.cc M src/kudu/tserver/tablet_copy_service-test.cc M src/kudu/tserver/tablet_copy_source_session.cc M src/kudu/tserver/tablet_copy_source_session.h M src/kudu/util/env-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/env_util.cc M src/kudu/util/env_util.h M src/kudu/util/pb_util-test.cc M src/kudu/util/pb_util.cc 13 files changed, 78 insertions(+), 163 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/6758/5 -- To view, visit http://gerrit.cloudera.org:8080/6758 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] env: Always read fully when reading files
Adar Dembo has posted comments on this change.
Change subject: env: Always read fully when reading files
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6758/4/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
PS4, Line 1455: pthread_once_t once = PTHREAD_ONCE_INIT;
: Env* default_env;
: void InitDefaultEnv() { default_env = new PosixEnv; }
These aren't in the anonymous namespace, so they should either remain static or
be placed in the namespace that ends on L1453.
FYI, such symbols are globally visible and could collide with other symbols of
the same name in other compilation units.
--
To view, visit http://gerrit.cloudera.org:8080/6758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1981 Kudu should run at hosts len(FQDN) > 64
Alexey Serbin has posted comments on this change.
Change subject: KUDU-1981 Kudu should run at hosts len(FQDN) > 64
..
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6734/3/src/kudu/security/cert.cc
File src/kudu/security/cert.cc:
Line 107: LOG(DFATAL) << "invalid DNS name is the SAN field";
> should 'is' be 'in' here? It's a bit confusing as written. And having a r
Done
http://gerrit.cloudera.org:8080/#/c/6734/3/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:
Line 297: config->hostnames = { fqdn };
> Looks like we would never set more than a single hostname for the FQDN? Sh
Done
--
To view, visit http://gerrit.cloudera.org:8080/6734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie142e76e9b2dcef3e07dd33d82b6758c746ced19
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1981 Kudu should run at hosts len(FQDN) > 64
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6734 to look at the new patch set (#4). Change subject: KUDU-1981 Kudu should run at hosts len(FQDN) > 64 .. KUDU-1981 Kudu should run at hosts len(FQDN) > 64 This is a fix for KUDU-1981: with security enabled, Kudu servers cannot start at machines with len(FQDN) > 64. Prior to this fix, the host FQDN was put into the CSR's CN (common name) field while generating self-signed certificate for server RPC messenger. Per RFC5280, the CN field cannot contain strings longer than 64 characters long, and it seems OpenSSL enforces that limit as required. The idea is to put FQDNs into the SAN X509v3 extension field as 'DNS' fields. That makes it possible to have names in the SAN which are even longer than 255 characters. This patch returns back a part of the SAN-related functionality which had been implemented initially in cert_management.cc and then removed since it was not used back then. This patch also adds a couple of unit tests to cover the new functionality and to make sure it's possible to set CN field of CSR to 64-chars length value and have corresponding X509 certificate generated with no issues. Change-Id: Ie142e76e9b2dcef3e07dd33d82b6758c746ced19 --- M src/kudu/integration-tests/master_cert_authority-itest.cc M src/kudu/security/ca/cert_management-test.cc M src/kudu/security/ca/cert_management.cc M src/kudu/security/ca/cert_management.h M src/kudu/security/cert-test.cc M src/kudu/security/cert.cc M src/kudu/security/cert.h M src/kudu/security/test/test_certs.cc M src/kudu/security/test/test_certs.h M src/kudu/security/tls_context.cc 10 files changed, 232 insertions(+), 74 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/6734/4 -- To view, visit http://gerrit.cloudera.org:8080/6734 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie142e76e9b2dcef3e07dd33d82b6758c746ced19 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] env: Always read fully when reading files
Grant Henke has posted comments on this change. Change subject: env: Always read fully when reading files .. Patch Set 3: As talked about via chat, I will follow up with a separate patch to simplify the Read() API. -- To view, visit http://gerrit.cloudera.org:8080/6758 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: No
[kudu-CR] env: Always read fully when reading files
Grant Henke has posted comments on this change.
Change subject: env: Always read fully when reading files
..
Patch Set 3:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/6758/3//COMMIT_MSG
Commit Message:
Line 9: In KUDU-9(see ) env_util::ReadFully was added to ensure
> Missing a link here? To be honest it's not necessary; KUDU-9 is descriptive
Done
Line 13: Later RWFile was implimented with “read fully” behavior by default.
> implemented
Done
Line 14: (see a15c795360e32885c00442efacd2a345f993f425)
> This is the same commit hash as above; one of them is probably wrong.
Done
Line 17: fully read regardles, this patch moves the “read fully” behavior
> Nit: regardless
Done
http://gerrit.cloudera.org:8080/#/c/6758/3/src/kudu/util/env-test.cc
File src/kudu/util/env-test.cc:
Line 376: class ShortReadRandomAccessFile : public RandomAccessFile {
> Not using this anymore, can be removed?
Done
http://gerrit.cloudera.org:8080/#/c/6758/3/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
Line 255: static Status DoRead(int fd, const string& filename, uint64_t offset,
size_t length,
> Mind fixing this? All these static functions are already in an anonymous na
Done
Line 273: Slice this_result(dst, r);
> Not really sure what 'this_result' adds; you could just as easily s/this_re
I just reverted back to the original RWFile::Read function. I agree though.
--
To view, visit http://gerrit.cloudera.org:8080/6758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] env: Always read fully when reading files
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6758 to look at the new patch set (#4). Change subject: env: Always read fully when reading files .. env: Always read fully when reading files In KUDU-9 env_util::ReadFully was added to ensure short reads were retried until all data was read. Later RWFile was implemented with “read fully” behavior by default. (see a15c795360e32885c00442efacd2a345f993f425) Given we almost always use ReadFully or expect the data to be fully read regardless, this patch moves the “read fully” behavior into the Read function so it is always used. Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 --- M src/kudu/consensus/log_util.cc M src/kudu/fs/file_block_manager.cc M src/kudu/fs/log_block_manager-test.cc M src/kudu/tserver/tablet_copy_client-test.cc M src/kudu/tserver/tablet_copy_service-test.cc M src/kudu/tserver/tablet_copy_source_session.cc M src/kudu/tserver/tablet_copy_source_session.h M src/kudu/util/env-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/env_util.cc M src/kudu/util/env_util.h M src/kudu/util/pb_util-test.cc M src/kudu/util/pb_util.cc 13 files changed, 81 insertions(+), 166 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/6758/4 -- To view, visit http://gerrit.cloudera.org:8080/6758 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1970: node density integration test
Adar Dembo has posted comments on this change. Change subject: KUDU-1970: node density integration test .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/6662/1//COMMIT_MSG Commit Message: Line 20: WIP because an itest isn't a great fit; we're not actually testing anything. > isn't it a sort of stress test, in that we're testing larger data volumes t Thanks for the feedback. Ended up doing a little bit of both: it's still a "stress test" that runs in about 25s on my laptop, but it also runs as part of benchmarks.sh and collects interesting metrics. -- To view, visit http://gerrit.cloudera.org:8080/6662 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] external mini cluster: spawn perf record for each daemon during Start()
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6742 to look at the new patch set (#2). Change subject: external mini cluster: spawn perf record for each daemon during Start() .. external mini cluster: spawn perf record for each daemon during Start() I want this for the dense node test, but it's not really possible to do from outside ExternalMiniCluster without missing out on time spent in Start(), which I was interested in measuring. So here's a generic approach that can be used by any itest. I wasn't sure whether this should be configured via EMC option or gflag. I ended up with the latter because it's not really something a test needs programmatic access to; it's just something the dev running the test might want to enable manually. I also changed the existing perf calls in full_stack-insert-scan-test to use the new subprocess custom destroy signal. While there I fixed the handling of "--call-graph"; passing it without "fp" is a syntax error on both el6.6 and Ubuntu 16.04. Change-Id: I92079f616648788b461d550057b8e23bc9174b71 --- M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h M src/kudu/integration-tests/full_stack-insert-scan-test.cc 3 files changed, 58 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/6742/2 -- To view, visit http://gerrit.cloudera.org:8080/6742 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I92079f616648788b461d550057b8e23bc9174b71 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1970: node density integration test
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6662 to look at the new patch set (#2). Change subject: KUDU-1970: node density integration test .. KUDU-1970: node density integration test This patch introduces a new itest that models a storage-dense Kudu deployment. The idea is simple: rather than actually generating and storing lots of data (which is both time intensive and developer unfriendly), let's produce a lot of metadata instead, as that's cheaper and can proxy for data. The test itself isn't that interesting; most of the challenge was in running it repeatedly to determine which flag values yielded the most metadata. In a run of the test on a 48 core el6.6 machine (max_blocks_per_container=8 and num_seconds=240), I produced ~110K blocks across ~21k LBM containers, which yielded a subsequent ~100s LBM startup time. I made the following modifications elsewhere to make this work: - TestWorkload now supports arbitrary schemas. - EMC-based tests can configure the amount of time they wait on each daemon process to start as the server info file isn't dumped until after FS startup is complete (maybe that should be changed?) - The benchmarks.sh script runs the test with some customized parameters. I also snuck in changes to remove an unused variable from random.h and to switch TestWorkload from kudu::Thread to std::thread. Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d --- M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/dense_node-itest.cc M src/kudu/integration-tests/external_mini_cluster-itest-base.cc M src/kudu/integration-tests/external_mini_cluster-itest-base.h M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/scripts/benchmarks.sh M src/kudu/tools/data_gen_util.cc M src/kudu/tools/data_gen_util.h M src/kudu/tools/kudu-ts-cli-test.cc M src/kudu/util/random.h 13 files changed, 336 insertions(+), 47 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/6662/2 -- To view, visit http://gerrit.cloudera.org:8080/6662 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1978: avoid corruption when deleting misaligned blocks
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6715 to look at the new patch set (#4). Change subject: KUDU-1978: avoid corruption when deleting misaligned blocks .. KUDU-1978: avoid corruption when deleting misaligned blocks The gist of the problem: when punching out blocks, we don't actually know where they end. The existing code assumes that proper alignment is always maintained, but when faced with a repeated sequence of blocks misaligned on both start and end offsets, this may not be true. It is conceivable (though not certain) that KUDU-1793 can cause such sequences. To fix, we could maintain per-block end offsets, but that'd be expensive for such a rare case (remember: we have millions of blocks). Instead, we'll look for blocks misaligned on their start offsets and skip the align up step when punching them out. This means we won't reclaim all of their space, but it's better than corrupting data belonging to the next block. A black box reproduction of KUDU-1793 is virtually impossible, so here's a white box implementation instead, via augmentation of the LBMCorruptor's misaligned block generator. Without the fix (though with the removal of the DCHECK_EQ), the new test fails 100% of the time. With the fix, it hasn't failed in 1000 runs. Change-Id: I6ed6e349ab10d8d04722cd7ef99e7a06554f51f1 --- M src/kudu/fs/log_block_manager-test-util.cc M src/kudu/fs/log_block_manager-test-util.h M src/kudu/fs/log_block_manager-test.cc M src/kudu/fs/log_block_manager.cc 4 files changed, 180 insertions(+), 42 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/6715/4 -- To view, visit http://gerrit.cloudera.org:8080/6715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6ed6e349ab10d8d04722cd7ef99e7a06554f51f1 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] subprocess: add KillAndWait() and allow customization of exit signal
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6741 to look at the new patch set (#2). Change subject: subprocess: add KillAndWait() and allow customization of exit signal .. subprocess: add KillAndWait() and allow customization of exit signal This patch does two things: - It introduces a KillAndWait() method that terminates and fully reaps a process. - It allows one to customize the exit signal delivered to a subprocess when it goes out of scope. The default signal is SIGKILL which doesn't let subprocesses clean up after themselves. Change-Id: Iaf31bd4ea6de2917521a9852714fb33cfbec1f61 --- M src/kudu/util/subprocess-test.cc M src/kudu/util/subprocess.cc M src/kudu/util/subprocess.h 3 files changed, 113 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/6741/2 -- To view, visit http://gerrit.cloudera.org:8080/6741 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iaf31bd4ea6de2917521a9852714fb33cfbec1f61 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] env: Always read fully when reading files
Adar Dembo has posted comments on this change.
Change subject: env: Always read fully when reading files
..
Patch Set 3:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/6758/3//COMMIT_MSG
Commit Message:
PS3, Line 9: KUDU-9(see )
Missing a link here? To be honest it's not necessary; KUDU-9 is descriptive
enough.
PS3, Line 13: implimented
implemented
Line 14: (see a15c795360e32885c00442efacd2a345f993f425)
This is the same commit hash as above; one of them is probably wrong.
PS3, Line 17: regardles
Nit: regardless
http://gerrit.cloudera.org:8080/#/c/6758/3/src/kudu/util/env-test.cc
File src/kudu/util/env-test.cc:
Line 376: class ShortReadRandomAccessFile : public RandomAccessFile {
Not using this anymore, can be removed?
http://gerrit.cloudera.org:8080/#/c/6758/3/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
Line 255: static Status DoRead(int fd, const string& filename, uint64_t offset,
size_t length,
> warning: 'DoRead' is a static definition in anonymous namespace; static is
Mind fixing this? All these static functions are already in an anonymous
namespace, so they needn't be declared static.
Line 273: Slice this_result(dst, r);
Not really sure what 'this_result' adds; you could just as easily
s/this_result.size()/r/ in the body of the loop.
--
To view, visit http://gerrit.cloudera.org:8080/6758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] env: Always read fully when reading files
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6758 to look at the new patch set (#3). Change subject: env: Always read fully when reading files .. env: Always read fully when reading files In KUDU-9(see ) env_util::ReadFully was added to ensure short reads were retried until all data was read. (see a15c795360e32885c00442efacd2a345f993f425) Later RWFile was implimented with “read fully” behavior by default. (see a15c795360e32885c00442efacd2a345f993f425) Given we almost always use ReadFully or expect the data to be fully read regardles, this patch moves the “read fully” behavior into the Read function so it is always used. Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 --- M src/kudu/consensus/log_util.cc M src/kudu/fs/file_block_manager.cc M src/kudu/fs/log_block_manager-test.cc M src/kudu/tserver/tablet_copy_client-test.cc M src/kudu/tserver/tablet_copy_service-test.cc M src/kudu/tserver/tablet_copy_source_session.cc M src/kudu/tserver/tablet_copy_source_session.h M src/kudu/util/env-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/env_util.cc M src/kudu/util/env_util.h M src/kudu/util/pb_util-test.cc M src/kudu/util/pb_util.cc 13 files changed, 76 insertions(+), 126 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/6758/3 -- To view, visit http://gerrit.cloudera.org:8080/6758 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] env: Always read fully when reading files
Grant Henke has posted comments on this change.
Change subject: env: Always read fully when reading files
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6758/1/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
Line 260: *result = Slice(scratch, 0);
> A calling convention we like to adhere to is: if the function returns a bad
Done
Line 270: Substitute("Can't short read great than or equal to the
$0 bytes requested", length));
> Since this is test-only, perhaps just CHECK fail here?
Done
Line 288: *result = Slice(scratch, result->size() + r);
> Given the above calling convention, you should only need to set up the resu
Done
--
To view, visit http://gerrit.cloudera.org:8080/6758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] env: Always read fully when reading files
Grant Henke has posted comments on this change. Change subject: env: Always read fully when reading files .. Patch Set 2: > In many places, env_util.h was only included for ReadFully(). Wherever you're > replacing ReadFully() with Read(), could you check if that was the last thing > that requires env_util.h, and if so, remove it? I did this, but I will go back through and double check again. > With short reads out of the picture, I wonder if we can simplify the Read() > API a bit. As per the comment I left in your checksumming patch, having both > the scratch/len and the slice seems redundant; one or the other should > suffice to fully describe the read input and output. I think the scratch and length are decoupled so that you can use a scratch that is larger than the request length you plan to read. This allows you to allocate a scratch and fill it with multiple reads. -- To view, visit http://gerrit.cloudera.org:8080/6758 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] env: Always read fully when reading files
Adar Dembo has posted comments on this change.
Change subject: env: Always read fully when reading files
..
Patch Set 1:
(3 comments)
Two high level comments:
- In many places, env_util.h was only included for ReadFully(). Wherever you're
replacing ReadFully() with Read(), could you check if that was the last thing
that requires env_util.h, and if so, remove it?
- With short reads out of the picture, I wonder if we can simplify the Read()
API a bit. As per the comment I left in your checksumming patch, having both
the scratch/len and the slice seems redundant; one or the other should suffice
to fully describe the read input and output.
http://gerrit.cloudera.org:8080/#/c/6758/1/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
PS1, Line 260: *result = Slice(scratch, 0);
A calling convention we like to adhere to is: if the function returns a bad
Status, don't modify any OUT parameters.
To apply that here, use a local Slice and set *result only when you know you're
going to return success.
PS1, Line 269: return Status::IOError(
: Substitute("Can't short read great than or equal to
the $0 bytes requested", length));
Since this is test-only, perhaps just CHECK fail here?
Line 288: *result = Slice(scratch, result->size() + r);
Given the above calling convention, you should only need to set up the results
slice once, when all the reading is done.
--
To view, visit http://gerrit.cloudera.org:8080/6758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] env: Always read fully when reading files
Grant Henke has uploaded a new patch set (#2). Change subject: env: Always read fully when reading files .. env: Always read fully when reading files In KUDU-9(see ) env_util::ReadFully was added to ensure short reads were retried until all data was read. (see a15c795360e32885c00442efacd2a345f993f425) Later RWFile was implimented with “read fully” behavior by default. (see a15c795360e32885c00442efacd2a345f993f425) Given we almost always use ReadFully or expect the data to be fully read regardles, this patch moves the “read fully” behavior into the Read function so it is always used. Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 --- M src/kudu/consensus/log_util.cc M src/kudu/fs/file_block_manager.cc M src/kudu/fs/log_block_manager-test.cc M src/kudu/tserver/tablet_copy_client-test.cc M src/kudu/tserver/tablet_copy_service-test.cc M src/kudu/tserver/tablet_copy_source_session.cc M src/kudu/tserver/tablet_copy_source_session.h M src/kudu/util/env-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/env_util.cc M src/kudu/util/env_util.h M src/kudu/util/pb_util-test.cc M src/kudu/util/pb_util.cc 13 files changed, 84 insertions(+), 126 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/6758/2 -- To view, visit http://gerrit.cloudera.org:8080/6758 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error
Alexey Serbin has posted comments on this change. Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/6640/11/src/kudu/integration-tests/security-unknown-tsk-itest.cc File src/kudu/integration-tests/security-unknown-tsk-itest.cc: Line 353: // Targeting the total runtime to be less than 15 minutes. A cycle might takes > This test was the long pole in the test suite: http://dist-test.cloudera.or That's a good observation. Sure -- we can run every iteration of this test no longer than , say 20 seconds, and have less iterations. I'll update this correspondingly. -- To view, visit http://gerrit.cloudera.org:8080/6640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959 Gerrit-PatchSet: 11 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] env: Always read fully when reading files
Grant Henke has uploaded a new change for review. http://gerrit.cloudera.org:8080/6758 Change subject: env: Always read fully when reading files .. env: Always read fully when reading files In KUDU-9(see ) env_util::ReadFully was added to ensure short reads were retried until all data was read. (see a15c795360e32885c00442efacd2a345f993f425) Later RWFile was implimented with “read fully” behavior by default. (see a15c795360e32885c00442efacd2a345f993f425) Given we almost always use ReadFully or expect the data to be fully written regardles, this patch moves the “read fully” behavior into the Read function so it is always used. Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 --- M src/kudu/consensus/log_util.cc M src/kudu/fs/file_block_manager.cc M src/kudu/fs/log_block_manager-test.cc M src/kudu/tserver/tablet_copy_client-test.cc M src/kudu/tserver/tablet_copy_service-test.cc M src/kudu/tserver/tablet_copy_source_session.cc M src/kudu/tserver/tablet_copy_source_session.h M src/kudu/util/env-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/env_util.cc M src/kudu/util/env_util.h M src/kudu/util/pb_util-test.cc M src/kudu/util/pb_util.cc 13 files changed, 84 insertions(+), 126 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/6758/1 -- To view, visit http://gerrit.cloudera.org:8080/6758 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke
[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error
Dan Burkert has posted comments on this change. Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/6640/11/src/kudu/integration-tests/security-unknown-tsk-itest.cc File src/kudu/integration-tests/security-unknown-tsk-itest.cc: Line 353: // Targeting the total runtime to be less than 15 minutes. A cycle might takes This test was the long pole in the test suite: http://dist-test.cloudera.org/job?job_id=jenkins-slave.1493279312.20587. Is there any way to speed this up? -- To view, visit http://gerrit.cloudera.org:8080/6640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959 Gerrit-PatchSet: 11 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] mapreduce: add support for fault tolerant scanner
Jean-Daniel Cryans has posted comments on this change.
Change subject: mapreduce: add support for fault tolerant scanner
..
Patch Set 2:
The test failure is related to the fault tolerant scanner patch:
01:09:34.954 [INFO - kudu-tserver:64040] (MiniKuduCluster.java:568) W0428
01:09:34.954133 17141 connection.cc:380] Connection torn down before Call
kudu.tserver.TabletServerService.Write from 127.0.0.1:35554 (ReqId={client:
21809ecd299e454fa1a5ce2dcc9d4faf, seq_no=16347, attempt_no=1}) could send its
response
01:09:34.954 [ERROR - Thread-10] (ITClient.java:134) Got error while row
counting
org.apache.kudu.client.RecoverableException: [Peer
7605e33bc78748a08bd01815dd8aff56] Connection disconnected
at org.apache.kudu.client.TabletClient.cleanup(TabletClient.java:657)
at
org.apache.kudu.client.TabletClient.channelDisconnected(TabletClient.java:610)
at
org.jboss.netty.channel.SimpleChannelUpstreamHandler.handleUpstream(SimpleChannelUpstreamHandler.java:102)
at
org.apache.kudu.client.TabletClient.handleUpstream(TabletClient.java:603)
at
org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
What this means is that the connection got closed before the scan was sent. I
think this should be a retryable exception even for non-fault tolerant
scanners? Maybe it would make more sense to make ITClient use fault tolerant
scanners too.
--
To view, visit http://gerrit.cloudera.org:8080/6745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc39472e2733bab4e00e73658f8a7619153bd7c6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] KUDU-579 [java client] Scanner fault tolerance
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: KUDU-579 [java_client] Scanner fault tolerance .. KUDU-579 [java_client] Scanner fault tolerance This patch adds java client support to restart scanners if they fail in the middle of table scanning. AsyncKuduScanner records the final primary key retrieved in the previous batch. If a tserver fails while scanning, the client marks the tserver as failed and retries the scan elsewhere, providing its last primary key. Change-Id: I89d3634c4255b69e28f2de5412e6a5a9d34e931b Reviewed-on: http://gerrit.cloudera.org:8080/6566 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert Reviewed-by: Jean-Daniel Cryans --- M java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java A java/kudu-client/src/main/java/org/apache/kudu/client/ScannerExpiredException.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java A java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java A java/kudu-client/src/test/java/org/apache/kudu/client/ITNonFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java 12 files changed, 467 insertions(+), 98 deletions(-) Approvals: Dan Burkert: Looks good to me, but someone else must approve Jean-Daniel Cryans: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/6566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I89d3634c4255b69e28f2de5412e6a5a9d34e931b Gerrit-PatchSet: 11 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-579 [java client] Scanner fault tolerance
Jean-Daniel Cryans has posted comments on this change. Change subject: KUDU-579 [java_client] Scanner fault tolerance .. Patch Set 10: Code-Review+2 Good job on the new test. -- To view, visit http://gerrit.cloudera.org:8080/6566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I89d3634c4255b69e28f2de5412e6a5a9d34e931b Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] KUDU-579 [java client] Scanner fault tolerance
Dan Burkert has posted comments on this change. Change subject: KUDU-579 [java_client] Scanner fault tolerance .. Patch Set 10: Code-Review+1 This is looking good to me. I'll leave it to JD to give a +2. -- To view, visit http://gerrit.cloudera.org:8080/6566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I89d3634c4255b69e28f2de5412e6a5a9d34e931b Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] KUDU-1981 Kudu should run at hosts len(FQDN) > 64
Dan Burkert has posted comments on this change.
Change subject: KUDU-1981 Kudu should run at hosts len(FQDN) > 64
..
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6734/3/src/kudu/security/cert.cc
File src/kudu/security/cert.cc:
Line 107: LOG(DFATAL) << "invalid DNS name is the SAN field";
should 'is' be 'in' here? It's a bit confusing as written. And having a
return after LOG(DFATAL) is a noop, right?
http://gerrit.cloudera.org:8080/#/c/6734/3/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:
Line 297: config->hostnames = { fqdn };
Looks like we would never set more than a single hostname for the FQDN? Should
we simplify the interface to only take a single string instead of a vector?
--
To view, visit http://gerrit.cloudera.org:8080/6734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie142e76e9b2dcef3e07dd33d82b6758c746ced19
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-861 Support changing column defaults and storage attributes
Dan Burkert has posted comments on this change. Change subject: KUDU-861 Support changing column defaults and storage attributes .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I48dcfd1ced488943c3da1eb0a26f62780ac9214f Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
