[kudu-CR] [util] Import "Or" function to BlockBloomFilter from Impala

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

Change subject: [util] Import "Or" function to BlockBloomFilter from Impala
..


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/15373/1/src/kudu/util/block_bloom_filter-test.cc
File src/kudu/util/block_bloom_filter-test.cc:

http://gerrit.cloudera.org:8080/#/c/15373/1/src/kudu/util/block_bloom_filter-test.cc@304
PS1, Line 304:   for (int i = 11; i < 50; ++i) ASSERT_TRUE(bf1->Find(i));
if this fails, how to know what 'i' it was?


http://gerrit.cloudera.org:8080/#/c/15373/1/src/kudu/util/block_bloom_filter.h
File src/kudu/util/block_bloom_filter.h:

http://gerrit.cloudera.org:8080/#/c/15373/1/src/kudu/util/block_bloom_filter.h@153
PS1, Line 153: Or
For more elegant syntax alternative, maybe introduce

  BlockBloomFilter& operator |=(const BlockBloomFilter& other)

as well?


http://gerrit.cloudera.org:8080/#/c/15373/1/src/kudu/util/block_bloom_filter.h@229
PS1, Line 229: AVX
AVX2 ?


http://gerrit.cloudera.org:8080/#/c/15373/1/src/kudu/util/block_bloom_filter.cc
File src/kudu/util/block_bloom_filter.cc:

http://gerrit.cloudera.org:8080/#/c/15373/1/src/kudu/util/block_bloom_filter.cc@287
PS1, Line 287: DCHECK_NE(, kAlwaysTrueFilter);
Why is this disallowed?  Could you add a short explanation?


http://gerrit.cloudera.org:8080/#/c/15373/1/src/kudu/util/block_bloom_filter.cc@291
PS1, Line 291: DCHECK_EQ
BTW, is this Or() method protected from inputs from the wild that might not 
come under radar in DEBUG case?  In other words, why DCHECK_EQ() is enough and 
CHECK_EQ() is not needed here?


http://gerrit.cloudera.org:8080/#/c/15373/1/src/kudu/util/block_bloom_filter_avx2.cc
File src/kudu/util/block_bloom_filter_avx2.cc:

http://gerrit.cloudera.org:8080/#/c/15373/1/src/kudu/util/block_bloom_filter_avx2.cc@86
PS1, Line 86: DCHECK_EQ
Is it guaranteed that debug builds provide enough coverage to make sure n % 
kAVXRegisterBytes == 0 in release builds?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe5f9311f73dcff883dd2cce18fd558e7d57d14f
Gerrit-Change-Number: 15373
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Bankim Bhavsar 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Fri, 06 Mar 2020 07:28:08 +
Gerrit-HasComments: Yes


[kudu-CR] [util] Add support for 32 & 64 byte alignment to Arena allocator

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

Change subject: [util] Add support for 32 & 64 byte alignment to Arena allocator
..


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/15372/1/src/kudu/util/block_bloom_filter-test.cc
File src/kudu/util/block_bloom_filter-test.cc:

http://gerrit.cloudera.org:8080/#/c/15372/1/src/kudu/util/block_bloom_filter-test.cc@118
PS1, Line 118: sometimes
How often 'sometimes' is?  Since it's quite simple scenario, is it possible to 
have 100% reproduction rate with prior non-patched code?


http://gerrit.cloudera.org:8080/#/c/15372/1/src/kudu/util/block_bloom_filter.cc
File src/kudu/util/block_bloom_filter.cc:

http://gerrit.cloudera.org:8080/#/c/15372/1/src/kudu/util/block_bloom_filter.cc@311
PS1, Line 311:   static_assert(CACHELINE_SIZE >= 32,
 : "For AVX operations, need buffers to be 32-bytes 
aligned or higher");
 :   *ptr = arena_->AllocateBytesAligned(bytes, CACHELINE_SIZE);
nit: I would expect it's enough to have just one static_assert (i.e. 
compile-time assert) of this sort since every method is processed by compiler?  
Maybe, move it into renaBlockBloomFilterBufferAllocator constructor?


http://gerrit.cloudera.org:8080/#/c/15372/1/src/kudu/util/memory/arena-test.cc
File src/kudu/util/memory/arena-test.cc:

http://gerrit.cloudera.org:8080/#/c/15372/1/src/kudu/util/memory/arena-test.cc@83
PS1, Line 83: upto
nit: up to


http://gerrit.cloudera.org:8080/#/c/15372/1/src/kudu/util/memory/arena-test.cc@115
PS1, Line 115: ThreadSafeArena
Does it make sense to run multiple threads performing those test allocations in 
case of ThreadSafeArena?


http://gerrit.cloudera.org:8080/#/c/15372/1/src/kudu/util/memory/arena-test.cc@127
PS1, Line 127:   int alignment = 32;
constexpr?


http://gerrit.cloudera.org:8080/#/c/15372/1/src/kudu/util/memory/arena.h
File src/kudu/util/memory/arena.h:

http://gerrit.cloudera.org:8080/#/c/15372/1/src/kudu/util/memory/arena.h@397
PS1, Line 397: const
nit: remove this and next useless 'const' specifier to be uniform with 
AllocateBytesFallback()?


http://gerrit.cloudera.org:8080/#/c/15372/1/src/kudu/util/memory/arena.h@409
PS1, Line 409: Atomic32
Is 32-bit number is always enough to store addresses here even for 64-bit 
architectures?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib665115fa0fc262a8b76c48f52947dedb84be2a7
Gerrit-Change-Number: 15372
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Bankim Bhavsar 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 06 Mar 2020 06:13:33 +
Gerrit-HasComments: Yes


[kudu-CR] util: remove all gscoped ptr

2020-03-05 Thread Adar Dembo (Code Review)
Adar Dembo has removed a vote on this change.

Change subject: util: remove all gscoped_ptr
..


Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/15377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
Gerrit-Change-Number: 15377
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)


[kudu-CR] util: remove all gscoped ptr

2020-03-05 Thread Adar Dembo (Code Review)
Adar Dembo has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/15377 )

Change subject: util: remove all gscoped_ptr
..

util: remove all gscoped_ptr

Except from callback_bind-test.cc, which tests gscoped_ptr functionality.

Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
Reviewed-on: http://gerrit.cloudera.org:8080/15377
Reviewed-by: Andrew Wong 
Tested-by: Adar Dembo 
---
M src/kudu/client/batcher.cc
M src/kudu/client/batcher.h
M src/kudu/client/scan_predicate-internal.h
M src/kudu/tablet/concurrent_btree.h
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/table_scanner.h
M src/kudu/util/bloom_filter.h
M src/kudu/util/compression/compression-test.cc
M src/kudu/util/crc-test.cc
M src/kudu/util/debug/trace_event_impl.cc
M src/kudu/util/debug/trace_event_impl.h
M src/kudu/util/env_posix.cc
M src/kudu/util/hdr_histogram.cc
M src/kudu/util/hdr_histogram.h
M src/kudu/util/inline_slice-test.cc
M src/kudu/util/kernel_stack_watchdog.h
M src/kudu/util/memory/arena.h
M src/kudu/util/memory/memory.h
M src/kudu/util/metrics.h
M src/kudu/util/mutex.h
M src/kudu/util/nvm_cache.cc
M src/kudu/util/nvm_cache.h
M src/kudu/util/object_pool-test.cc
M src/kudu/util/object_pool.h
M src/kudu/util/path_util.cc
M src/kudu/util/path_util.h
M src/kudu/util/protoc-gen-insertions.cc
M src/kudu/util/trace.h
M src/kudu/util/user.cc
29 files changed, 167 insertions(+), 189 deletions(-)

Approvals:
  Andrew Wong: Looks good to me, approved
  Adar Dembo: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
Gerrit-Change-Number: 15377
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)


[kudu-CR] util: remove all gscoped ptr

2020-03-05 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15377 )

Change subject: util: remove all gscoped_ptr
..


Patch Set 3: Verified+1

Overriding Jenkins, another dist-test temp dir issue.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
Gerrit-Change-Number: 15377
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 06 Mar 2020 05:12:10 +
Gerrit-HasComments: No


[kudu-CR] util: remove all gscoped ptr

2020-03-05 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15377 )

Change subject: util: remove all gscoped_ptr
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
Gerrit-Change-Number: 15377
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 06 Mar 2020 04:57:42 +
Gerrit-HasComments: No


[kudu-CR] util: remove all gscoped ptr

2020-03-05 Thread Adar Dembo (Code Review)
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,

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

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

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

Change subject: util: remove all gscoped_ptr
..

util: remove all gscoped_ptr

Except from callback_bind-test.cc, which tests gscoped_ptr functionality.

Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
---
M src/kudu/client/batcher.cc
M src/kudu/client/batcher.h
M src/kudu/client/scan_predicate-internal.h
M src/kudu/tablet/concurrent_btree.h
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/table_scanner.h
M src/kudu/util/bloom_filter.h
M src/kudu/util/compression/compression-test.cc
M src/kudu/util/crc-test.cc
M src/kudu/util/debug/trace_event_impl.cc
M src/kudu/util/debug/trace_event_impl.h
M src/kudu/util/env_posix.cc
M src/kudu/util/hdr_histogram.cc
M src/kudu/util/hdr_histogram.h
M src/kudu/util/inline_slice-test.cc
M src/kudu/util/kernel_stack_watchdog.h
M src/kudu/util/memory/arena.h
M src/kudu/util/memory/memory.h
M src/kudu/util/metrics.h
M src/kudu/util/mutex.h
M src/kudu/util/nvm_cache.cc
M src/kudu/util/nvm_cache.h
M src/kudu/util/object_pool-test.cc
M src/kudu/util/object_pool.h
M src/kudu/util/path_util.cc
M src/kudu/util/path_util.h
M src/kudu/util/protoc-gen-insertions.cc
M src/kudu/util/trace.h
M src/kudu/util/user.cc
29 files changed, 167 insertions(+), 189 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
Gerrit-Change-Number: 15377
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)


[kudu-CR] util: remove all gscoped ptr

2020-03-05 Thread Adar Dembo (Code Review)
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,

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

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

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

Change subject: util: remove all gscoped_ptr
..

util: remove all gscoped_ptr

Except from callback_bind-test.cc, which tests gscoped_ptr functionality.

Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
---
M src/kudu/client/batcher.cc
M src/kudu/client/batcher.h
M src/kudu/client/scan_predicate-internal.h
M src/kudu/tablet/concurrent_btree.h
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/table_scanner.h
M src/kudu/util/bloom_filter.h
M src/kudu/util/compression/compression-test.cc
M src/kudu/util/crc-test.cc
M src/kudu/util/debug/trace_event_impl.cc
M src/kudu/util/debug/trace_event_impl.h
M src/kudu/util/env_posix.cc
M src/kudu/util/hdr_histogram.cc
M src/kudu/util/hdr_histogram.h
M src/kudu/util/inline_slice-test.cc
M src/kudu/util/kernel_stack_watchdog.h
M src/kudu/util/memory/arena.h
M src/kudu/util/memory/memory.h
M src/kudu/util/metrics.h
M src/kudu/util/mutex.h
M src/kudu/util/nvm_cache.cc
M src/kudu/util/nvm_cache.h
M src/kudu/util/object_pool-test.cc
M src/kudu/util/object_pool.h
M src/kudu/util/path_util.cc
M src/kudu/util/path_util.h
M src/kudu/util/protoc-gen-insertions.cc
M src/kudu/util/trace.h
M src/kudu/util/user.cc
29 files changed, 167 insertions(+), 188 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
Gerrit-Change-Number: 15377
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)


[kudu-CR] util: remove all gscoped ptr

2020-03-05 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15377 )

Change subject: util: remove all gscoped_ptr
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/15377/1/src/kudu/tablet/concurrent_btree.h
File src/kudu/tablet/concurrent_btree.h:

http://gerrit.cloudera.org:8080/#/c/15377/1/src/kudu/tablet/concurrent_btree.h@1101
PS1, Line 1101:
> nit: maybe, drop the extra space while you are at it?
Done


http://gerrit.cloudera.org:8080/#/c/15377/1/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

http://gerrit.cloudera.org:8080/#/c/15377/1/src/kudu/util/env_posix.cc@1522
PS1, Line 1522: []
> Actually nevermind. Interesting that this worked before.
Yeah it was pretty weird.


http://gerrit.cloudera.org:8080/#/c/15377/1/src/kudu/util/user.cc
File src/kudu/util/user.cc:

http://gerrit.cloudera.org:8080/#/c/15377/1/src/kudu/util/user.cc@60
PS1, Line 60:   size_t bufsize = retval > 0 ? retval : 16384;
:
:   unique_ptr buf(new char[bufsize]);
> nit: is it possible to allocate 16K buffer on stack and then use it or it's
Well we still need to account for the possibility that sysconf() returns a 
value much greater than 16K. It's very unlikely, but I don't know if it's a 
guarantee.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
Gerrit-Change-Number: 15377
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 06 Mar 2020 04:15:33 +
Gerrit-HasComments: Yes


[kudu-CR] util: remove all gscoped ptr

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

Change subject: util: remove all gscoped_ptr
..


Patch Set 1: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15377/1/src/kudu/tablet/concurrent_btree.h
File src/kudu/tablet/concurrent_btree.h:

http://gerrit.cloudera.org:8080/#/c/15377/1/src/kudu/tablet/concurrent_btree.h@1101
PS1, Line 1101:
nit: maybe, drop the extra space while you are at it?


http://gerrit.cloudera.org:8080/#/c/15377/1/src/kudu/util/user.cc
File src/kudu/util/user.cc:

http://gerrit.cloudera.org:8080/#/c/15377/1/src/kudu/util/user.cc@60
PS1, Line 60:   size_t bufsize = retval > 0 ? retval : 16384;
:
:   unique_ptr buf(new char[bufsize]);
nit: is it possible to allocate 16K buffer on stack and then use it or it's 
part instead of calling new?  Adding CHECK_LE(retval, 16384) for sanity check.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
Gerrit-Change-Number: 15377
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 06 Mar 2020 03:52:02 +
Gerrit-HasComments: Yes


[kudu-CR] [tserver] report the newly bootstrapped tablet after OpenTablet completes

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

Change subject: [tserver] report the newly bootstrapped tablet after OpenTablet 
completes
..


Patch Set 12: Code-Review+2

Looks good to me, the only question is whether to add a regression test like it 
was in PS1: is it possible to come up with a test scenario that would expose 
the issue that was problematic prior to this patch?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I591dc8daceb81f9e800098be77c928d391cdc00a
Gerrit-Change-Number: 15307
Gerrit-PatchSet: 12
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Fri, 06 Mar 2020 03:13:22 +
Gerrit-HasComments: No


[kudu-CR] util: remove all gscoped ptr

2020-03-05 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15377 )

Change subject: util: remove all gscoped_ptr
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15377/1/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

http://gerrit.cloudera.org:8080/#/c/15377/1/src/kudu/util/env_posix.cc@1522
PS1, Line 1522: []
> Why this change?
Actually nevermind. Interesting that this worked before.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
Gerrit-Change-Number: 15377
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 06 Mar 2020 02:24:03 +
Gerrit-HasComments: Yes


[kudu-CR] util: remove all gscoped ptr

2020-03-05 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15377 )

Change subject: util: remove all gscoped_ptr
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
Gerrit-Change-Number: 15377
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 06 Mar 2020 02:22:43 +
Gerrit-HasComments: No


[kudu-CR] util: remove all gscoped ptr

2020-03-05 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15377 )

Change subject: util: remove all gscoped_ptr
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15377/1/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

http://gerrit.cloudera.org:8080/#/c/15377/1/src/kudu/util/env_posix.cc@1522
PS1, Line 1522: []
Why this change?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
Gerrit-Change-Number: 15377
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 06 Mar 2020 02:22:31 +
Gerrit-HasComments: Yes


[kudu-CR] subprocess: add server metric for queue size

2020-03-05 Thread Andrew Wong (Code Review)
Andrew Wong has removed a vote on this change.

Change subject: subprocess: add server metric for queue size
..


Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/15375
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I62d2d5727dca3f54b59ff1044431326cbdde855d
Gerrit-Change-Number: 15375
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)


[kudu-CR] subprocess: add server metric for queue size

2020-03-05 Thread Andrew Wong (Code Review)
Andrew Wong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/15375 )

Change subject: subprocess: add server metric for queue size
..

subprocess: add server metric for queue size

Unlike the Java in/outbound queues, the C++ queues are measured in
bytes. This exposes histogram metrics for the sizes of the inbound and
outbound queue at Put-time.

This also adds a method to get the logical size of a blocking queue.

Change-Id: I62d2d5727dca3f54b59ff1044431326cbdde855d
Reviewed-on: http://gerrit.cloudera.org:8080/15375
Reviewed-by: Adar Dembo 
Tested-by: Andrew Wong 
---
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/echo_subprocess.cc
M src/kudu/subprocess/server.cc
M src/kudu/subprocess/server.h
M src/kudu/subprocess/subprocess_proxy-test.cc
M src/kudu/util/blocking_queue-test.cc
M src/kudu/util/blocking_queue.h
7 files changed, 115 insertions(+), 53 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Andrew Wong: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I62d2d5727dca3f54b59ff1044431326cbdde855d
Gerrit-Change-Number: 15375
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)


[kudu-CR] subprocess: add server metric for queue size

2020-03-05 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15375 )

Change subject: subprocess: add server metric for queue size
..


Patch Set 2: Verified+1

Flaky: HmsSentryConfigurations/MasterFailoverTest.TestMasterPermanentFailure


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I62d2d5727dca3f54b59ff1044431326cbdde855d
Gerrit-Change-Number: 15375
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 06 Mar 2020 02:03:42 +
Gerrit-HasComments: No


[kudu-CR] util: remove all gscoped ptr

2020-03-05 Thread Adar Dembo (Code Review)
Hello Alexey Serbin, Andrew Wong,

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

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

to review the following change.


Change subject: util: remove all gscoped_ptr
..

util: remove all gscoped_ptr

Except from callback_bind-test.cc, which tests gscoped_ptr functionality.

Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
---
M src/kudu/client/batcher.cc
M src/kudu/client/batcher.h
M src/kudu/client/scan_predicate-internal.h
M src/kudu/tablet/concurrent_btree.h
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/table_scanner.h
M src/kudu/util/bloom_filter.h
M src/kudu/util/compression/compression-test.cc
M src/kudu/util/crc-test.cc
M src/kudu/util/debug/trace_event_impl.cc
M src/kudu/util/debug/trace_event_impl.h
M src/kudu/util/env_posix.cc
M src/kudu/util/hdr_histogram.cc
M src/kudu/util/hdr_histogram.h
M src/kudu/util/inline_slice-test.cc
M src/kudu/util/kernel_stack_watchdog.h
M src/kudu/util/memory/arena.h
M src/kudu/util/memory/memory.h
M src/kudu/util/metrics.h
M src/kudu/util/mutex.h
M src/kudu/util/nvm_cache.cc
M src/kudu/util/nvm_cache.h
M src/kudu/util/object_pool-test.cc
M src/kudu/util/object_pool.h
M src/kudu/util/path_util.cc
M src/kudu/util/path_util.h
M src/kudu/util/protoc-gen-insertions.cc
M src/kudu/util/trace.h
M src/kudu/util/user.cc
29 files changed, 162 insertions(+), 185 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia110feea85b6ca43ba53d2d4783f47000794e196
Gerrit-Change-Number: 15377
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 


[kudu-CR] subprocess: add server metric for queue size

2020-03-05 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15375 )

Change subject: subprocess: add server metric for queue size
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I62d2d5727dca3f54b59ff1044431326cbdde855d
Gerrit-Change-Number: 15375
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 06 Mar 2020 01:15:15 +
Gerrit-HasComments: No


[kudu-CR] subprocess: add server metric for queue size

2020-03-05 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15375 )

Change subject: subprocess: add server metric for queue size
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15375/1/src/kudu/util/blocking_queue.h
File src/kudu/util/blocking_queue.h:

http://gerrit.cloudera.org:8080/#/c/15375/1/src/kudu/util/blocking_queue.h@277
PS1, Line 277:   const size_t max_size_;
> Could you declare this const?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I62d2d5727dca3f54b59ff1044431326cbdde855d
Gerrit-Change-Number: 15375
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 06 Mar 2020 01:13:48 +
Gerrit-HasComments: Yes


[kudu-CR] subprocess: add server metric for queue size

2020-03-05 Thread Andrew Wong (Code Review)
Hello Kudu Jenkins, Adar Dembo, Hao Hao,

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

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

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

Change subject: subprocess: add server metric for queue size
..

subprocess: add server metric for queue size

Unlike the Java in/outbound queues, the C++ queues are measured in
bytes. This exposes histogram metrics for the sizes of the inbound and
outbound queue at Put-time.

This also adds a method to get the logical size of a blocking queue.

Change-Id: I62d2d5727dca3f54b59ff1044431326cbdde855d
---
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/echo_subprocess.cc
M src/kudu/subprocess/server.cc
M src/kudu/subprocess/server.h
M src/kudu/subprocess/subprocess_proxy-test.cc
M src/kudu/util/blocking_queue-test.cc
M src/kudu/util/blocking_queue.h
7 files changed, 115 insertions(+), 53 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I62d2d5727dca3f54b59ff1044431326cbdde855d
Gerrit-Change-Number: 15375
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)


[kudu-CR] subprocess: add server metric for queue size

2020-03-05 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15375 )

Change subject: subprocess: add server metric for queue size
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15375/1/src/kudu/util/blocking_queue.h
File src/kudu/util/blocking_queue.h:

http://gerrit.cloudera.org:8080/#/c/15375/1/src/kudu/util/blocking_queue.h@277
PS1, Line 277:   size_t max_size_;
Could you declare this const?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I62d2d5727dca3f54b59ff1044431326cbdde855d
Gerrit-Change-Number: 15375
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 06 Mar 2020 01:08:25 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2972 Add Ranger authorization provider

2020-03-05 Thread Hao Hao (Code Review)
Hao Hao has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/15207 )

Change subject: KUDU-2972 Add Ranger authorization provider
..

KUDU-2972 Add Ranger authorization provider

Adds a new authz_provider implementation that uses Ranger and plugs it
in to catalog_manager. As of this patch this is considered experimental
and an experimental flag is needed to be set to enable it.

Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Reviewed-on: http://gerrit.cloudera.org:8080/15207
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
Reviewed-by: Hao Hao 
---
M src/kudu/master/CMakeLists.txt
M src/kudu/master/catalog_manager.cc
A src/kudu/master/ranger_authz_provider.cc
A src/kudu/master/ranger_authz_provider.h
M src/kudu/master/sentry_authz_provider.cc
5 files changed, 317 insertions(+), 2 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Adar Dembo: Looks good to me, but someone else must approve
  Hao Hao: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 30
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)


[kudu-CR] KUDU-2972 Add Ranger authorization provider

2020-03-05 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15207 )

Change subject: KUDU-2972 Add Ranger authorization provider
..


Patch Set 29: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 29
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 06 Mar 2020 00:12:18 +
Gerrit-HasComments: No


[kudu-CR] subprocess: add server metric for queue size

2020-03-05 Thread Andrew Wong (Code Review)
Hello Adar Dembo, Hao Hao,

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

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

to review the following change.


Change subject: subprocess: add server metric for queue size
..

subprocess: add server metric for queue size

Unlike the Java in/outbound queues, the C++ queues are measured in
bytes. This exposes histogram metrics for the sizes of the inbound and
outbound queue at Put-time.

This also adds a method to get the logical size of a blocking queue.

Change-Id: I62d2d5727dca3f54b59ff1044431326cbdde855d
---
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/echo_subprocess.cc
M src/kudu/subprocess/server.cc
M src/kudu/subprocess/server.h
M src/kudu/subprocess/subprocess_proxy-test.cc
M src/kudu/util/blocking_queue-test.cc
M src/kudu/util/blocking_queue.h
7 files changed, 114 insertions(+), 52 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I62d2d5727dca3f54b59ff1044431326cbdde855d
Gerrit-Change-Number: 15375
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Hao Hao 


[kudu-CR] [WIP] Mini Ranger + Postgres

2020-03-05 Thread Attila Bukor (Code Review)
Attila Bukor has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15374 )

Change subject: [WIP] Mini Ranger + Postgres
..


Patch Set 1:

> Uploaded patch set 1.

I'm getting close, but Ranger still fails to start. I'll investigate it further 
and fix it, but uploaded it in case anyone feels like playing around with it.

Sorry for this unreadable mess, this is the result of days of trial & error, 
I'll clean it up once it works.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iba40126aa60e0ecbc5ae10cc1328493194c345bc
Gerrit-Change-Number: 15374
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Anonymous Coward (314)
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 05 Mar 2020 23:41:25 +
Gerrit-HasComments: No


[kudu-CR] [WIP] Mini Ranger + Postgres

2020-03-05 Thread Attila Bukor (Code Review)
Hello Andrew Wong, Anonymous Coward (314), Adar Dembo,

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

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

to review the following change.


Change subject: [WIP] Mini Ranger + Postgres
..

[WIP] Mini Ranger + Postgres

Change-Id: Iba40126aa60e0ecbc5ae10cc1328493194c345bc
---
M src/kudu/mini-cluster/CMakeLists.txt
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/ranger/CMakeLists.txt
A src/kudu/ranger/mini_ranger-test.cc
A src/kudu/ranger/mini_ranger.cc
A src/kudu/ranger/mini_ranger.h
M src/kudu/util/subprocess.cc
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
13 files changed, 629 insertions(+), 3 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iba40126aa60e0ecbc5ae10cc1328493194c345bc
Gerrit-Change-Number: 15374
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Anonymous Coward (314)


[kudu-CR] KUDU-2972 Add Ranger authorization provider

2020-03-05 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15207 )

Change subject: KUDU-2972 Add Ranger authorization provider
..


Patch Set 29: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 29
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 05 Mar 2020 23:33:10 +
Gerrit-HasComments: No


[kudu-CR] [util] Import "Or" function to BlockBloomFilter from Impala

2020-03-05 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15373 )

Change subject: [util] Import "Or" function to BlockBloomFilter from Impala
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/15373/1/src/kudu/util/block_bloom_filter.h
File src/kudu/util/block_bloom_filter.h:

http://gerrit.cloudera.org:8080/#/c/15373/1/src/kudu/util/block_bloom_filter.h@161
PS1, Line 161:   static constexpr BlockBloomFilter* const kAlwaysTrueFilter = 
nullptr;
Does this need to be public? Why?


http://gerrit.cloudera.org:8080/#/c/15373/1/src/kudu/util/block_bloom_filter.cc
File src/kudu/util/block_bloom_filter.cc:

http://gerrit.cloudera.org:8080/#/c/15373/1/src/kudu/util/block_bloom_filter.cc@50
PS1, Line 50: constexpr BlockBloomFilter* const 
BlockBloomFilter::kAlwaysTrueFilter;
Does this need to be initialized to something meaningful? In the header it's 
null; is that intentional?


http://gerrit.cloudera.org:8080/#/c/15373/1/src/kudu/util/block_bloom_filter.cc@292
PS1, Line 292: reinterpret_cast(other.directory_)
Nit: push this to the next line so the symmetry with the second 
reinterpret_cast is more obvious?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe5f9311f73dcff883dd2cce18fd558e7d57d14f
Gerrit-Change-Number: 15373
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Bankim Bhavsar 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Thu, 05 Mar 2020 23:31:56 +
Gerrit-HasComments: Yes


[kudu-CR] [util] Add support for 32 & 64 byte alignment to Arena allocator

2020-03-05 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15372 )

Change subject: [util] Add support for 32 & 64 byte alignment to Arena allocator
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15372/1/src/kudu/util/memory/arena-test.cc
File src/kudu/util/memory/arena-test.cc:

http://gerrit.cloudera.org:8080/#/c/15372/1/src/kudu/util/memory/arena-test.cc@88
PS1, Line 88:   ASSERT_EQ(0, reinterpret_cast(ret) % alignment) 
<<
Should we also write to the memory, so that ASAN will catch any issues with the 
underlying allocation for the component?

Just make sure the test isn't dog slow in ASAN though.


http://gerrit.cloudera.org:8080/#/c/15372/1/src/kudu/util/memory/arena.h
File src/kudu/util/memory/arena.h:

http://gerrit.cloudera.org:8080/#/c/15372/1/src/kudu/util/memory/arena.h@433
PS1, Line 433:
 :   // Component start address "data_" is only guaranteed to be 
16-byte aligned with enough
 :   // bytes for the first request size plus any padding needed 
for alignment.
 :   // So to support alignment values greater than 16 bytes, align 
the destination address ptr
 :   // that'll be returned to the caller and not just the 
"offset_".
 :   const auto data_start_addr = 
reinterpret_cast(data_);
 :   size_t aligned = KUDU_ALIGN_UP((data_start_addr + offset_), 
alignment) - data_start_addr;
Could you refactor this into another inline function, so that the alignment 
calculation is done just once and the comment placement is clear?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib665115fa0fc262a8b76c48f52947dedb84be2a7
Gerrit-Change-Number: 15372
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Bankim Bhavsar 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 05 Mar 2020 23:25:29 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2972 Add Ranger authorization provider

2020-03-05 Thread Attila Bukor (Code Review)
Attila Bukor has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15207 )

Change subject: KUDU-2972 Add Ranger authorization provider
..


Patch Set 29:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15207/28/src/kudu/master/ranger_authz_provider.h
File src/kudu/master/ranger_authz_provider.h:

http://gerrit.cloudera.org:8080/#/c/15207/28/src/kudu/master/ranger_authz_provider.h@87
PS28, Line 87:   ranger::RangerClient client_;
> Don't need this since we're inheriting IsTrustedUser() from the base class.
Done


http://gerrit.cloudera.org:8080/#/c/15207/28/src/kudu/master/ranger_authz_provider.cc
File src/kudu/master/ranger_authz_provider.cc:

http://gerrit.cloudera.org:8080/#/c/15207/28/src/kudu/master/ranger_authz_provider.cc@31
PS28, Line 31:
 : DECLARE_string(ranger_config_path);
 :
 : namespace kudu {
> nit: can move this into the kudu namespace below
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 29
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 05 Mar 2020 22:30:04 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2972 Add Ranger authorization provider

2020-03-05 Thread Attila Bukor (Code Review)
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao,

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

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

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

Change subject: KUDU-2972 Add Ranger authorization provider
..

KUDU-2972 Add Ranger authorization provider

Adds a new authz_provider implementation that uses Ranger and plugs it
in to catalog_manager. As of this patch this is considered experimental
and an experimental flag is needed to be set to enable it.

Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
---
M src/kudu/master/CMakeLists.txt
M src/kudu/master/catalog_manager.cc
A src/kudu/master/ranger_authz_provider.cc
A src/kudu/master/ranger_authz_provider.h
M src/kudu/master/sentry_authz_provider.cc
5 files changed, 317 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/29
--
To view, visit http://gerrit.cloudera.org:8080/15207
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 29
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)


[kudu-CR] KUDU-2972 Add Ranger authorization provider

2020-03-05 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15207 )

Change subject: KUDU-2972 Add Ranger authorization provider
..


Patch Set 28: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15207/28/src/kudu/master/ranger_authz_provider.h
File src/kudu/master/ranger_authz_provider.h:

http://gerrit.cloudera.org:8080/#/c/15207/28/src/kudu/master/ranger_authz_provider.h@87
PS28, Line 87:   std::unordered_set trusted_users_;
Don't need this since we're inheriting IsTrustedUser() from the base class.


http://gerrit.cloudera.org:8080/#/c/15207/28/src/kudu/master/ranger_authz_provider.cc
File src/kudu/master/ranger_authz_provider.cc:

http://gerrit.cloudera.org:8080/#/c/15207/28/src/kudu/master/ranger_authz_provider.cc@31
PS28, Line 31:
 : namespace kudu {
 : class MetricEntity;
 : }  // namespace kudu
nit: can move this into the kudu namespace below



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 28
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 05 Mar 2020 22:14:48 +
Gerrit-HasComments: Yes


[kudu-CR] subprocess: add server-side metrics

2020-03-05 Thread Andrew Wong (Code Review)
Andrew Wong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/15367 )

Change subject: subprocess: add server-side metrics
..

subprocess: add server-side metrics

This introduces server-side timing information to the list of
subprocess-related metrics.

Change-Id: I51294741ff82bd47e64ceaba18a6d04ae0144179
Reviewed-on: http://gerrit.cloudera.org:8080/15367
Reviewed-by: Hao Hao 
Reviewed-by: Adar Dembo 
Tested-by: Andrew Wong 
---
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/echo_subprocess.cc
M src/kudu/subprocess/server.cc
M src/kudu/subprocess/server.h
M src/kudu/subprocess/subprocess_proxy-test.cc
5 files changed, 141 insertions(+), 48 deletions(-)

Approvals:
  Hao Hao: Looks good to me, but someone else must approve
  Adar Dembo: Looks good to me, approved
  Andrew Wong: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I51294741ff82bd47e64ceaba18a6d04ae0144179
Gerrit-Change-Number: 15367
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)


[kudu-CR] subprocess: add server-side metrics

2020-03-05 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15367 )

Change subject: subprocess: add server-side metrics
..


Patch Set 3: Verified+1

(1 comment)

Unrelated flake: security-itest

http://gerrit.cloudera.org:8080/#/c/15367/3/src/kudu/subprocess/echo_subprocess.cc
File src/kudu/subprocess/echo_subprocess.cc:

http://gerrit.cloudera.org:8080/#/c/15367/3/src/kudu/subprocess/echo_subprocess.cc@53
PS3, Line 53: METRIC_DEFINE_histogram(server, 
echo_server_outbound_queue_time_ms,
: "Echo server outbound queue time (ms)",
: kudu::MetricUnit::kMilliseconds,
: "Duration of time in ms spent in the Echo server's outbound 
request queue",
: kudu::MetricLevel::kInfo,
: 6LU, 1);
: METRIC_DEFINE_histogram(server, echo_server_inbound_queue_time_ms,
: "Echo server inbound queue time (ms)",
: kudu::MetricUnit::kMilliseconds,
: "Duration of time in ms spent in the Echo server's inbound 
response queue",
: kudu::MetricLevel::kInfo,
: 6LU, 1);
> Wondering why don't have inbound/outbound queue length metrics on the serve
Yeah, I wasn't sure between showing the length vs showing the size in bytes. 
I'm happy to include either at some point, but they're not currently available 
in BlockingQueue, so it's probably worth separating that out into its own patch 
if we do want one of those metrics.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51294741ff82bd47e64ceaba18a6d04ae0144179
Gerrit-Change-Number: 15367
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 05 Mar 2020 22:13:35 +
Gerrit-HasComments: Yes


[kudu-CR] subprocess: add server-side metrics

2020-03-05 Thread Andrew Wong (Code Review)
Andrew Wong has removed a vote on this change.

Change subject: subprocess: add server-side metrics
..


Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/15367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I51294741ff82bd47e64ceaba18a6d04ae0144179
Gerrit-Change-Number: 15367
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)


[kudu-CR] KUDU-2972 Add Ranger authorization provider

2020-03-05 Thread Attila Bukor (Code Review)
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao,

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

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

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

Change subject: KUDU-2972 Add Ranger authorization provider
..

KUDU-2972 Add Ranger authorization provider

Adds a new authz_provider implementation that uses Ranger and plugs it
in to catalog_manager. As of this patch this is considered experimental
and an experimental flag is needed to be set to enable it.

Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
---
M src/kudu/master/CMakeLists.txt
M src/kudu/master/catalog_manager.cc
A src/kudu/master/ranger_authz_provider.cc
A src/kudu/master/ranger_authz_provider.h
M src/kudu/master/sentry_authz_provider.cc
5 files changed, 321 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/28
--
To view, visit http://gerrit.cloudera.org:8080/15207
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 28
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)


[kudu-CR] Init kudu.write duration accumulator lazily

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

Change subject: Init kudu.write_duration accumulator lazily
..


Patch Set 1:

Yes, this patch should not be needed. See 
https://github.com/apache/kudu/commit/5f199c16d46fc6bd6003f00d4e56ba75923f192c 
for details.

Let me know if that patch isn't satisfactory.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I76dea33d652874f9cfcbb72a42ab3061fd1e6974
Gerrit-Change-Number: 15368
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 05 Mar 2020 21:37:00 +
Gerrit-HasComments: No


[kudu-CR] [util] Import "Or" function to BlockBloomFilter from Impala

2020-03-05 Thread Bankim Bhavsar (Code Review)
Bankim Bhavsar has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/15373


Change subject: [util] Import "Or" function to BlockBloomFilter from Impala
..

[util] Import "Or" function to BlockBloomFilter from Impala

Impala will be switching to using the Block Bloom filter from kudu-util.
"Or" function was missing and this change adds it.

Note that original implementation for OrEqualArrayAvx() in Impala is
targeted for AVX and not AVX2, however AVX2 is super-set of AVX instructions
and there is already provision in the Block Bloom filter to separate out
AVX2 v/s non-AVX2 (SSE) code. Hence don't see need to add separate AVX
specific file/implementation.

Change-Id: Ibe5f9311f73dcff883dd2cce18fd558e7d57d14f
---
M src/kudu/util/block_bloom_filter-test.cc
M src/kudu/util/block_bloom_filter.cc
M src/kudu/util/block_bloom_filter.h
M src/kudu/util/block_bloom_filter_avx2.cc
4 files changed, 101 insertions(+), 1 deletion(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibe5f9311f73dcff883dd2cce18fd558e7d57d14f
Gerrit-Change-Number: 15373
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar 


[kudu-CR] [util] Add support for 32 & 64 byte alignment to Arena allocator

2020-03-05 Thread Bankim Bhavsar (Code Review)
Bankim Bhavsar has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/15372


Change subject: [util] Add support for 32 & 64 byte alignment to Arena allocator
..

[util] Add support for 32 & 64 byte alignment to Arena allocator

On adding couple of member variables and methods to BlockBloomFilter class
for importing Or() function, predicate-test would sometimes randomly crash
with SIGSEGV in AVX operations.

On debugging, the crash would only happen when the "directory_" structure
in BlockBloomFilter is not 32-bytes aligned. It's surprising without the
addition of those methods, "directory_" so far has always been 32 bytes
aligned despite Arena allocator not supporting alignment values greater than
16 bytes.

With input from Todd, explored 3 options to fix the alignment problem:
1) Update the HeapBufferAllocator in util/memory to align allocation to
64 bytes. See AllocateInternal() implementation. Surprisingly the
FLAGS_allocator_aligned_mode is OFF by default so we appear to be relying
on the allocator to always allocate 16 byte aligned buffers. So this option
would require turning ON the FLAGS_allocator_aligned_mode flag by default.
2) Update the Arena allocator such that when needed extra bytes are allocated
to allow aligning with 32/64 bytes considering the new component will always
be 16 byte aligned. This requires updating some address/alignment logic
with offset_ and the new component allocation.
3) Don't touch the Arena allocator and simply add padding in the
ArenaBlockBloomFilterBufferAllocator to minimize any risk to other parts of
the codebase.

Opted for option #2 since it broadly adds support for 32/64 byte alignment
instead of limited scope of option #3. Option #1 is tempting but unsure about
the unknowns that turning on the allocator_aligned_mode would bring.

Although we need only support for 32 byte alignment for AVX operations,
also added support for 64 bytes to better align with cache line size.

Additionally this change:
- Adds a simple BlockBloomFilter unit test that reproduced the alignment
problem compared to end-to-end predicate-test which was turning out to be
difficult to debug.
- Fixes and enhances the arena-test with bunch of variations.

Change-Id: Ib665115fa0fc262a8b76c48f52947dedb84be2a7
---
M src/kudu/util/block_bloom_filter-test.cc
M src/kudu/util/block_bloom_filter.cc
M src/kudu/util/memory/arena-test.cc
M src/kudu/util/memory/arena.cc
M src/kudu/util/memory/arena.h
5 files changed, 103 insertions(+), 36 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib665115fa0fc262a8b76c48f52947dedb84be2a7
Gerrit-Change-Number: 15372
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar 


[kudu-CR] KUDU-3063: Set a ratio to reserve some nvm memory

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

Change subject: KUDU-3063: Set a ratio to reserve some nvm memory
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/15313/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15313/1//COMMIT_MSG@8
PS1, Line 8:
Could you add more information to provide more context here?  I.e., describing 
the essence of the issue: the fragmentation.  Also, I think adding a reference 
to 
https://github.com/memkind/memkind/blob/master/test/fragmentation_benchmark_pmem.cpp
 is  useful as well.


http://gerrit.cloudera.org:8080/#/c/15313/1/src/kudu/util/nvm_cache.cc
File src/kudu/util/nvm_cache.cc:

http://gerrit.cloudera.org:8080/#/c/15313/1/src/kudu/util/nvm_cache.cc@76
PS1, Line 76: 1.2
> There's a micro benchmark under memkind, https://github.com/memkind/memkind
Thank you for the information.  Hopefully, 0.7 (or 10/7 with multiplying 
memkind_malloc_usable_size upon allocations) is indeed the worst case.


http://gerrit.cloudera.org:8080/#/c/15313/1/src/kudu/util/nvm_cache.cc@80
PS1, Line 80:
> Is that ok to validate it before memkind_create_pmem and also log the reduc
I think with the approach of multiplying memkind_malloc_usable_size, it's 
enough to make sure the multiplication factor is greater that 1: if it's less, 
return an error.  If it's less than 5/4, issue a warning about memkind 
fragmentation issues.


http://gerrit.cloudera.org:8080/#/c/15313/3/src/kudu/util/nvm_cache.cc
File src/kudu/util/nvm_cache.cc:

http://gerrit.cloudera.org:8080/#/c/15313/3/src/kudu/util/nvm_cache.cc@77
PS3, Line 77: The usage is equal to block "
:  "cache capacity divided by the ratio
It seems this has changed in PS3 (compared with PS2) once the semantics of the 
--nvm_cache_usage_ration has changed.


http://gerrit.cloudera.org:8080/#/c/15313/3/src/kudu/util/nvm_cache.cc@714
PS3, Line 714:   if (ratio < 1.0) {
 : LOG(WARNING) << "Confiture NVM usage ratio " << ratio << " 
is lower than minimum value 1.0. "
 :  << "Raise --nvm_cache_usage_ratio ";
 :   } else {
I think it's safer to prohibit setting ratio less than 1 at all (with new 
semantics for that parameter).  Most appropriate place to do so in a flag 
validator for the --nvm_cache_usage_ratio flag, so the issue is reported 
upfront when the process is starting.

For a reference/example, you can take a look at 
https://github.com/apache/kudu/blob/master/src/kudu/util/process_memory.cc#L108-L120



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceb1931f5addbf76774854ca1613b6a085b577e3
Gerrit-Change-Number: 15313
Gerrit-PatchSet: 3
Gerrit-Owner: ye yuqiang 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Bankim Bhavsar 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: ye yuqiang 
Gerrit-Comment-Date: Thu, 05 Mar 2020 20:09:06 +
Gerrit-HasComments: Yes


[kudu-CR] subprocess: add server-side metrics

2020-03-05 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15367 )

Change subject: subprocess: add server-side metrics
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51294741ff82bd47e64ceaba18a6d04ae0144179
Gerrit-Change-Number: 15367
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 05 Mar 2020 19:19:20 +
Gerrit-HasComments: No


[kudu-CR] Init kudu.write duration accumulator lazily

2020-03-05 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15368 )

Change subject: Init kudu.write_duration accumulator lazily
..


Patch Set 1:

I think Grant already addressed this issue within the HdrHistogramAccumulator 
itself.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I76dea33d652874f9cfcbb72a42ab3061fd1e6974
Gerrit-Change-Number: 15368
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 05 Mar 2020 19:08:58 +
Gerrit-HasComments: No


[kudu-CR] Init kudu.write duration accumulator lazily

2020-03-05 Thread Anonymous Coward (Code Review)
pengchengliu_b...@163.com has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/15368


Change subject: Init kudu.write_duration accumulator lazily
..

Init kudu.write_duration accumulator lazily

Change-Id: I76dea33d652874f9cfcbb72a42ab3061fd1e6974
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
1 file changed, 7 insertions(+), 2 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I76dea33d652874f9cfcbb72a42ab3061fd1e6974
Gerrit-Change-Number: 15368
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward 


[kudu-CR] [WIP] Ranger authorization provider

2020-03-05 Thread Attila Bukor (Code Review)
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao,

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

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

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

Change subject: [WIP] Ranger authorization provider
..

[WIP] Ranger authorization provider

Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
---
M src/kudu/master/CMakeLists.txt
M src/kudu/master/catalog_manager.cc
A src/kudu/master/ranger_authz_provider-test.cc
A src/kudu/master/ranger_authz_provider.cc
A src/kudu/master/ranger_authz_provider.h
M src/kudu/master/sentry_authz_provider.cc
6 files changed, 348 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/27
--
To view, visit http://gerrit.cloudera.org:8080/15207
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 27
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)


[kudu-CR] [WIP] Ranger authorization provider

2020-03-05 Thread Attila Bukor (Code Review)
Attila Bukor has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15207 )

Change subject: [WIP] Ranger authorization provider
..


Patch Set 27:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15207/26/src/kudu/master/ranger_authz_provider.cc
File src/kudu/master/ranger_authz_provider.cc:

http://gerrit.cloudera.org:8080/#/c/15207/26/src/kudu/master/ranger_authz_provider.cc@186
PS26, Line 186: 
> nit: indent.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 27
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 05 Mar 2020 11:11:34 +
Gerrit-HasComments: Yes