You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Will Berkeley (Code Review)" <ge...@cloudera.org> on 2018/11/06 02:36:48 UTC

[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode

Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11885


Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode
......................................................................

Limit number of rowsets in compaction selection to 32 in TSAN mode

TSAN limits the number of simultaneous lock acquisitions in a single
thread to 64 when using the deadlock detector[1]. However, compaction
can select up to 128 (128MB budget / 1MB min rowset size) rowsets in a
single op. kudu-tool-test's TestNonRandomWorkloadLoadgen almost always
hits TSAN's limit when the KUDU-1400 changes following this patch are
applied. This patch prevents this by limiting the number of rowsets
selected for a compaction to 32 when running under TSAN.

I ran the test with the KUDU-1400 changes on top and saw 97/100
failures. With the change, I saw 100 successes.

[1]: https://github.com/google/sanitizers/issues/950

Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
---
M src/kudu/tablet/tablet.cc
1 file changed, 19 insertions(+), 0 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Gerrit-Change-Number: 11885
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>

[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 )

Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode
......................................................................


Patch Set 2:

> Where did that 256KB figure come from? Can you point me at it?

I'm spending some time digging into the iterators underneath compaction to see if I can find it :). Right now it's just what Todd said.

There is a 32KiB arena on the top of all the iterators for compactions. See compaction.cc L205.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Gerrit-Change-Number: 11885
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 22:56:38 +0000
Gerrit-HasComments: No

[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, 

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

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

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

Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode
......................................................................

Limit number of rowsets in compaction selection to 32 in TSAN mode

TSAN limits the number of simultaneous lock acquisitions in a single
thread to 64 when using the deadlock detector[1]. However, compaction
can select up to 128 (128MB budget / 1MB min rowset size) rowsets in a
single op. kudu-tool-test's TestNonRandomWorkloadLoadgen almost always
hits TSAN's limit when the KUDU-1400 changes following this patch are
applied. This patch prevents this by limiting the number of rowsets
selected for a compaction to 32 when running under TSAN.

I ran the test with the KUDU-1400 changes on top and saw 97/100
failures. With the change, I saw 100 successes.

[1]: https://github.com/google/sanitizers/issues/950

Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
---
M src/kudu/tablet/tablet.cc
1 file changed, 19 insertions(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Gerrit-Change-Number: 11885
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11885 )

Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode
......................................................................

Limit number of rowsets in compaction selection to 32 in TSAN mode

TSAN limits the number of simultaneous lock acquisitions in a single
thread to 64 when using the deadlock detector[1]. However, compaction
can select up to 128 (128MB budget / 1MB min rowset size) rowsets in a
single op. kudu-tool-test's TestNonRandomWorkloadLoadgen almost always
hits TSAN's limit when the KUDU-1400 changes following this patch are
applied. This patch prevents this by limiting the number of rowsets
selected for a compaction to 32 when running under TSAN.

I ran the test with the KUDU-1400 changes on top and saw 97/100
failures. With the change, I saw 100 successes.

[1]: https://github.com/google/sanitizers/issues/950

Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Reviewed-on: http://gerrit.cloudera.org:8080/11885
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/tablet/tablet.cc
1 file changed, 19 insertions(+), 0 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Gerrit-Change-Number: 11885
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 )

Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode
......................................................................


Patch Set 2:

> Do I understand correctly that limiting number of rowsets for
 > compaction in general case (i.e. what Todd suggested) will be
 > published as a separate changelist?

Yeah. I need to understand the memory usage for compactions better to decide on what a reasonable limit is.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Gerrit-Change-Number: 11885
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 18:27:07 +0000
Gerrit-HasComments: No

[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 )

Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode
......................................................................


Patch Set 1:

The 256K is coming from the fact that the iterators are holding onto a BlockDecoder for each column, and the default cfile page size is 256K. So, those BlockDecoders that work by decompressing a page at a time up front are likely to retain ~256K of memory each.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Gerrit-Change-Number: 11885
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 13 Nov 2018 18:49:22 +0000
Gerrit-HasComments: No

[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 )

Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Gerrit-Change-Number: 11885
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 23:35:14 +0000
Gerrit-HasComments: No

[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 )

Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode
......................................................................


Patch Set 2: Code-Review+2

> Patch Set 2:
> I'm not sure what typical usage per column would be, though. Is it a lot less than 256KB?

Where did that 256KB figure come from? Can you point me at it?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Gerrit-Change-Number: 11885
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 21:58:33 +0000
Gerrit-HasComments: No

[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 )

Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1378
PS1, Line 1378: take its compact_flush_lock
> Wait, but those compact_flush_locks are taken and released one by one by th
I missed the move() part.  Please ignore this comment.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Gerrit-Change-Number: 11885
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 06:22:35 +0000
Gerrit-HasComments: Yes

[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 )

Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode
......................................................................


Patch Set 2:

Todd mentioned offline that it might be a good idea to limit the number of rowsets in a compaction even in release mode, since there is a memory cost per column per rowset involved in a compaction, up to 256KB.

At our column limit of 300 and the max number of rowsets in a compaction with default settings, 128, this means at worst using

300 * 128 * 256KB = 9.8GB

which is a lot. With a more-typical 50 columns, it's still over a GB of memory. I'm not sure what typical usage per column would be, though. Is it a lot less than 256KB?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Gerrit-Change-Number: 11885
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 16:11:49 +0000
Gerrit-HasComments: No

[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 )

Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1378
PS1, Line 1378: take its compact_flush_lock
> See L1407 below. The lock is moved into the 'picked' collection.
Right, it seems I missed that the move() part.


http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1385
PS1, Line 1385:     #if defined(THREAD_SANITIZER)
> This is an alternative way to do indentation? I'm not understanding what th
Yep; the point was to keep the '#' in the beginning of the line.  It might be easier to read, but I don't see that in the google style guide, so feel free to ignore.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Gerrit-Change-Number: 11885
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 06:33:09 +0000
Gerrit-HasComments: Yes

[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 )

Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1378
PS1, Line 1378: take its compact_flush_lock
Wait, but those compact_flush_locks are taken and released one by one by this code, right?  How could it happen that one thread takes and holds more than 64 of those?


http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1385
PS1, Line 1385:     #if defined(THREAD_SANITIZER)
style nit: there is an alternative style for those ifdefs, if you want:

#   if defined(...)
    
    const auto kMaxPickedUnderTsan = 32;
    
#   endif


http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1386
PS1, Line 1386: const
nit: constexpr ?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Gerrit-Change-Number: 11885
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 06 Nov 2018 06:13:30 +0000
Gerrit-HasComments: Yes

[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 )

Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1386
PS1, Line 1386: const
> nit: constexpr ?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Gerrit-Change-Number: 11885
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 16:07:57 +0000
Gerrit-HasComments: Yes

[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 )

Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode
......................................................................


Patch Set 2:

> > Where did that 256KB figure come from? Can you point me at it?
 > 
 > I'm spending some time digging into the iterators underneath
 > compaction to see if I can find it :). Right now it's just what
 > Todd said.
 > 
 > There is a 32KiB arena on the top of all the iterators for
 > compactions. See compaction.cc L205.

Do I understand correctly that limiting number of rowsets for compaction in general case (i.e. what Todd suggested) will be published as a separate changelist?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Gerrit-Change-Number: 11885
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 00:25:31 +0000
Gerrit-HasComments: No

[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 )

Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1378
PS1, Line 1378: take its compact_flush_lock
> Wait, but those compact_flush_locks are taken and released one by one by th
See L1407 below. The lock is moved into the 'picked' collection.


http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1385
PS1, Line 1385:     #if defined(THREAD_SANITIZER)
> style nit: there is an alternative style for those ifdefs, if you want:
This is an alternative way to do indentation? I'm not understanding what the point of the difference is.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766
Gerrit-Change-Number: 11885
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 06:19:29 +0000
Gerrit-HasComments: Yes