[kudu-CR] [fs] fixed file num low live ratio metadata compaction policy when startup
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15830 ) Change subject: [fs] fixed file num low live ratio metadata compaction policy when startup .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/15830/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15830/4//COMMIT_MSG@7 PS4, Line 7: [fs] fixed file num low live ratio metadata compaction policy when : startup > nit: keep the title on a single line Also I think this would be a clearer: "[fs] limit to the number of metadata file compactions done at startup" http://gerrit.cloudera.org:8080/#/c/15830/4/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/15830/4/src/kudu/fs/log_block_manager.cc@2668 PS4, Line 2668: low_live_ratio_metadata_to_compact_num_.Increment(); > Using the number of metadata files seems kind of odd, given it requires som Looking at the definition of BlockRecordPB, it seems to have the following members: ::google::protobuf::internal::InternalMetadataWithArena _internal_metadata_; ::google::protobuf::internal::HasBits<1> _has_bits_; mutable int _cached_size_; ::kudu::BlockIdPB* block_id_; ::google::protobuf::uint64 timestamp_us_; ::google::protobuf::int64 offset_; ::google::protobuf::int64 length_; int op_type_; Most of these seem like trivial types, so we might be able to use `sizeof(BlockRecordPB) + sizeof(BlockIdPB)` as the individual size of a record on the heap (i.e. per element in the vector). -- To view, visit http://gerrit.cloudera.org:8080/15830 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifad1d6de4e61a1ddcfb35743abd71b57d6418896 Gerrit-Change-Number: 15830 Gerrit-PatchSet: 4 Gerrit-Owner: wangning <1994wangn...@gmail.com> Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: wangning <1994wangn...@gmail.com> Gerrit-Comment-Date: Thu, 14 May 2020 05:56:33 + Gerrit-HasComments: Yes
[kudu-CR] [fs] fixed file num low live ratio metadata compaction policy when startup
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/15830 )
Change subject: [fs] fixed file num low live ratio metadata compaction policy
when startup
..
Patch Set 4:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/15830/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15830/4//COMMIT_MSG@7
PS4, Line 7: [fs] fixed file num low live ratio metadata compaction policy when
: startup
nit: keep the title on a single line
http://gerrit.cloudera.org:8080/#/c/15830/4//COMMIT_MSG@10
PS4, Line 10: it loads all low live ratio metadata into memory,
nit: I wasn't sure what "low live ratio metadata" referred to. Could you also
add a small description here, e.g.
"it loads all low live ratio metadata into memory (i.e. metadata for containers
that contain a low number of live blocks), and this is..."
http://gerrit.cloudera.org:8080/#/c/15830/4//COMMIT_MSG@11
PS4, Line 11: controll
nit: "control" with one 'l'
http://gerrit.cloudera.org:8080/#/c/15830/4//COMMIT_MSG@16
PS4, Line 16: excess
nit: "exceed"
http://gerrit.cloudera.org:8080/#/c/15830/4//COMMIT_MSG@18
PS4, Line 18: livemetadata
nit: add a space: "live metadata"
http://gerrit.cloudera.org:8080/#/c/15830/4//COMMIT_MSG@29
PS4, Line 29: rting(Not
nit: add a space in between "starting (", and "Not" should be lower cased.
http://gerrit.cloudera.org:8080/#/c/15830/4//COMMIT_MSG@30
PS4, Line 30: excess
nit: "exceeds"
http://gerrit.cloudera.org:8080/#/c/15830/4/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
http://gerrit.cloudera.org:8080/#/c/15830/4/src/kudu/fs/log_block_manager.cc@119
PS4, Line 119: DEFINE_int32(max_num_low_live_ratio_metadata_file_to_compact, -1,
nit: can you put this up by log_container_live_metadata_before_compact_ratio so
the compaction-related flags are adjacent to one another?
http://gerrit.cloudera.org:8080/#/c/15830/4/src/kudu/fs/log_block_manager.cc@120
PS4, Line 120:
nit: spell out "number"
http://gerrit.cloudera.org:8080/#/c/15830/4/src/kudu/fs/log_block_manager.cc@2668
PS4, Line 2668: low_live_ratio_metadata_to_compact_num_.Increment();
Using the number of metadata files seems kind of odd, given it requires some
experimentation or knowledge beforehand about what is being stored in memory to
use it effectively.
Would it be possible to measure size via BlockRecordPB::ByteSizeLong() and
limit the number of bytes we're allowed to collect for compaction instead? As
far as I can tell, ByteSizeLong() is the post-encoding size, so it may not be
accurate. But if we could get the in-memory size of the records, we could apply
some bytes limit, which seems more natural.
--
To view, visit http://gerrit.cloudera.org:8080/15830
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad1d6de4e61a1ddcfb35743abd71b57d6418896
Gerrit-Change-Number: 15830
Gerrit-PatchSet: 4
Gerrit-Owner: wangning <1994wangn...@gmail.com>
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: wangning <1994wangn...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2020 03:51:36 +
Gerrit-HasComments: Yes
[kudu-CR] [tools] add --ignore nonexistent for 'local replica delete'
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15911 to look at the new patch set (#2). Change subject: [tools] add --ignore_nonexistent for 'local_replica delete' .. [tools] add --ignore_nonexistent for 'local_replica delete' Added --ignore_nonexistent flag for the 'local_replica delete' tool. The motivation for this change is to make the real-world scripting scenarios easier if trying to clean up tablet servers of particular tablet replicas. Also added a test to verify the newly introduced functionality. Change-Id: I3ecd00dea7f19747566b11e3e2a545c97d5f8194 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_local_replica.cc 2 files changed, 39 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/15911/2 -- To view, visit http://gerrit.cloudera.org:8080/15911 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3ecd00dea7f19747566b11e3e2a545c97d5f8194 Gerrit-Change-Number: 15911 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [tools] add --ignore nonexistent for 'local replica delete'
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/15911 )
Change subject: [tools] add --ignore_nonexistent for 'local_replica delete'
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15911/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/15911/1/src/kudu/tools/kudu-tool-test.cc@2757
PS1, Line 2757: SCOPED_TRACE(stderr);
> nit: probably don't need this since ASSERT_STR_CONTAINS() prints the stderr
Done
http://gerrit.cloudera.org:8080/#/c/15911/1/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/15911/1/src/kudu/tools/tool_action_local_replica.cc@415
PS1, Line 415: // Force the specified tablet on this node to be in 'state'.
: s = TSTabletManager::DeleteTabletData(meta, cmeta_manager,
state, last_logged_opid);
: if (FLAGS_ignore_nonexistent && s.IsNotFound()) {
: LOG(INFO) << Substitute("ignoring error for tablet replica
$0 because "
: "of the --ignore_nonexistent flag:
$1",
: tablet_id, s.ToString());
: return Status::OK();
: }
> nit: I don't think this is necessary; AFAICT deleting data is mostly a meta
Right -- DeletaTabletData() removes Raft metadata and does some in-memory
changes, but I though I'd handle the absence of the Raft metadata as well since
I already went this route.
I added x.AndThen() piece, thanks.
--
To view, visit http://gerrit.cloudera.org:8080/15911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ecd00dea7f19747566b11e3e2a545c97d5f8194
Gerrit-Change-Number: 15911
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 14 May 2020 02:44:30 +
Gerrit-HasComments: Yes
[kudu-CR] [perf] Check range predicate first while evaluating Bloom filter predicate
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/15913 )
Change subject: [perf] Check range predicate first while evaluating Bloom
filter predicate
..
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/15913/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15913/1//COMMIT_MSG@18
PS1, Line 18: Tests:
nit: is this a best-case scenario? average scenario? worst case? If we tuned
the values a bit, would we be able to see a greater improvement? Is 20-30% the
most we'll get from this?
http://gerrit.cloudera.org:8080/#/c/15913/1/src/kudu/common/column_predicate.h
File src/kudu/common/column_predicate.h:
http://gerrit.cloudera.org:8080/#/c/15913/1/src/kudu/common/column_predicate.h@326
PS1, Line 326: return false;
: }
: } else if (upper_ != nullptr) {
: if (DataTypeTraits::Compare(cell,
this->upper_) >= 0) {
: return false;
: }
: } else if (lower_ != nullptr) {
: if (DataTypeTraits::Compare(cell,
this->lower_) < 0) {
: return false;
:
nit: spacing for 'return false'
http://gerrit.cloudera.org:8080/#/c/15913/1/src/kudu/common/column_predicate.h@322
PS1, Line 322: // Check optional lower and upper bound.
: if (lower_ != nullptr && upper_ != nullptr) {
: if (DataTypeTraits::Compare(cell,
this->upper_) >= 0 ||
: DataTypeTraits::Compare(cell,
this->lower_) < 0) {
: return false;
: }
: } else if (upper_ != nullptr) {
: if (DataTypeTraits::Compare(cell,
this->upper_) >= 0) {
: return false;
: }
: } else if (lower_ != nullptr) {
: if (DataTypeTraits::Compare(cell,
this->lower_) < 0) {
: return false;
: }
: }
nit: would this work:
if (lower_ && Compare(cell, lower_) < 0) {
return false;
}
if (upper_ && Compare(cell, upper_) >= 0) {
return false;
}
?
http://gerrit.cloudera.org:8080/#/c/15913/1/src/kudu/common/column_predicate.h@349
PS1, Line 349: return false;
nit: spacing
--
To view, visit http://gerrit.cloudera.org:8080/15913
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8451d6ddfe1fbdf307b3e9f2cc23a8d06e655ba3
Gerrit-Change-Number: 15913
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 14 May 2020 01:09:01 +
Gerrit-HasComments: Yes
[kudu-CR] [perf] Check range predicate first while evaluating Bloom filter predicate
Bankim Bhavsar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15913 Change subject: [perf] Check range predicate first while evaluating Bloom filter predicate .. [perf] Check range predicate first while evaluating Bloom filter predicate Range predicates can be specified along with Bloom filter predicate for the same column. It's cheaper to check against range predicate and exit early if the column value is out of bounds compared to computing hash and then looking up the value in Bloom filter. This case is common when Impala pushes down Bloom filter predicate as it'll likely be accompained by min-max filter (i.e. range predicate) on the same column. Tests: Added a test case that scans against 100M column values. Across iterations observed an improvement of 20-30% when the range predicate check prevents hash computation and Bloom filter lookup. Don't see any noticeable regression for the case where values are within range bounds. Without perf change: Time spent TestKuduBloomFilterPredicateBenchmark: Counting rows when no rows expected: real 0.953s user 0.001s sys 0.000s Time spent TestKuduBloomFilterPredicateBenchmark: Counting rows when range predicate doesn't prune: real 0.767s user 0.001s sys 0.000s Time spent TestKuduBloomFilterPredicateBenchmark: Counting rows when no rows expected: real 0.899s user 0.000s sys 0.000s Time spent TestKuduBloomFilterPredicateBenchmark: Counting rows when range predicate doesn't prune: real 0.775s user 0.000s sys 0.001s Time spent TestKuduBloomFilterPredicateBenchmark: Counting rows when no rows expected: real 0.983s user 0.000s sys 0.000s Time spent TestKuduBloomFilterPredicateBenchmark: Counting rows when range predicate doesn't prune: real 0.832s user 0.001s sys 0.000s With perf change: Time spent TestKuduBloomFilterPredicateBenchmark: Counting rows when no rows expected: real 0.725s user 0.001s sys 0.000s Time spent TestKuduBloomFilterPredicateBenchmark: Counting rows when range predicate doesn't prune: real 0.847s user 0.000s sys 0.000s Time spent TestKuduBloomFilterPredicateBenchmark: Counting rows when no rows expected: real 0.664s user 0.000s sys 0.000s Time spent TestKuduBloomFilterPredicateBenchmark: Counting rows when range predicate doesn't prune: real 0.794s user 0.001s sys 0.000s Time spent TestKuduBloomFilterPredicateBenchmark: Counting rows when no rows expected: real 0.706s user 0.001s sys 0.000s Time spent TestKuduBloomFilterPredicateBenchmark: Counting rows when range predicate doesn't prune: real 0.774s user 0.000s sys 0.000s Change-Id: I8451d6ddfe1fbdf307b3e9f2cc23a8d06e655ba3 --- M src/kudu/client/predicate-test.cc M src/kudu/common/column_predicate.h 2 files changed, 69 insertions(+), 42 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/15913/1 -- To view, visit http://gerrit.cloudera.org:8080/15913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8451d6ddfe1fbdf307b3e9f2cc23a8d06e655ba3 Gerrit-Change-Number: 15913 Gerrit-PatchSet: 1 Gerrit-Owner: Bankim Bhavsar
[kudu-CR] WIP [catalog manager] added CreateTable DoS scenario
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15912 Change subject: WIP [catalog_manager] added CreateTable DoS scenario .. WIP [catalog_manager] added CreateTable DoS scenario WIP: * address the issue in the catalog manager * enable the newly introduced scenario Change-Id: Ida42c59b2227ea9a71ee24cc8f8a50bdcb9648fb --- M src/kudu/integration-tests/create-table-itest.cc 1 file changed, 94 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/15912/1 -- To view, visit http://gerrit.cloudera.org:8080/15912 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ida42c59b2227ea9a71ee24cc8f8a50bdcb9648fb Gerrit-Change-Number: 15912 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] WIP [tools] multiple tablet ids in 'local replica delete'
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/15909 )
Change subject: WIP [tools] multiple tablet ids in 'local_replica delete'
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/15909/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/15909/1/src/kudu/tools/kudu-tool-test.cc@2782
PS1, Line 2782:
ts->server()->tablet_manager()->GetTabletReplicas(_replicas);
You probably need to clear this list before shutting down.
http://gerrit.cloudera.org:8080/#/c/15909/1/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/15909/1/src/kudu/tools/tool_action_local_replica.cc@411
PS1, Line 411: vector tablet_ids = strings::Split(tablet_ids_str, ",",
strings::SkipEmpty());
: if (tablet_ids.empty()) {
: return Status::InvalidArgument("no tablet identifiers
provided");
: }
nit: Split() is templatized and can return a set. Why not use that? Is order
preservation important here?
http://gerrit.cloudera.org:8080/#/c/15909/1/src/kudu/tools/tool_action_local_replica.cc@421
PS1, Line 421: LOG(INFO) << "removed duplicate tablet identifiers";
nit: might it be worth displaying how many unique tablet IDs there are? Or
maybe VLOGging a newline-separated list of tablet IDs?
--
To view, visit http://gerrit.cloudera.org:8080/15909
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0e509d1775be2a728e4e3b10c724c1f15a96ec1
Gerrit-Change-Number: 15909
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 13 May 2020 22:30:05 +
Gerrit-HasComments: Yes
[kudu-CR] [tools] add --ignore nonexistent for 'local replica delete'
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/15911 )
Change subject: [tools] add --ignore_nonexistent for 'local_replica delete'
..
Patch Set 1: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15911/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/15911/1/src/kudu/tools/kudu-tool-test.cc@2757
PS1, Line 2757: SCOPED_TRACE(stderr);
nit: probably don't need this since ASSERT_STR_CONTAINS() prints the stderr as
well
http://gerrit.cloudera.org:8080/#/c/15911/1/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/15911/1/src/kudu/tools/tool_action_local_replica.cc@415
PS1, Line 415: // Force the specified tablet on this node to be in 'state'.
: s = TSTabletManager::DeleteTabletData(meta, cmeta_manager,
state, last_logged_opid);
: if (FLAGS_ignore_nonexistent && s.IsNotFound()) {
: LOG(INFO) << Substitute("ignoring error for tablet replica
$0 because "
: "of the --ignore_nonexistent flag:
$1",
: tablet_id, s.ToString());
: return Status::OK();
: }
nit: I don't think this is necessary; AFAICT deleting data is mostly a metadata
operation and at this point, we've established that the metadata already
exists, though I don't mind it as it's more conservative.
Also, if you want, we could do something like:
auto s = TabletMetadata::Load().AndThen([&]{
return TSTabletManager::DeleteTabletData();
});
if (FLAGS_ignore_nonexistent && s.IsNotFound()) {
LOG(INFO) << ...
return Status::OK();
}
return s;
--
To view, visit http://gerrit.cloudera.org:8080/15911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ecd00dea7f19747566b11e3e2a545c97d5f8194
Gerrit-Change-Number: 15911
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 13 May 2020 22:13:33 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3096: Fix core dump when thread race between GetThreadStack and SuperviseThread
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/15899 ) Change subject: KUDU-3096: Fix core dump when thread race between GetThreadStack and SuperviseThread .. Patch Set 2: > Patch Set 1: > > Todd, thank you for replying to share these information, looks like two > things impact the part of code: 1.libunwind call malloc() in thread stack > trace collection, I find upstream have fixed it in libunwind 1.4.0 > https://github.com/libunwind/libunwind/pull/72 ; 2.save waiting time for > spawning thread. So I update the patch to address #2, set tid after > strings::Substitute that cause coredump in patchset 2, I think we maybe > update libunwind in the future. Why not update libunwind now? My point above is that, even if you've fixed this particular core dump, you still have a potential deadlock bug because of the call to malloc in libunwind. -- To view, visit http://gerrit.cloudera.org:8080/15899 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icc722cd5e8ed4ed668d279f6ec831e4eeb69f955 Gerrit-Change-Number: 15899 Gerrit-PatchSet: 2 Gerrit-Owner: RuiChen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: RuiChen Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 13 May 2020 22:01:36 + Gerrit-HasComments: No
[kudu-CR] [tools] add --ignore nonexistent for 'local replica delete'
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15911 ) Change subject: [tools] add --ignore_nonexistent for 'local_replica delete' .. Patch Set 1: Verified+1 Unrelated test failure in TsTabletManagerITest.TestTableStats (TSAN build) -- To view, visit http://gerrit.cloudera.org:8080/15911 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ecd00dea7f19747566b11e3e2a545c97d5f8194 Gerrit-Change-Number: 15911 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 May 2020 21:45:17 + Gerrit-HasComments: No
[kudu-CR] [tools] add --ignore nonexistent for 'local replica delete'
Alexey Serbin has removed a vote on this change. Change subject: [tools] add --ignore_nonexistent for 'local_replica delete' .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15911 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I3ecd00dea7f19747566b11e3e2a545c97d5f8194 Gerrit-Change-Number: 15911 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR](branch-1.12.x) docs: add Ranger integration
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15910 ) Change subject: docs: add Ranger integration .. docs: add Ranger integration Staged version here: https://github.com/haohaoc/kudu/blob/ranger-docs/docs/security.adoc Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2 Reviewed-on: http://gerrit.cloudera.org:8080/15897 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke (cherry picked from commit a961fbc7cac61737d03a0c9cf8898199a101f67e) Reviewed-on: http://gerrit.cloudera.org:8080/15910 --- M docs/kudu_impala_integration.adoc M docs/security.adoc 2 files changed, 218 insertions(+), 17 deletions(-) Approvals: Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15910 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.12.x Gerrit-MessageType: merged Gerrit-Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2 Gerrit-Change-Number: 15910 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [KUDU-3116] Enhance KuduContext row operation metrics
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15882 ) Change subject: [KUDU-3116] Enhance KuduContext row operation metrics .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/15882/6/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala: http://gerrit.cloudera.org:8080/#/c/15882/6/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@92 PS6, Line 92: , tableName: String = "*" Based on the current usage, why do we need a default at all? Aren't all operations associated with a table? http://gerrit.cloudera.org:8080/#/c/15882/6/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala: PS6: Could you also add a test that writes to multiple tables and confirms that the results make sense? -- To view, visit http://gerrit.cloudera.org:8080/15882 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie66dab95041310c27ef62dacccbcc0977a84857e Gerrit-Change-Number: 15882 Gerrit-PatchSet: 6 Gerrit-Owner: Brian McDevitt Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Brian McDevitt Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 May 2020 21:12:22 + Gerrit-HasComments: Yes
[kudu-CR](branch-1.12.x) docs: add Ranger integration
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15910 ) Change subject: docs: add Ranger integration .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15910 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.12.x Gerrit-MessageType: comment Gerrit-Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2 Gerrit-Change-Number: 15910 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 May 2020 21:06:39 + Gerrit-HasComments: No
[kudu-CR] [KUDU-3116] Enhance KuduContext row operation metrics
Hello Kudu Jenkins, Andrew Wong, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15882 to look at the new patch set (#6). Change subject: [KUDU-3116] Enhance KuduContext row operation metrics .. [KUDU-3116] Enhance KuduContext row operation metrics Adds the ability to track operation counts per table. Introduces the MapAccumulator to track these metrics in a single accumulator per operation type. Change-Id: Ie66dab95041310c27ef62dacccbcc0977a84857e --- M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala A java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/MapAccumulator.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala 3 files changed, 123 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/15882/6 -- To view, visit http://gerrit.cloudera.org:8080/15882 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie66dab95041310c27ef62dacccbcc0977a84857e Gerrit-Change-Number: 15882 Gerrit-PatchSet: 6 Gerrit-Owner: Brian McDevitt Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Brian McDevitt Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [KUDU-3116] Enhance KuduContext row operation metrics
Brian McDevitt has posted comments on this change. ( http://gerrit.cloudera.org:8080/15882 ) Change subject: [KUDU-3116] Enhance KuduContext row operation metrics .. Patch Set 5: These test failures are not in the Java code, but timeouts in the C tests. -- To view, visit http://gerrit.cloudera.org:8080/15882 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie66dab95041310c27ef62dacccbcc0977a84857e Gerrit-Change-Number: 15882 Gerrit-PatchSet: 5 Gerrit-Owner: Brian McDevitt Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Brian McDevitt Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 May 2020 19:41:50 + Gerrit-HasComments: No
[kudu-CR] [tools] add --ignore nonexistent for 'local replica delete'
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15911 Change subject: [tools] add --ignore_nonexistent for 'local_replica delete' .. [tools] add --ignore_nonexistent for 'local_replica delete' Added --ignore_nonexistent flag for the 'local_replica delete' tool. The motivation for this change is to make the real-world scripting scenarios easier if trying to clean up tablet servers of particular tablet replicas. Also added a test to verify the newly introduced functionality. Change-Id: I3ecd00dea7f19747566b11e3e2a545c97d5f8194 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_local_replica.cc 2 files changed, 47 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/15911/1 -- To view, visit http://gerrit.cloudera.org:8080/15911 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3ecd00dea7f19747566b11e3e2a545c97d5f8194 Gerrit-Change-Number: 15911 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR](branch-1.12.x) docs: add Ranger integration
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15910 Change subject: docs: add Ranger integration .. docs: add Ranger integration Staged version here: https://github.com/haohaoc/kudu/blob/ranger-docs/docs/security.adoc Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2 Reviewed-on: http://gerrit.cloudera.org:8080/15897 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke (cherry picked from commit a961fbc7cac61737d03a0c9cf8898199a101f67e) --- M docs/kudu_impala_integration.adoc M docs/security.adoc 2 files changed, 218 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/15910/1 -- To view, visit http://gerrit.cloudera.org:8080/15910 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.12.x Gerrit-MessageType: newchange Gerrit-Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2 Gerrit-Change-Number: 15910 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[kudu-CR] docs: add Ranger integration
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15897 ) Change subject: docs: add Ranger integration .. docs: add Ranger integration Staged version here: https://github.com/haohaoc/kudu/blob/ranger-docs/docs/security.adoc Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2 Reviewed-on: http://gerrit.cloudera.org:8080/15897 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke --- M docs/kudu_impala_integration.adoc M docs/security.adoc 2 files changed, 218 insertions(+), 17 deletions(-) Approvals: Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2 Gerrit-Change-Number: 15897 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] docs: add Ranger integration
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15897 ) Change subject: docs: add Ranger integration .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc File docs/security.adoc: http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc@506 PS2, Line 506: [[privilege-caching]] : === Kudu Master Caching for Sentry : : To avoid overwhelming Sentry with requests to fetch user privileges, the Kudu : master can be configured to cache user privileges. A by-product of this caching : is that when privileges are changed in Sentry, they may not be reflected in : Kudu for a configurable amount of time, defined by the following Kudu master : configurations: : : `--sentry_privileges_cache_ttl_factor * --authz_token_validity_interval_secs` : : The default value is fifty minutes. If privilege updates need to be reflected : in Kudu sooner than this, the Kudu CLI tool can be used to invalidate the : cached privileges to force Kudu to fetch new ones from Sentry: : : [source,bash] : : kudu master authz_cache reset > We need these docs to land in master to exist going forward. We can and sho Ack -- To view, visit http://gerrit.cloudera.org:8080/15897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2 Gerrit-Change-Number: 15897 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 May 2020 18:21:24 + Gerrit-HasComments: Yes
[kudu-CR] WIP [tools] multiple tablet ids in 'local replica delete'
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15909 ) Change subject: WIP [tools] multiple tablet ids in 'local_replica delete' .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/15909/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15909/1//COMMIT_MSG@10 PS1, Line 10: specified and processed at once > Right now those identifiers have to be specified as a comma-separated list Good points. Perhaps they could be follow-up changes. http://gerrit.cloudera.org:8080/#/c/15909/1/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/15909/1/src/kudu/tools/kudu-tool-test.cc@2771 PS1, Line 2771: const not needed with constexpr http://gerrit.cloudera.org:8080/#/c/15909/1/src/kudu/tools/tool_action_common.cc File src/kudu/tools/tool_action_common.cc: http://gerrit.cloudera.org:8080/#/c/15909/1/src/kudu/tools/tool_action_common.cc@199 PS1, Line 199: List lower case list -- To view, visit http://gerrit.cloudera.org:8080/15909 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If0e509d1775be2a728e4e3b10c724c1f15a96ec1 Gerrit-Change-Number: 15909 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 May 2020 18:00:07 + Gerrit-HasComments: Yes
[kudu-CR] docs: add Ranger integration
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15897 ) Change subject: docs: add Ranger integration .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc File docs/security.adoc: http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc@506 PS2, Line 506: [[privilege-caching]] : === Kudu Master Caching for Sentry : : To avoid overwhelming Sentry with requests to fetch user privileges, the Kudu : master can be configured to cache user privileges. A by-product of this caching : is that when privileges are changed in Sentry, they may not be reflected in : Kudu for a configurable amount of time, defined by the following Kudu master : configurations: : : `--sentry_privileges_cache_ttl_factor * --authz_token_validity_interval_secs` : : The default value is fifty minutes. If privilege updates need to be reflected : in Kudu sooner than this, the Kudu CLI tool can be used to invalidate the : cached privileges to force Kudu to fetch new ones from Sentry: : : [source,bash] : : kudu master authz_cache reset > Wait, but this changelist is for master branch, not kudu-1.12. That's why We need these docs to land in master to exist going forward. We can and should backport to 1.12 as well. A separate patch can remove the Sentry docs before the 1.13 release. -- To view, visit http://gerrit.cloudera.org:8080/15897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2 Gerrit-Change-Number: 15897 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 May 2020 17:40:02 + Gerrit-HasComments: Yes
[kudu-CR] docs: add Ranger integration
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15897 ) Change subject: docs: add Ranger integration .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc File docs/security.adoc: http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc@506 PS2, Line 506: [[privilege-caching]] : === Kudu Master Caching for Sentry : : To avoid overwhelming Sentry with requests to fetch user privileges, the Kudu : master can be configured to cache user privileges. A by-product of this caching : is that when privileges are changed in Sentry, they may not be reflected in : Kudu for a configurable amount of time, defined by the following Kudu master : configurations: : : `--sentry_privileges_cache_ttl_factor * --authz_token_validity_interval_secs` : : The default value is fifty minutes. If privilege updates need to be reflected : in Kudu sooner than this, the Kudu CLI tool can be used to invalidate the : cached privileges to force Kudu to fetch new ones from Sentry: : : [source,bash] : : kudu master authz_cache reset > yes, that is for the next release (1.13.0). This release still supports bot Wait, but this changelist is for master branch, not kudu-1.12. That's why I asked. Is that simply the wrong target branch for this patch? -- To view, visit http://gerrit.cloudera.org:8080/15897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2 Gerrit-Change-Number: 15897 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 May 2020 16:15:43 + Gerrit-HasComments: Yes
[kudu-CR] [KUDU-3116] Enhance KuduContext row operation metrics
Hello Kudu Jenkins, Andrew Wong, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15882 to look at the new patch set (#4). Change subject: [KUDU-3116] Enhance KuduContext row operation metrics .. [KUDU-3116] Enhance KuduContext row operation metrics Adds the ability to track operation counts per table. Introduces the MapAccumulator to track these metrics in a single accumulator per operation type. Change-Id: Ie66dab95041310c27ef62dacccbcc0977a84857e --- M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala A java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/MapAccumulator.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala 3 files changed, 123 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/15882/4 -- To view, visit http://gerrit.cloudera.org:8080/15882 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie66dab95041310c27ef62dacccbcc0977a84857e Gerrit-Change-Number: 15882 Gerrit-PatchSet: 4 Gerrit-Owner: Brian McDevitt Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Brian McDevitt Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] docs: add Ranger integration
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15897 ) Change subject: docs: add Ranger integration .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc File docs/security.adoc: http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc@506 PS2, Line 506: [[privilege-caching]] : === Kudu Master Caching for Sentry : : To avoid overwhelming Sentry with requests to fetch user privileges, the Kudu : master can be configured to cache user privileges. A by-product of this caching : is that when privileges are changed in Sentry, they may not be reflected in : Kudu for a configurable amount of time, defined by the following Kudu master : configurations: : : `--sentry_privileges_cache_ttl_factor * --authz_token_validity_interval_secs` : : The default value is fifty minutes. If privilege updates need to be reflected : in Kudu sooner than this, the Kudu CLI tool can be used to invalidate the : cached privileges to force Kudu to fetch new ones from Sentry: : : [source,bash] : : kudu master authz_cache reset > Is this and other Sentry-related pieces relevant at all after removing Sent yes, that is for the next release (1.13.0). This release still supports both. -- To view, visit http://gerrit.cloudera.org:8080/15897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2 Gerrit-Change-Number: 15897 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 May 2020 13:37:32 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3112. Fix WaitForBind method for checking service status
Hello Tidy Bot, Alexey Serbin, Attila Bukor, Kudu Jenkins, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15893 to look at the new patch set (#4). Change subject: KUDU-3112. Fix WaitForBind method for checking service status .. KUDU-3112. Fix WaitForBind method for checking service status The WaitForBind method is used to check a service status according to outputs of 'lsof' command, in this method, it will wait for the service listening at a port number. If a the outputs of 'lsof' command includes 'n*:', it means the service listen all the addresses on the port number, but when we pass a specific address to the method, e.g. 127.0.0.1, and the 'lsof' command show 'n*:', this also raise error. Change-Id: Ib79297e0eb59cc96a91c6e301f6a70ba123f4644 --- M src/kudu/util/test_util.cc 1 file changed, 18 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/15893/4 -- To view, visit http://gerrit.cloudera.org:8080/15893 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib79297e0eb59cc96a91c6e301f6a70ba123f4644 Gerrit-Change-Number: 15893 Gerrit-PatchSet: 4 Gerrit-Owner: liusheng Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: liusheng
[kudu-CR] util: quell warning in sockaddr.cc
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15908 ) Change subject: util: quell warning in sockaddr.cc .. util: quell warning in sockaddr.cc Building in DEBUG mode, I would see the following warning. [397/1515] Building CXX object src/kudu/util/CMakeFiles/kudu_util.dir/net/sockaddr.cc.o ../../src/kudu/util/net/sockaddr.cc: In member function ‘std::string kudu::Sockaddr::UnixDomainPath() const’: ../../src/kudu/util/net/sockaddr.cc:152:1: warning: control reaches end of non-void function [-Wreturn-type] } ^ [509/1515] Building CXX object src/kudu/util/CMakeFiles/kudu_util_exported.dir/net/sockaddr.cc.o ../../src/kudu/util/net/sockaddr.cc: In member function ‘std::string kudu::Sockaddr::UnixDomainPath() const’: ../../src/kudu/util/net/sockaddr.cc:152:1: warning: control reaches end of non-void function [-Wreturn-type] } ^ Change-Id: I97d75b5b917b2e562c7a97884e7bd93360c7a0e7 Reviewed-on: http://gerrit.cloudera.org:8080/15908 Reviewed-by: Bankim Bhavsar Reviewed-by: Alexey Serbin Tested-by: Andrew Wong --- M src/kudu/util/net/sockaddr.cc 1 file changed, 2 insertions(+), 0 deletions(-) Approvals: Bankim Bhavsar: Looks good to me, approved Alexey Serbin: Looks good to me, approved Andrew Wong: Verified -- To view, visit http://gerrit.cloudera.org:8080/15908 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I97d75b5b917b2e562c7a97884e7bd93360c7a0e7 Gerrit-Change-Number: 15908 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] util: quell warning in sockaddr.cc
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15908 ) Change subject: util: quell warning in sockaddr.cc .. Patch Set 2: Verified+1 Unrelated failure. -- To view, visit http://gerrit.cloudera.org:8080/15908 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I97d75b5b917b2e562c7a97884e7bd93360c7a0e7 Gerrit-Change-Number: 15908 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 May 2020 07:38:52 + Gerrit-HasComments: No
[kudu-CR] util: quell warning in sockaddr.cc
Andrew Wong has removed a vote on this change. Change subject: util: quell warning in sockaddr.cc .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15908 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I97d75b5b917b2e562c7a97884e7bd93360c7a0e7 Gerrit-Change-Number: 15908 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3112. Fix WaitForBind method for checking service status
liusheng has posted comments on this change. (
http://gerrit.cloudera.org:8080/15893 )
Change subject: KUDU-3112. Fix WaitForBind method for checking service status
..
Patch Set 3:
(5 comments)
Thanks a lot for you review, Alexey Serbin.
http://gerrit.cloudera.org:8080/#/c/15893/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15893/3//COMMIT_MSG@13
PS3, Line 13: but when we pass a specific address to the method,
: e.g. 127.0.0.1, and the 'lsof' command show 'n*:'
> Right, and there was an idea to find the appropriate socket. I.e., if a pr
I have also tried to find why Ranger starts listening on all interfaces than
127.0.0.1, but didn't find the reason. I have tried locally many times, it is
randomly but frequently occurred
http://gerrit.cloudera.org:8080/#/c/15893/3/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:
http://gerrit.cloudera.org:8080/#/c/15893/3/src/kudu/util/test_util.cc@445
PS3, Line 445: string all_pattern = "n*:";
> nit: add 'static const'
Done
http://gerrit.cloudera.org:8080/#/c/15893/3/src/kudu/util/test_util.cc@446
PS3, Line 446: string addr_pattern = (!addr || *addr == "0.0.0.0") ? "" :
Substitute("n$0:", *addr);
> I think it would be simpler if we had:
yes, this looks more pretty, thank you.
http://gerrit.cloudera.org:8080/#/c/15893/3/src/kudu/util/test_util.cc@446
PS3, Line 446: string addr_pattern = (!addr || *addr == "0.0.0.0") ? "" :
Substitute("n$0:", *addr);
> nit: add 'const'
Done
http://gerrit.cloudera.org:8080/#/c/15893/3/src/kudu/util/test_util.cc@458
PS3, Line 458: if (!cur_line.contains("->")) {
: if (HasPrefixString(cur_line.ToString(), all_pattern))
{
: cur_line.remove_prefix(all_pattern.size());
: } else if ((!addr_pattern.empty()) &&
: HasPrefixString(cur_line.ToString(),
addr_pattern)) {
: cur_line.remove_prefix(addr_pattern.size());
: } else {
: continue;
: }
> Would it be easier to read if it were written as:
yes, this looks more clearly, thank you.
--
To view, visit http://gerrit.cloudera.org:8080/15893
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib79297e0eb59cc96a91c6e301f6a70ba123f4644
Gerrit-Change-Number: 15893
Gerrit-PatchSet: 3
Gerrit-Owner: liusheng
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: liusheng
Gerrit-Comment-Date: Wed, 13 May 2020 06:58:28 +
Gerrit-HasComments: Yes
[kudu-CR] WIP [tools] multiple tablet ids in 'local replica delete'
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15909 ) Change subject: WIP [tools] multiple tablet ids in 'local_replica delete' .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15909/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15909/1//COMMIT_MSG@10 PS1, Line 10: specified and processed at once Right now those identifiers have to be specified as a comma-separated list (CSV), but I think it might be useful to add functionality to read those from a file: there is a limit on command-line length, so reading them from a file might be an option if a really long list of identifiers is used. Also, it might be useful to add an --ignore_non_existent option so the non-existent tablet replicas are ignored and the operation continues (instead of failing and not moving forward with the rest of tablet identifiers). -- To view, visit http://gerrit.cloudera.org:8080/15909 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If0e509d1775be2a728e4e3b10c724c1f15a96ec1 Gerrit-Change-Number: 15909 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 May 2020 06:25:34 + Gerrit-HasComments: Yes
[kudu-CR] [master] add 'runtime' tag for master ts rpc timeout ms
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15906 ) Change subject: [master] add 'runtime' tag for master_ts_rpc_timeout_ms .. [master] add 'runtime' tag for master_ts_rpc_timeout_ms The --master_ts_rpc_timeout_ms flag has the runtime behavior already. This patch simply adds the corresponding tag so it's not necessary to use the --force option when using `kudu master set_flag`. Change-Id: Icd658c1a2085d71c48648e9caa57e39d37152f56 Reviewed-on: http://gerrit.cloudera.org:8080/15906 Tested-by: Kudu Jenkins Reviewed-by: Bankim Bhavsar --- M src/kudu/master/catalog_manager.cc 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: Kudu Jenkins: Verified Bankim Bhavsar: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15906 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Icd658c1a2085d71c48648e9caa57e39d37152f56 Gerrit-Change-Number: 15906 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP [tools] multiple tablet ids in 'local replica delete'
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15909 Change subject: WIP [tools] multiple tablet ids in 'local_replica delete' .. WIP [tools] multiple tablet ids in 'local_replica delete' This patch updates the 'local_replica delete' tool to allow for multiple tablet identifiers to be specified and processed at once. The rationale behind this change is that opening tablet server's metadata takes significant time, and it's possible to reduce the overall latency if removing multiple tablet replicas at once after the metadata has already been opened. A new test to verify the new functionality has been added as well. WIP: * the test fails with assertion somewhere in ThreadPool, need to clarify on that Change-Id: If0e509d1775be2a728e4e3b10c724c1f15a96ec1 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h M src/kudu/tools/tool_action_local_replica.cc 4 files changed, 84 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/15909/1 -- To view, visit http://gerrit.cloudera.org:8080/15909 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If0e509d1775be2a728e4e3b10c724c1f15a96ec1 Gerrit-Change-Number: 15909 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
