[kudu-CR] [compaction] Cleanup of compaction policy code

2018-11-02 Thread Will Berkeley (Code Review)
Will Berkeley has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11827 )

Change subject: [compaction] Cleanup of compaction policy code
..

[compaction] Cleanup of compaction policy code

I'm reading over the compaction policy code before starting to work in
earnest on KUDU-1400, and I made a number of small changes that I think
help make the code more readable.

This patch contains no functional changes.

Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Reviewed-on: http://gerrit.cloudera.org:8080/11827
Reviewed-by: Alexey Serbin 
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong 
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/compaction_policy.h
M src/kudu/tablet/svg_dump.cc
M src/kudu/tablet/svg_dump.h
M src/kudu/tablet/tablet.cc
6 files changed, 291 insertions(+), 268 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Kudu Jenkins: Verified
  Andrew Wong: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-11-02 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11827 )

Change subject: [compaction] Cleanup of compaction policy code
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 17:31:27 +
Gerrit-HasComments: No


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-11-02 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11827 )

Change subject: [compaction] Cleanup of compaction policy code
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 16:33:57 +
Gerrit-HasComments: No


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-11-02 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11827 )

Change subject: [compaction] Cleanup of compaction policy code
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.h
File src/kudu/tablet/compaction_policy.h:

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.h@66
PS3, Line 66: std::numeric_limits::max()
> Ah, I see.  Would it be a better choice making this method pure virtual?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 16:07:26 +
Gerrit-HasComments: Yes


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-11-02 Thread Will Berkeley (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong,

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

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

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

Change subject: [compaction] Cleanup of compaction policy code
..

[compaction] Cleanup of compaction policy code

I'm reading over the compaction policy code before starting to work in
earnest on KUDU-1400, and I made a number of small changes that I think
help make the code more readable.

This patch contains no functional changes.

Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/compaction_policy.h
M src/kudu/tablet/svg_dump.cc
M src/kudu/tablet/svg_dump.h
M src/kudu/tablet/tablet.cc
6 files changed, 291 insertions(+), 268 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-11-01 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11827 )

Change subject: [compaction] Cleanup of compaction policy code
..


Patch Set 5: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.h
File src/kudu/tablet/compaction_policy.h:

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.h@66
PS3, Line 66: ompaction policies may prefer to dea
> Yes. For two reasons:
Ah, I see.  Would it be a better choice making this method pure virtual?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 01 Nov 2018 18:44:04 +
Gerrit-HasComments: Yes


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-11-01 Thread Will Berkeley (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong,

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

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

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

Change subject: [compaction] Cleanup of compaction policy code
..

[compaction] Cleanup of compaction policy code

I'm reading over the compaction policy code before starting to work in
earnest on KUDU-1400, and I made a number of small changes that I think
help make the code more readable.

This patch contains no functional changes.

Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/compaction_policy.h
M src/kudu/tablet/svg_dump.cc
M src/kudu/tablet/svg_dump.h
M src/kudu/tablet/tablet.cc
6 files changed, 292 insertions(+), 266 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-11-01 Thread Will Berkeley (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong,

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

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

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

Change subject: [compaction] Cleanup of compaction policy code
..

[compaction] Cleanup of compaction policy code

I'm reading over the compaction policy code before starting to work in
earnest on KUDU-1400, and I made a number of small changes that I think
help make the code more readable.

This patch contains no functional changes.

Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/compaction_policy.h
M src/kudu/tablet/svg_dump.cc
M src/kudu/tablet/svg_dump.h
M src/kudu/tablet/tablet.cc
6 files changed, 289 insertions(+), 263 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-11-01 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11827 )

Change subject: [compaction] Cleanup of compaction policy code
..


Patch Set 3:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy-test.cc
File src/kudu/tablet/compaction_policy-test.cc:

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy-test.cc@58
PS3, Line 58: unordered_set
> nit: introduce a typedef?
Done


http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.h
File src/kudu/tablet/compaction_policy.h:

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.h@62
PS3, Line 62: size
> nit: size in bytes ?
Done


http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.h@66
PS3, Line 66: std::numeric_limits::max()
> Are you sure this is not a functional change?
Yes. For two reasons:

1. CompactionPolicy is never instantiated. Only its subclass 
BudgetedCompactionPolicy is, and it overrides this.
2. 1GB is larger than the default compaction budget of 128MB, so this value 
would not have been effective anyway.

Using uint64_t's max value is more consonant with the "no rolling" intention.


http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.h@99
PS3, Line 99:   void SetupKnapsackInput(const RowSetTree& tree,
> warning: function 'kudu::tablet::BudgetedCompactionPolicy::SetupKnapsackInp
Done


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

http://gerrit.cloudera.org:8080/#/c/11827/1/src/kudu/tablet/compaction_policy.h@a53
PS1, Line 53:
> Is this done, or just a non-goal now? Regardless, it'd probably be helpful
It's not important (at least right now) what the units are. It's just a 
double...there's no particular range for it. It tends to be in the range 0-1 
for winning selections, but it could be much larger, and it is negative for 
lots of bad selections.

I don't want to refer to the doc here, again. The doc is already referred to a 
number of times throughout the compaction code.


http://gerrit.cloudera.org:8080/#/c/11827/1/src/kudu/tablet/compaction_policy.h@42
PS1, Line 42: ompactionPolicy() = default;
:
> not needed?
The compiler tells me I need it to implicitly initialize the base class in the 
subclass constructor and also to delete the base class when stored in a smart 
pointer.


http://gerrit.cloudera.org:8080/#/c/11827/1/src/kudu/tablet/compaction_policy.h@100
PS1, Line 100:   std::vector* asc_min_key,
> warning: function 'kudu::tablet::BudgetedCompactionPolicy::SetupKnapsackInp
Done


http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.cc
File src/kudu/tablet/compaction_policy.cc:

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.cc@53
PS3, Line 53: advanced
> As I understand, this change moves the flag into the non-experimental domai
Whoops. That was not intentional. Good catch.


http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.cc@374
PS3, Line 374:  &
> style nit: stick the ampersand to the parameter type.
Done


http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/tablet.cc@2317
PS3, Line 2317: *o
> nit: maybe, dereference the pointer just once and use the result reference
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 01 Nov 2018 16:51:40 +
Gerrit-HasComments: Yes


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-10-31 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11827 )

Change subject: [compaction] Cleanup of compaction policy code
..


Patch Set 3:

(6 comments)

Some nits, I didn't really looked at the essence of how compaction works.

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy-test.cc
File src/kudu/tablet/compaction_policy-test.cc:

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy-test.cc@58
PS3, Line 58: unordered_set
nit: introduce a typedef?


http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.h
File src/kudu/tablet/compaction_policy.h:

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.h@62
PS3, Line 62: size
nit: size in bytes ?


http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.h@66
PS3, Line 66: std::numeric_limits::max()
Are you sure this is not a functional change?


http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.cc
File src/kudu/tablet/compaction_policy.cc:

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.cc@53
PS3, Line 53: advanced
As I understand, this change moves the flag into the non-experimental domain, 
meaning --unlock_experimental_flags no longer necessary to use it.  Is that OK?


http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.cc@374
PS3, Line 374:  &
style nit: stick the ampersand to the parameter type.


http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/tablet.cc@2317
PS3, Line 2317: *o
nit: maybe, dereference the pointer just once and use the result reference 
everywhere in the body of this method.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 31 Oct 2018 18:02:11 +
Gerrit-HasComments: Yes


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-10-31 Thread Will Berkeley (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong,

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

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

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

Change subject: [compaction] Cleanup of compaction policy code
..

[compaction] Cleanup of compaction policy code

I'm reading over the compaction policy code before starting to work in
earnest on KUDU-1400, and I made a number of small changes that I think
help make the code more readable.

This patch contains no functional changes.

Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/compaction_policy.h
M src/kudu/tablet/svg_dump.cc
M src/kudu/tablet/svg_dump.h
M src/kudu/tablet/tablet.cc
6 files changed, 241 insertions(+), 220 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-10-31 Thread Will Berkeley (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong,

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

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

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

Change subject: [compaction] Cleanup of compaction policy code
..

[compaction] Cleanup of compaction policy code

I'm reading over the compaction policy code before starting to work in
earnest on KUDU-1400, and I made a number of small changes that I think
help make the code more readable.

This patch contains no functional changes.

Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/compaction_policy.h
M src/kudu/tablet/svg_dump.cc
M src/kudu/tablet/svg_dump.h
M src/kudu/tablet/tablet.cc
6 files changed, 241 insertions(+), 219 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-10-31 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11827 )

Change subject: [compaction] Cleanup of compaction policy code
..


Patch Set 1: Code-Review+1

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/11827/1/src/kudu/tablet/compaction_policy.h@a53
PS1, Line 53:
Is this done, or just a non-goal now? Regardless, it'd probably be helpful for 
further compaction-enthusiasts to define it here or point to the appropriate 
design doc explaining what 'quality' is.


http://gerrit.cloudera.org:8080/#/c/11827/1/src/kudu/tablet/compaction_policy.h@42
PS1, Line 42: Policy() = default;
:   virtual ~Co
not needed?


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

http://gerrit.cloudera.org:8080/#/c/11827/1/src/kudu/tablet/compaction_policy.cc@395
PS1, Line 395: computes some upper and lower bounds
nit: ... of the quality score



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 31 Oct 2018 06:36:43 +
Gerrit-HasComments: Yes


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-10-30 Thread Will Berkeley (Code Review)
Will Berkeley has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11827


Change subject: [compaction] Cleanup of compaction policy code
..

[compaction] Cleanup of compaction policy code

I'm reading over the compaction policy code before starting to work in
earnest on KUDU-1400, and I made a number of small changes that I think
help make the code more readable.

This patch contains no functional changes.

Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/compaction_policy.h
M src/kudu/tablet/svg_dump.cc
M src/kudu/tablet/svg_dump.h
M src/kudu/tablet/tablet.cc
6 files changed, 233 insertions(+), 213 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley