You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "KeDeng (Code Review)" <ge...@cloudera.org> on 2021/11/26 07:05:27 UTC

[kudu-CR] KUDU-3340 Disable compact on the specified table 

KeDeng has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18054


Change subject: KUDU-3340 Disable compact on the specified table
......................................................................

KUDU-3340 Disable compact on the specified table

For tables with only inserts but no updates, we can close the
compact operation of the table by added enable_compact attribute,
which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
5 files changed, 68 insertions(+), 2 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 1
Gerrit-Owner: KeDeng <kd...@gmail.com>

[kudu-CR] KUDU-3340 Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

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

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

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

Change subject: KUDU-3340 Disable compact on the specified table
......................................................................

KUDU-3340 Disable compact on the specified table

For tables with only inserts but no updates, we can close the
compact operation of the table by added enable_compact attribute,
which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
5 files changed, 68 insertions(+), 1 deletion(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 2
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-3340 [compaction] Disable compaction on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compaction on the specified table
......................................................................


Patch Set 26: Code-Review+2

Thank you for the contribution!


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 26
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 13 Jan 2022 04:11:06 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18054/15/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/15/src/kudu/integration-tests/alter_table-test.cc@2535
PS15, Line 2535:   // Reset to default disable_rowset_compaction.
               :   ASSERT_OK(table_alterer->AlterExtraConfig(
               :                            {{"kudu.table.disable_rowset_compaction", ""}})->Alter());
It seems like this doesn't actually reset anything, and is rather a no-op, at least from the conditional at https://gerrit.cloudera.org/c/18054/15/src/kudu/common/wire_protocol.cc#711

Perhaps we should test that's the case when disable_rowset_compaction=true?

Also could you also add a test that ensures we get expected errors when setting the value to an invalid string?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 15
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 04 Jan 2022 22:22:25 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 Disable compact on the specified table
......................................................................

KUDU-3340 Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
M src/kudu/util/maintenance_manager.h
9 files changed, 150 insertions(+), 7 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 5
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 12: Code-Review+1

LGTM


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 12
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 24 Dec 2021 05:54:46 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3340 Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins, 

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

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

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

Change subject: KUDU-3340 Disable compact on the specified table
......................................................................

KUDU-3340 Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added enable_compact attribute,
which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
5 files changed, 68 insertions(+), 1 deletion(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 3
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................

KUDU-3340 [compaction] Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
M src/kudu/util/maintenance_manager.h
9 files changed, 149 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/11
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 11
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................

KUDU-3340 [compaction] Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
8 files changed, 138 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/17
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 17
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 Disable compact on the specified table
......................................................................

KUDU-3340 Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
M src/kudu/util/maintenance_manager.h
9 files changed, 150 insertions(+), 7 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 4
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 Disable compact on the specified table

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

Change subject: KUDU-3340 Disable compact on the specified table
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18054/2/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/18054/2/src/kudu/tablet/tablet.cc@2394
PS2, Line 2394: setted
set


http://gerrit.cloudera.org:8080/#/c/18054/2/src/kudu/tablet/tablet.cc@2394
PS2, Line 2394: has
have



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 2
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 29 Nov 2021 01:37:18 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................

KUDU-3340 [compaction] Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
8 files changed, 138 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/20
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 20
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compaction on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compaction on the specified table
......................................................................


Patch Set 26:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18054/24/src/kudu/tablet/tablet_mm_ops.cc
File src/kudu/tablet/tablet_mm_ops.cc:

http://gerrit.cloudera.org:8080/#/c/18054/24/src/kudu/tablet/tablet_mm_ops.cc@134
PS24, Line 134:  
> nit for here and below: add a space after the name of compaction-related fl
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 26
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 13 Jan 2022 02:58:01 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 Disable compact on the specified table

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

Change subject: KUDU-3340 Disable compact on the specified table
......................................................................


Patch Set 10:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/common/common.proto@477
PS4, Line 477: If set true, the table will
> If set true
Done


http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/common/wire_protocol.cc
File src/kudu/common/wire_protocol.cc:

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/common/wire_protocol.cc@675
PS4, Line 675: return Status::
> Is this necessary?
Done


http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/common/wire_protocol.cc@684
PS4, Line 684: 
> 'kTableDisableCompact' should be better.
Done


http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/tablet/tablet.h
File src/kudu/tablet/tablet.h:

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/tablet/tablet.h@506
PS4, Line 506: Return 
> Return
Done


http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/tablet/tablet_mm_ops.cc
File src/kudu/tablet/tablet_mm_ops.cc:

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/tablet/tablet_mm_ops.cc@117
PS4, Line 117: disable_rowset_compaction
> What about using tablet_->metadata()->extra_config() directly?
Even I use it directly, I still need to judge the specific content of extra_config. It is more convenient to use the encapsulated function.


http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/util/maintenance_manager.h
File src/kudu/util/maintenance_manager.h:

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/util/maintenance_manager.h@187
PS4, Line 187: last_worked_
> Why we need this new viarable? Seems 'last_modified_' is enough.
The 'last_modified_' changed when UpdateStats() happens, even if do set_runnable() only. We need a new viarable used to distinguish whether only set_runnable() has run.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 10
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 23 Dec 2021 02:37:17 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 21:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18054/21/src/kudu/tablet/tablet_mm_ops-test.cc
File src/kudu/tablet/tablet_mm_ops-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/21/src/kudu/tablet/tablet_mm_ops-test.cc@102
PS21, Line 102: all_possible_metrics_
> Seems 'all_possible_metrics_' contains flush and compaction related metrics
I don't think it's necessary to distinguish all the metrics. Because in this scenario, all metrics should not be changed.


http://gerrit.cloudera.org:8080/#/c/18054/21/src/kudu/tablet/tablet_mm_ops-test.cc@108
PS21, Line 108: UpdateStats
> Seems this 'UpdateStates' doesn't change metrics‘ value actually even if we
I've just tested and will fail when we enable compaction. The changes like this:
diff --git a/src/kudu/tablet/tablet_mm_ops-test.cc b/src/kudu/tablet/tablet_mm_ops-test.cc
index 7fa4fb7..4d8e44d 100644
--- a/src/kudu/tablet/tablet_mm_ops-test.cc
+++ b/src/kudu/tablet/tablet_mm_ops-test.cc
@@ -131,7 +131,7 @@ TEST_F(KuduTabletMmOpsTest, TestCompactRowSetsOpCacheStats) {
 TEST_F(KuduTabletMmOpsTest, TestDisableCompactRowSetsOp) {
   CompactRowSetsOp op(tablet().get());
   TableExtraConfigPB extra_config;
-  extra_config.set_disable_compaction(true);
+  //extra_config.set_disable_compaction(true);
   NO_FATALS(AlterSchema(*harness_->tablet()->schema(), boost::make_optional(extra_config)));
   ASSERT_TRUE(op.DisableCompact());
   NO_FATALS(TestNoAffectedMetrics(&op));
@@ -150,7 +150,7 @@ TEST_F(KuduTabletMmOpsTest, TestMinorDeltaCompactionOpCacheStats) {
 TEST_F(KuduTabletMmOpsTest, TestDisableMinorDeltaCompactionOp) {
   MinorDeltaCompactionOp op(tablet().get());
   TableExtraConfigPB extra_config;
-  extra_config.set_disable_compaction(true);
+  //extra_config.set_disable_compaction(true);
   NO_FATALS(AlterSchema(*harness_->tablet()->schema(), boost::make_optional(extra_config)));
   ASSERT_TRUE(op.DisableCompact());
   NO_FATALS(TestNoAffectedMetrics(&op));
@@ -167,11 +167,10 @@ TEST_F(KuduTabletMmOpsTest, TestMajorDeltaCompactionOpCacheStats) {
                                        tablet()->metrics()->delta_major_compact_rs_duration }));
 }

-
 TEST_F(KuduTabletMmOpsTest, TestDisableMajorDeltaCompactionOp) {
   MajorDeltaCompactionOp op(tablet().get());
   TableExtraConfigPB extra_config;
-  extra_config.set_disable_compaction(true);
+  //extra_config.set_disable_compaction(true);
   NO_FATALS(AlterSchema(*harness_->tablet()->schema(), boost::make_optional(extra_config)));
   ASSERT_TRUE(op.DisableCompact());
   NO_FATALS(TestNoAffectedMetrics(&op));

And the test result is :
[----------] Global test environment tear-down
[==========] 6 tests from 1 test suite ran. (73 ms total)
[  PASSED  ] 3 tests.
[  FAILED  ] 3 tests, listed below:
[  FAILED  ] KuduTabletMmOpsTest.TestDisableCompactRowSetsOp
[  FAILED  ] KuduTabletMmOpsTest.TestDisableMinorDeltaCompactionOp
[  FAILED  ] KuduTabletMmOpsTest.TestDisableMajorDeltaCompactionOp



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 21
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 06 Jan 2022 03:53:54 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compaction on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compaction on the specified table
......................................................................

KUDU-3340 [compaction] Disable compaction on the specified table

For tables with only inserts but no updates, we can disable the
compaction of the table's data, which can improve the search
efficiency (the effect is more obvious in the search scenario
involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
8 files changed, 152 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/24
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 24
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compaction on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compaction on the specified table
......................................................................

KUDU-3340 [compaction] Disable compaction on the specified table

For tables with only inserts but no updates, we can disable the
compaction of the table's data, which can improve the search
efficiency (the effect is more obvious in the search scenario
involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
8 files changed, 152 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/25
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 25
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 Disable compact on the specified table
......................................................................

KUDU-3340 Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
M src/kudu/util/maintenance_manager.h
9 files changed, 149 insertions(+), 7 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 6
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................

KUDU-3340 [compaction] Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
8 files changed, 138 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/21
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 21
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 21: Code-Review+1

(2 comments)

> Patch Set 21:
> 
> (2 comments)

This patch is good overall except the tests didn't actually verify the functionality (in my opinions). I give my +1 but I'd like you to improve the tests.

http://gerrit.cloudera.org:8080/#/c/18054/21/src/kudu/tablet/tablet_mm_ops-test.cc
File src/kudu/tablet/tablet_mm_ops-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/21/src/kudu/tablet/tablet_mm_ops-test.cc@102
PS21, Line 102: all_possible_metrics_
> I don't think it's necessary to distinguish all the metrics. Because in thi
We only disable compaction on the tablet, so why all metrics should not change?


http://gerrit.cloudera.org:8080/#/c/18054/21/src/kudu/tablet/tablet_mm_ops-test.cc@108
PS21, Line 108: UpdateStats
> I've just tested and will fail when we enable compaction. The changes like 
The tests failed because this 'ASSERT_TRUE(op.DisableCompact())' failed, if this assert was removed, tests will pass. That means, we don't really test 'the metrics didn't change because we disable compaction on the tablet' in the tests.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 21
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 06 Jan 2022 05:37:47 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compaction on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compaction on the specified table
......................................................................


Patch Set 23:

(14 comments)

http://gerrit.cloudera.org:8080/#/c/18054/22//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18054/22//COMMIT_MSG@7
PS22, Line 7: compact
> compaction
Done


http://gerrit.cloudera.org:8080/#/c/18054/22//COMMIT_MSG@9
PS22, Line 9: , we can drop it since
            : it doesn't seem relevant. By disable the com
> Drop this since it doesn't seem relevant.
Done


http://gerrit.cloudera.org:8080/#/c/18054/22//COMMIT_MSG@11
PS22, Line 11: iency (the effect is more
> This name for the attribute seems to be out-of-date?
Done


http://gerrit.cloudera.org:8080/#/c/18054/22//COMMIT_MSG@10
PS22, Line 10: f the table's
             : data, which can improve the se
> disable the compaction of the table's data
Done


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/common/common.proto@476
PS22, Line 476:  If set true, the table's data on disk is no
> How about:
Done


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/common/wire_protocol.cc
File src/kudu/common/wire_protocol.cc:

http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/common/wire_protocol.cc@688
PS22, Line 688: kTableDisableCompact
> nit: rename kTableDisableCompact --> kTableDisableCompaction ?
Done


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/integration-tests/alter_table-test.cc@379
PS22, Line 379: CheckDisableCompact
> nit: rename CheckDisableCompact --> CheckDisableCompaction
Done


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/integration-tests/alter_table-test.cc@2535
PS22, Line 2535: Set disable_compaction false.
> nit: maybe, replace with
Done


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet.cc@1748
PS22, Line 1748: if (metadata_->extra_con
> That should have been a pointer, not a reference to a pointer, right?
Done


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops-test.cc
File src/kudu/tablet/tablet_mm_ops-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops-test.cc@107
PS22, Line 107: ar
> are
Done


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops-test.cc@112
PS22, Line 112: ar
> are
Done


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops-test.cc@172
PS22, Line 172: DisableCompact
> Here and in other places: rename DisableCompact --> DisableCompaction.
Done


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops.cc
File src/kudu/tablet/tablet_mm_ops.cc:

http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops.cc@116
PS22, Line 116: DisableCompact
> Rename DisableCompact --> DisableCompaction
Done


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops.cc@135
PS22, Line 135: disable_compaction in extra_config
> For this and all the updated messages below: prior to this update, the log 
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 23
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Jan 2022 03:53:43 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compaction on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compaction on the specified table
......................................................................

KUDU-3340 [compaction] Disable compaction on the specified table

For tables with only inserts but no updates, we can drop it since
it doesn't seem relevant. By disable the compaction of the table's
data, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
8 files changed, 156 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/23
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 23
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 21:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18054/21/src/kudu/tablet/tablet_mm_ops-test.cc
File src/kudu/tablet/tablet_mm_ops-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/21/src/kudu/tablet/tablet_mm_ops-test.cc@102
PS21, Line 102: all_possible_metrics_
Seems 'all_possible_metrics_' contains flush and compaction related metrics, should we test compaction metrics only?


http://gerrit.cloudera.org:8080/#/c/18054/21/src/kudu/tablet/tablet_mm_ops-test.cc@108
PS21, Line 108: UpdateStats
Seems this 'UpdateStates' doesn't change metrics‘ value actually even if we enable compaction, since there are no writes to the tablet.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 21
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 06 Jan 2022 03:19:13 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 21: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 21
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 05 Jan 2022 16:35:45 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 12:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/18054/10//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18054/10//COMMIT_MSG@7
PS10, Line 7: KUDU-3340 [compaction] Disable compact on the specified table
> Add square brackets to describe the module you modified, could be [compacti
Done


http://gerrit.cloudera.org:8080/#/c/18054/10/src/kudu/tablet/tablet.h
File src/kudu/tablet/tablet.h:

http://gerrit.cloudera.org:8080/#/c/18054/10/src/kudu/tablet/tablet.h@508
PS10, Line 508:   bool disable_rowset_compaction() const;
> nit: declare as a const function
Done


http://gerrit.cloudera.org:8080/#/c/18054/10/src/kudu/tablet/tablet_mm_ops.h
File src/kudu/tablet/tablet_mm_ops.h:

http://gerrit.cloudera.org:8080/#/c/18054/10/src/kudu/tablet/tablet_mm_ops.h@45
PS10, Line 45:   bool DisableCompact() const;
> nit: also declare it as a const function
Done


http://gerrit.cloudera.org:8080/#/c/18054/10/src/kudu/util/maintenance_manager.h
File src/kudu/util/maintenance_manager.h:

http://gerrit.cloudera.org:8080/#/c/18054/10/src/kudu/util/maintenance_manager.h@185
PS10, Line 185: set. T
> set
Done


http://gerrit.cloudera.org:8080/#/c/18054/10/src/kudu/util/maintenance_manager.h@186
PS10, Line 186: set wi
> set
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 12
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 24 Dec 2021 03:48:01 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 Disable compact on the specified table
......................................................................

KUDU-3340 Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
M src/kudu/util/maintenance_manager.h
9 files changed, 149 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/8
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 8
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/util/maintenance_manager.h
File src/kudu/util/maintenance_manager.h:

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/util/maintenance_manager.h@187
PS4, Line 187: last_worked_
> I don't think 'last_worked_' is much better than 'runuable_', at least in t
+1, we can check some metrics in this case.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 12
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 29 Dec 2021 02:59:23 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/util/maintenance_manager.h
File src/kudu/util/maintenance_manager.h:

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/util/maintenance_manager.h@187
PS4, Line 187: last_worked_
> Yes, we can use 'runnable()' only for this test scenario. However, by addin
I don't think 'last_worked_' is much better than 'runuable_', at least in this test case. If you want to make sure compaction ops are not scheduled, use compaction related metrics may be a more intuitive approach.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 12
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 28 Dec 2021 13:03:14 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compaction on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compaction on the specified table
......................................................................


Patch Set 25: Verified+1

unrelated test failures in (tests in the RELEASE build configuration passed):

    ConcurrentRebalancersTest.TwoConcurrentRebalancers/0
    HmsConfigurations/MasterFailoverTest.TestRenameTableSync/1
    AdminCliTest.TestExtraConfig


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 25
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 13 Jan 2022 02:22:44 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 22: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops-test.cc
File src/kudu/tablet/tablet_mm_ops-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops-test.cc@124
PS22, Line 124: op->UpdateStats(&stats_);
              :       ASSERT_FALSE(stats_.runnable());
              :       ASSERT_EQ(min_value, c->histogram()->MinValue());
              :       ASSERT_EQ(mean_value, c->histogram()->MeanValue());
              :       ASSERT_EQ(max_value, c->histogram()->MaxValue());
              :       ASSERT_EQ(total_count, c->histogram()->TotalCount());
              :       ASSERT_EQ(total_sum, c->histogram()->TotalSum());
nit: This 'UpdateStats' just modified the stats_, it didn't modify compaction_metrics_ in any case.  Maybe we should verify the value of these metrics hasn't been changed in tablet_server-test or somewhere. But I think the verification of stats_.runnable() is enough.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 22
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 11 Jan 2022 08:04:12 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 14:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/common/common.proto@476
PS14, Line 476: // It's a compact operation switch.
nit: This line could be removed.


http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/integration-tests/alter_table-test.cc@379
PS14, Line 379: CheckEnableCompact
nit: it should be 'CheckDisableCompact'.


http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/integration-tests/alter_table-test.cc@2524
PS14, Line 2524: // Set to disable_rowset_compaction.
nit: Set disable_rowset_compaction true.


http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/tablet/tablet_mm_ops-test.cc
File src/kudu/tablet/tablet_mm_ops-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/tablet/tablet_mm_ops-test.cc@107
PS14, Line 107: if (ContainsKey(metrics, c))
Could we just traverse `metrics`?


http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/tablet/tablet_mm_ops-test.cc@113
PS14, Line 113: SleepFor
Is this sleep necessary?


http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/tablet/tablet_mm_ops-test.cc@137
PS14, Line 137: TableExtraConfigPB extra_config;
              :   extra_config.set_disable_rowset_compaction(true);
              :   NO_FATALS(AlterSchema(*harness_->tablet()->schema(), boost::make_optional(extra_config)));
              :   ASSERT_TRUE(op.DisableCompact());
              :   NO_FATALS(TestNoAffectedMetrics(&op, { tablet()->metrics()->flush_mrs_duration,
What about adding a new test as 'TestDisableCompact'? It's better to keep these tests having single responsibility.


http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/tablet/tablet_mm_ops.h
File src/kudu/tablet/tablet_mm_ops.h:

http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/tablet/tablet_mm_ops.h@43
PS14, Line 43: Returns
nit: Return



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 14
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 30 Dec 2021 11:56:59 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compaction on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compaction on the specified table
......................................................................


Patch Set 24: Code-Review+1

(1 comment)

Almost there!  One more nit to make sure the warning messages are well-formed :)

http://gerrit.cloudera.org:8080/#/c/18054/24/src/kudu/tablet/tablet_mm_ops.cc
File src/kudu/tablet/tablet_mm_ops.cc:

http://gerrit.cloudera.org:8080/#/c/18054/24/src/kudu/tablet/tablet_mm_ops.cc@134
PS24, Line 134: "
nit for here and below: add a space after the name of compaction-related flags



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 24
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Jan 2022 16:13:54 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compaction on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compaction on the specified table
......................................................................


Patch Set 24:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/18054/22//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18054/22//COMMIT_MSG@9
PS22, Line 9: , we can disable the
            : compaction of the table's data, which can im
> Well, I meant dropping/removing the part of the sentence which was
Done


http://gerrit.cloudera.org:8080/#/c/18054/23/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/18054/23/src/kudu/tablet/tablet.cc@1747
PS23, Line 1747:   if (metadata_->extra_config() && metadata_->extra_config()->has_disable_compaction()) {
               :     return metadata_->extra_config()->disable_compaction();
               :   }
               :   return false;
               : }
> nit: what do you think of going a bit further and getting rid of the 'disab
Done


http://gerrit.cloudera.org:8080/#/c/18054/23/src/kudu/tablet/tablet_mm_ops.cc
File src/kudu/tablet/tablet_mm_ops.cc:

http://gerrit.cloudera.org:8080/#/c/18054/23/src/kudu/tablet/tablet_mm_ops.cc@134
PS23, Line 134: Substitute("Rowset compaction is disabled (check --enable_rowset_compaction"
              :            "and disable_compaction in extra_config for tablet:$0)", tablet_->tablet_id());
              :     stats->set_runnable(false);
> Do you mind using Substitute() to construct the log messages here and below
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 24
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Jan 2022 08:55:49 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compaction on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compaction on the specified table
......................................................................

KUDU-3340 [compaction] Disable compaction on the specified table

For tables with only inserts but no updates, we can disable the
compaction of the table's data, which can improve the search
efficiency (the effect is more obvious in the search scenario
involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Reviewed-on: http://gerrit.cloudera.org:8080/18054
Tested-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
8 files changed, 152 insertions(+), 7 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 27
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 14:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/util/maintenance_manager.h
File src/kudu/util/maintenance_manager.h:

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/util/maintenance_manager.h@187
PS4, Line 187: will periodi
> I don't think 'last_worked_' is much better than 'runuable_', at least in t
Done


http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/util/maintenance_manager.h@187
PS4, Line 187: will periodi
> +1, we can check some metrics in this case.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 14
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 29 Dec 2021 08:13:31 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 Disable compact on the specified table

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

Change subject: KUDU-3340 Disable compact on the specified table
......................................................................


Patch Set 10:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/18054/10//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18054/10//COMMIT_MSG@7
PS10, Line 7: KUDU-3340 Disable compact on the specified table
Add square brackets to describe the module you modified, could be [compaction]


http://gerrit.cloudera.org:8080/#/c/18054/10/src/kudu/tablet/tablet.h
File src/kudu/tablet/tablet.h:

http://gerrit.cloudera.org:8080/#/c/18054/10/src/kudu/tablet/tablet.h@508
PS10, Line 508:   bool disable_rowset_compaction();
nit: declare as a const function


http://gerrit.cloudera.org:8080/#/c/18054/10/src/kudu/tablet/tablet_mm_ops.h
File src/kudu/tablet/tablet_mm_ops.h:

http://gerrit.cloudera.org:8080/#/c/18054/10/src/kudu/tablet/tablet_mm_ops.h@45
PS10, Line 45:   bool DisableCompact();
nit: also declare it as a const function


http://gerrit.cloudera.org:8080/#/c/18054/10/src/kudu/util/maintenance_manager.h
File src/kudu/util/maintenance_manager.h:

http://gerrit.cloudera.org:8080/#/c/18054/10/src/kudu/util/maintenance_manager.h@185
PS10, Line 185: setted
set


http://gerrit.cloudera.org:8080/#/c/18054/10/src/kudu/util/maintenance_manager.h@186
PS10, Line 186: setted
set



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 10
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 23 Dec 2021 16:13:22 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................

KUDU-3340 [compaction] Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
8 files changed, 154 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/22
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 22
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................

KUDU-3340 [compaction] Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
8 files changed, 138 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/18
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 18
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 20: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 20
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 05 Jan 2022 06:50:34 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 18: Code-Review+1

(2 comments)

Just a couple more nits and this looks good to me.

http://gerrit.cloudera.org:8080/#/c/18054/18/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/18054/18/src/kudu/common/common.proto@477
PS18, Line 477: rowset_
Sorry, I suggested this name before knowing that we'd be disabling all compactions.

Let's call this 'disable_compaction', since "rowset compaction" in my opinion refers to merge compaction.


http://gerrit.cloudera.org:8080/#/c/18054/18/src/kudu/tablet/tablet_mm_ops.cc
File src/kudu/tablet/tablet_mm_ops.cc:

http://gerrit.cloudera.org:8080/#/c/18054/18/src/kudu/tablet/tablet_mm_ops.cc@134
PS18, Line 134:         << "Rowset compaction is disabled (check --enable_rowset_compaction"
              :         " and disable_rowset_compaction in extra_config)";
nit: align the quotation marks and put the space at the end of the first line, i.e.

  << "Rowset comp...check --enable_rowset_compaction "
     "and disable..."

same elsewhere.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 18
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 05 Jan 2022 03:48:06 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compaction on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compaction on the specified table
......................................................................

KUDU-3340 [compaction] Disable compaction on the specified table

For tables with only inserts but no updates, we can disable the
compaction of the table's data, which can improve the search
efficiency (the effect is more obvious in the search scenario
involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
8 files changed, 152 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/26
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 26
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compaction on the specified table

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.

Change subject: KUDU-3340 [compaction] Disable compaction on the specified table
......................................................................


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 25
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 Disable compact on the specified table
......................................................................

KUDU-3340 Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
M src/kudu/util/maintenance_manager.h
9 files changed, 149 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/9
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 9
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................

KUDU-3340 [compaction] Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
8 files changed, 138 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/14
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 14
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/util/maintenance_manager.h
File src/kudu/util/maintenance_manager.h:

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/util/maintenance_manager.h@187
PS4, Line 187: last_worked_
> What about using `bool runnable()` to check compaction ops are disabled? Se
Yes, we can use 'runnable()' only for this test scenario. However, by adding 'last_modified_', we can more clearly express our intention and facilitate further expansion in the future



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 12
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 28 Dec 2021 09:17:42 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 Disable compact on the specified table
......................................................................

KUDU-3340 Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
M src/kudu/util/maintenance_manager.h
9 files changed, 149 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/10
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 10
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................

KUDU-3340 [compaction] Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
8 files changed, 133 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/15
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 15
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 15:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/common/common.proto@476
PS14, Line 476: // If set true, the table will not 
> nit: This line could be removed.
Done


http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/integration-tests/alter_table-test.cc@379
PS14, Line 379: CheckDisableCompac
> nit: it should be 'CheckDisableCompact'.
Done


http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/integration-tests/alter_table-test.cc@2524
PS14, Line 2524: // Set disable_rowset_compaction tru
> nit: Set disable_rowset_compaction true.
Done


http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/tablet/tablet_mm_ops-test.cc
File src/kudu/tablet/tablet_mm_ops-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/tablet/tablet_mm_ops-test.cc@107
PS14, Line 107: auto total_sum = c->histogra
> Could we just traverse `metrics`?
Done


http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/tablet/tablet_mm_ops-test.cc@113
PS14, Line 113: SERT_EQ(
> Is this sleep necessary?
Done


http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/tablet/tablet_mm_ops-test.cc@137
PS14, Line 137: NO_FATALS(TestNoAffectedMetrics(&op));
              : }
              : 
              : TEST_F(KuduTabletMmOpsTest, TestMinorDeltaCompactionOpCacheStats) {
              :   MinorDeltaCompactionOp op(tablet().get());
> What about adding a new test as 'TestDisableCompact'? It's better to keep t
Done


http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/tablet/tablet_mm_ops.h
File src/kudu/tablet/tablet_mm_ops.h:

http://gerrit.cloudera.org:8080/#/c/18054/14/src/kudu/tablet/tablet_mm_ops.h@43
PS14, Line 43: Return 
> nit: Return
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 15
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Dec 2021 06:09:29 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 Disable compact on the specified table

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

Change subject: KUDU-3340 Disable compact on the specified table
......................................................................


Patch Set 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/18054/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18054/3//COMMIT_MSG@7
PS3, Line 7: KUDU-3340 Disable compact on the specified table
> Should this affect all compactions? Or just rowset merge compactions?
Yes, in this scenario, the three types of compact are meaningless and can be closed together.


http://gerrit.cloudera.org:8080/#/c/18054/3/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/18054/3/src/kudu/common/common.proto@476
PS3, Line 476:   // It's a compact operation switch.
             :   // If it was be setted to true, the table will not do compact.
             :   optional bool disable_rowset_comp
> Typically these extra configs are "off by default". To follow that style, h
Done


http://gerrit.cloudera.org:8080/#/c/18054/3/src/kudu/common/common.proto@476
PS3, Line 476:   // It's a compact operation switch.
             :   // If it was be setted to true, the table will not do compact.
             :   optional bool disable_rowset_comp
> +1
Done


http://gerrit.cloudera.org:8080/#/c/18054/3/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/3/src/kudu/integration-tests/alter_table-test.cc@2536
PS3, Line 2536:   ASSERT_OK(table_alterer->AlterExtraConfig(
> Does it make sense to add a test scenario which verifies that the actual fu
Done


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

http://gerrit.cloudera.org:8080/#/c/18054/3/src/kudu/tablet/tablet.cc@2394
PS3, Line 2394:   MonoDelta elapse = MonoTime::Now() - last_update_workload_stats_time_;
              :   if (metrics_) {
              :     int64_t scans_started = metrics_->scans_started->value();
              :    
> I agree with Andrew. The `workload_score` is used to define how 'hot' table
Done


http://gerrit.cloudera.org:8080/#/c/18054/3/src/kudu/tablet/tablet.cc@2394
PS3, Line 2394:   MonoDelta elapse = MonoTime::Now() - last_update_workload_stats_time_;
              :   if (metrics_) {
              :     int64_t scans_started = metrics_->scans_started->value();
              :    
> I don't think this is the score we want to be updating. You likely want to 
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 4
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 14 Dec 2021 03:48:32 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 22:

(15 comments)

http://gerrit.cloudera.org:8080/#/c/18054/22//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18054/22//COMMIT_MSG@7
PS22, Line 7: compact
compaction


http://gerrit.cloudera.org:8080/#/c/18054/22//COMMIT_MSG@9
PS22, Line 9: , with only a few queries
            : and will be migrated to HDFS in a short time
Drop this since it doesn't seem relevant.


http://gerrit.cloudera.org:8080/#/c/18054/22//COMMIT_MSG@11
PS22, Line 11: disable_rowset_compaction
This name for the attribute seems to be out-of-date?


http://gerrit.cloudera.org:8080/#/c/18054/22//COMMIT_MSG@10
PS22, Line 10: close the
             : compact operation of the table
disable the compaction of the table's data


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/common/common.proto@476
PS22, Line 476:  If set true, the table will not do compact.
How about:

If set true, the table's data on disk is not compacted.


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/common/wire_protocol.cc
File src/kudu/common/wire_protocol.cc:

http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/common/wire_protocol.cc@688
PS22, Line 688: kTableDisableCompact
nit: rename kTableDisableCompact --> kTableDisableCompaction ?


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/integration-tests/alter_table-test.cc@379
PS22, Line 379: CheckDisableCompact
nit: rename CheckDisableCompact --> CheckDisableCompaction


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/integration-tests/alter_table-test.cc@2535
PS22, Line 2535: Set to another disable_compaction
nit: maybe, replace with

  Set disable_compaction false


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet.cc@1748
PS22, Line 1748: const auto& extra_config
That should have been a pointer, not a reference to a pointer, right?

  const auto* extra_config = metadata_->extra_config();


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops-test.cc
File src/kudu/tablet/tablet_mm_ops-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops-test.cc@107
PS22, Line 107: is
are


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops-test.cc@112
PS22, Line 112: is
are


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops-test.cc@172
PS22, Line 172: DisableCompact
Here and in other places: rename DisableCompact --> DisableCompaction.


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops.h
File src/kudu/tablet/tablet_mm_ops.h:

http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops.h@45
PS22, Line 45: DisableCompact
Rename DisableCompact --> DisableCompaction.


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops.cc
File src/kudu/tablet/tablet_mm_ops.cc:

http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops.cc@116
PS22, Line 116: DisableCompact
Rename DisableCompact --> DisableCompaction


http://gerrit.cloudera.org:8080/#/c/18054/22/src/kudu/tablet/tablet_mm_ops.cc@135
PS22, Line 135: disable_compaction in extra_config
For this and all the updated messages below: prior to this update, the log messages were actionable because it was clear what to do if it's necessary to enable compaction -- just update corresponding flag for the tablet server.  With this update, how do operators know which tablet's extra config they should look at for the 'disable_compaction' property if they see this warning message?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 22
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 11 Jan 2022 16:24:20 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................

KUDU-3340 [compaction] Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
M src/kudu/util/maintenance_manager.h
9 files changed, 149 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/12
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 12
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/util/maintenance_manager.h
File src/kudu/util/maintenance_manager.h:

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/util/maintenance_manager.h@187
PS4, Line 187: last_worked_
> The 'last_modified_' changed when UpdateStats() happens, even if do set_run
What about using `bool runnable()` to check compaction ops are disabled? Seems 'last_worked_' is for use in test only.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 12
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 28 Dec 2021 06:13:14 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................

KUDU-3340 [compaction] Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
8 files changed, 138 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/19
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 19
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................


Patch Set 21:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18054/21/src/kudu/tablet/tablet_mm_ops-test.cc
File src/kudu/tablet/tablet_mm_ops-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/21/src/kudu/tablet/tablet_mm_ops-test.cc@102
PS21, Line 102: all_possible_metrics_
> We only disable compaction on the tablet, so why all metrics should not cha
Oh,I see what you mean.


http://gerrit.cloudera.org:8080/#/c/18054/21/src/kudu/tablet/tablet_mm_ops-test.cc@108
PS21, Line 108: UpdateStats
> The tests failed because this 'ASSERT_TRUE(op.DisableCompact())' failed, if
Yes, you are right.I'll improve this test.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 21
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 06 Jan 2022 06:17:17 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................

KUDU-3340 [compaction] Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
8 files changed, 138 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/16
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 16
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 [compaction] Disable compaction on the specified table

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

Change subject: KUDU-3340 [compaction] Disable compaction on the specified table
......................................................................


Patch Set 23: Code-Review+1

(3 comments)

Thank you for addressing those!

LGTM with a few more nits to take care of.

http://gerrit.cloudera.org:8080/#/c/18054/22//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18054/22//COMMIT_MSG@9
PS22, Line 9: , we can drop it since
            : it doesn't seem relevant. By disable the com
> Done
Well, I meant dropping/removing the part of the sentence which was

... with only a few queries and will be migrated to HDFS in a short time ...

:)

The idea was that the mentioned use case was specific to a particular workload, but the newly added feature can be used in many other scenarios.


http://gerrit.cloudera.org:8080/#/c/18054/23/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/18054/23/src/kudu/tablet/tablet.cc@1747
PS23, Line 1747:   bool disable_compaction = false;
               :   if (metadata_->extra_config() && metadata_->extra_config()->has_disable_compaction()) {
               :     disable_compaction = metadata_->extra_config()->disable_compaction();
               :   }
               :   return disable_compaction;
nit: what do you think of going a bit further and getting rid of the 'disable_compaction' variable as well?  It might be something like

bool Tablet::disable_compaction() const {
  if (metadata_->extra_config() && metadata_->extra_config()->has_disable_compaction()) {
    return metadata_->extra_config()->disable_compaction();
  }
  return false;
}


http://gerrit.cloudera.org:8080/#/c/18054/23/src/kudu/tablet/tablet_mm_ops.cc
File src/kudu/tablet/tablet_mm_ops.cc:

http://gerrit.cloudera.org:8080/#/c/18054/23/src/kudu/tablet/tablet_mm_ops.cc@134
PS23, Line 134: "Rowset compaction is disabled (check --enable_rowset_compaction"
              :            " and disable_compaction in extra_config for tablet:"
              :         << tablet_->tablet_id() << ")"
Do you mind using Substitute() to construct the log messages here and below?  Using Substitute() is better from the code readability perspective.

Thanks!



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 23
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Jan 2022 05:57:36 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 [compaction] Disable compact on the specified table

Posted by "KeDeng (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: KUDU-3340 [compaction] Disable compact on the specified table
......................................................................

KUDU-3340 [compaction] Disable compact on the specified table

For tables with only inserts but no updates, with only a few queries
and will be migrated to HDFS in a short time, we can close the
compact operation of the table by added disable_rowset_compaction
attribute, which can improve the search efficiency (the effect is more
obvious in the search scenario involving multiple tables).

Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
---
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops-test.cc
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
8 files changed, 109 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18054/13
-- 
To view, visit http://gerrit.cloudera.org:8080/18054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 13
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3340 Disable compact on the specified table

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

Change subject: KUDU-3340 Disable compact on the specified table
......................................................................


Patch Set 3:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/18054/3/src/kudu/tablet/tablet.cc@2394
PS3, Line 2394:   // For tables that have been set to not do compact op, return directly
              :   if (!enable_compact()) {
              :     return workload_score;
              :   }
I agree with Andrew. The `workload_score` is used to define how 'hot' tablets are in order to prioritize flush/compaction ops of hot tablets, and this feature is turned off by default (i.e. --enable_workload_score_for_perf_improvement_ops=false). That means we may still schedule compaction operations even if workload_score = 0. See the calculation of perf_score here: https://github.com/apache/kudu/blob/master/src/kudu/util/maintenance_manager.cc#L538



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 3
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 10 Dec 2021 11:22:14 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 Disable compact on the specified table

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

Change subject: KUDU-3340 Disable compact on the specified table
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18054/3/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/18054/3/src/kudu/common/common.proto@476
PS3, Line 476:   // It's a compact operation switch.
             :   // If it was be setted to false, the table will not do compact.
             :   optional bool enable_compact = 3;
> Typically these extra configs are "off by default". To follow that style, h
+1


http://gerrit.cloudera.org:8080/#/c/18054/3/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:

http://gerrit.cloudera.org:8080/#/c/18054/3/src/kudu/integration-tests/alter_table-test.cc@2536
PS3, Line 2536: }
Does it make sense to add a test scenario which verifies that the actual functionality works as expected, i.e. the corresponding type of compaction isn't running for the table in question once compaction is disabled by the configuration property?  It might be a separate scenario in some other test suite.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 3
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 10 Dec 2021 18:18:21 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3340 Disable compact on the specified table

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

Change subject: KUDU-3340 Disable compact on the specified table
......................................................................


Patch Set 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/common/common.proto@477
PS4, Line 477: If it was be setted to true
If set true


http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/common/wire_protocol.cc
File src/kudu/common/wire_protocol.cc:

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/common/wire_protocol.cc@675
PS4, Line 675: *result = true;
Is this necessary?


http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/common/wire_protocol.cc@684
PS4, Line 684: kTableEnableCompact
'kTableDisableCompact' should be better.


http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/tablet/tablet.h
File src/kudu/tablet/tablet.h:

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/tablet/tablet.h@506
PS4, Line 506: Returns
Return


http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/tablet/tablet_mm_ops.cc
File src/kudu/tablet/tablet_mm_ops.cc:

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/tablet/tablet_mm_ops.cc@117
PS4, Line 117: disable_rowset_compaction
What about using tablet_->metadata()->extra_config() directly?


http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/util/maintenance_manager.h
File src/kudu/util/maintenance_manager.h:

http://gerrit.cloudera.org:8080/#/c/18054/4/src/kudu/util/maintenance_manager.h@187
PS4, Line 187: last_worked_
Why we need this new viarable? Seems 'last_modified_' is enough.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8452bd9151f345fcad72bb9e0f07cd78432757e
Gerrit-Change-Number: 18054
Gerrit-PatchSet: 4
Gerrit-Owner: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 20 Dec 2021 10:08:58 +0000
Gerrit-HasComments: Yes