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

[kudu-CR] Improve logging of maintenance ops

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


Change subject: Improve logging of maintenance ops
......................................................................

Improve logging of maintenance ops

MRS flushes and rowset compactions
=================================

MRS flush and rowset compaction logging now includes the number of new rowsets
flushed.

Before:

I0830 14:54:24.353442 2830984064 tablet.cc:1651] T test_tablet_id P 78c16245dcd84048bf66debf1958c169: Flush successful on 100 rows (9272 bytes)

I0830 14:58:23.309068 2830984064 tablet.cc:1651] T test_tablet_id P 15853c32ec1d4f70b82aef90da2108a6: Compaction successful on 30 rows (4957 bytes)

After:

I0830 14:54:24.353442 2830984064 tablet.cc:1651] T test_tablet_id P 78c16245dcd84048bf66debf1958c169: Flush successful on 100 rows (1 rowsets, 9272 bytes)

I0830 14:58:23.309068 2830984064 tablet.cc:1651] T test_tablet_id P 15853c32ec1d4f70b82aef90da2108a6: Compaction successful on 30 rows (1 rowsets, 4957 bytes)

Major delta compactions
=======================

Major delta compaction had logging that was a little too verbose- for
each delta store compacted, it printed out a separate log message with
mutation counts. Instead, this patch makes the old more detailed output
appear only when verbose logging is on and at INFO level substitutes a
total count of each mutation type and a count of delta files compacted,
as part of the "Finished" message.

Before:

I0830 15:09:31.731609 2830984064 delta_compaction.cc:296] Starting major delta compaction for columns val1[int32 NOT NULL] val3[int32 NOT NULL] val4[string NOT NULL]
I0830 15:09:31.731645 2830984064 delta_compaction.cc:300] Preparing to major compact delta file: 0044448509586267 (ts range=[101, 150], delete_count=[0], reinsert_count=[0], update_counts_by_col_id=[11:50,13:50,14:50])
I0830 15:09:31.731672 2830984064 delta_compaction.cc:300] Preparing to major compact delta file: 0044448509586268 (ts range=[151, 200], delete_count=[0], reinsert_count=[0], update_counts_by_col_id=[11:50,13:50,14:50])
I0830 15:09:31.735069 2830984064 delta_compaction.cc:306] Finished major delta compaction of columns val1[int32 NOT NULL] val3[int32 NOT NULL] val4[string NOT NULL]

After:

I0830 14:45:38.433037 2830984064 delta_compaction.cc:326] Starting major delta compaction for columns val1[int32 NOT NULL], val3[int32 NOT NULL], val4[string NOT NULL]
I0830 14:45:38.435420 2830984064 delta_compaction.cc:341] Finished major delta compaction of columns val1[int32 NOT NULL], val3[int32 NOT NULL], val4[string NOT NULL]. Compacted 1 delta files. Overall stats: delete_count=0, reinsert_count=0, update_count=150

Minor delta compactions and deltamemstore flushes already include good
information on how much was compacted and flushed, respectively. Their
logging is unchanged.

Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
---
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_stats.cc
M src/kudu/tablet/delta_stats.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/tablet.cc
6 files changed, 64 insertions(+), 13 deletions(-)



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

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

[kudu-CR] Improve logging of maintenance ops

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

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

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

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

Change subject: Improve logging of maintenance ops
......................................................................

Improve logging of maintenance ops

MRS flushes and rowset compactions
=================================

MRS flush and rowset compaction logging now includes the number of new rowsets
flushed.

Before:

I0830 14:54:24.353442 2830984064 tablet.cc:1651] T test_tablet_id P 78c16245dcd84048bf66debf1958c169: Flush successful on 100 rows (9272 bytes)

I0830 14:58:23.309068 2830984064 tablet.cc:1651] T test_tablet_id P 15853c32ec1d4f70b82aef90da2108a6: Compaction successful on 30 rows (4957 bytes)

After:

I0830 14:54:24.353442 2830984064 tablet.cc:1651] T test_tablet_id P 78c16245dcd84048bf66debf1958c169: Flush successful on 100 rows (1 rowsets, 9272 bytes)

I0830 14:58:23.309068 2830984064 tablet.cc:1651] T test_tablet_id P 15853c32ec1d4f70b82aef90da2108a6: Compaction successful on 30 rows (1 rowsets, 4957 bytes)

Major delta compactions
=======================

Major delta compaction had logging that was a little too verbose- for
each delta store compacted, it printed out a separate log message with
mutation counts. Instead, this patch makes the old more detailed output
appear only when verbose logging is on and at INFO level substitutes a
total count of each mutation type and a count of delta files compacted,
as part of the "Finished" message.

Before:

I0830 15:22:05.918965 2830984064 delta_compaction.cc:296] Starting major delta compaction for columns val1[int32 NOT NULL] val3[int32 NOT NULL] val4[string NOT NULL]
I0830 15:22:05.919018 2830984064 delta_compaction.cc:300] Preparing to major compact delta file: 0145461451762035 (ts range=[101, 150], delete_count=[0], reinsert_count=[0], update_counts_by_col_id=[11:50,13:50,14:50])
I0830 15:22:05.919056 2830984064 delta_compaction.cc:300] Preparing to major compact delta file: 0145461451762036 (ts range=[151, 200], delete_count=[0], reinsert_count=[0], update_counts_by_col_id=[11:50,13:50,14:50])
I0830 15:22:05.921931 2830984064 delta_compaction.cc:306] Finished major delta compaction of columns val1[int32 NOT NULL] val3[int32 NOT NULL] val4[string NOT NULL]

After:

I0830 15:28:02.737797 2830984064 delta_compaction.cc:326] Starting major delta compaction for columns val1[int32 NOT NULL], val3[int32 NOT NULL], val4[string NOT NULL]
I0830 15:28:02.740360 2830984064 delta_compaction.cc:341] Finished major delta compaction of columns val1[int32 NOT NULL], val3[int32 NOT NULL], val4[string NOT NULL]. Compacted 2 delta files. Overall stats: delete_count=0, reinsert_count=0, update_count=300

Minor delta compactions and deltamemstore flushes already include good
information on how much was compacted and flushed, respectively. Their
logging is unchanged.

Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
---
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_stats.cc
M src/kudu/tablet/delta_stats.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/tablet.cc
6 files changed, 64 insertions(+), 13 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
Gerrit-Change-Number: 11367
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] [tablet] Improve logging of maintenance ops

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

Change subject: [tablet] Improve logging of maintenance ops
......................................................................


Patch Set 2:

(14 comments)

http://gerrit.cloudera.org:8080/#/c/11367/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11367/2//COMMIT_MSG@7
PS2, Line 7: Improve logging of maintenance ops
> Could you update this to follow the pattern in most of other Kudu commmits:
Done


http://gerrit.cloudera.org:8080/#/c/11367/2//COMMIT_MSG@12
PS2, Line 12: MRS flush and rowset compaction logging now includes the number of new rowsets
> nit: could you keep these lines no longer than 72 symbols?  Other lines wit
Done


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

http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_compaction.cc@107
PS2, Line 107: push_back
> nit: maybe, use emplace_back(std::move(...)) instead
The value being pushed is a temporary of type std::string, so I think it will be moved from by the overload of push_back that takes an rvalue.


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_compaction.cc@109
PS2, Line 109:  
> nit: indent
Done


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_compaction.cc@109
PS2, Line 109: push_back
> ditto
ditto :)


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_compaction.cc@296
PS2, Line 296: // Throwaway struct to combine stats from multiple delta stores.
             : struct MergedDeltaStats {
> Do you expect to use this more extensively? Why not just a `string DeltaSto
Done


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_compaction.cc@316
PS2, Line 316:  
> nit: add const specifier
N/A see Andrew's comment.


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_compaction.cc@330
PS2, Line 330: const shared_ptr<DeltaStore>&
> nit: would 'const auto&' work here as well?
Done


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_compaction.cc@341
PS2, Line 341: << "Finished major delta compaction of columns "
             :             << ColumnNamesToString() << ". Compacted " << included_stores_.size()
             :             << " delta files. Overall stats: " << mds.ToString();
> nit: consider using strings::Substitute() here
Done


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_stats.h
File src/kudu/tablet/delta_stats.h:

http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_stats.h@89
PS2, Line 89: int64_t UpdateCount() const;
> Maybe, follow the style of already existing update_count_for_col_id() metho
I'm following the GSG rule strictly here

"Regular functions have mixed case; accessors and mutators may be named like variables."

from https://google.github.io/styleguide/cppguide.html#Function_Names.

This function isn't an accessor- it does real work iterating over the update_counts_by_col_id_ member- so I named it as a function.


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

http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_tracker.cc@408
PS2, Line 408:   LOG_WITH_PREFIX(INFO) << "Flushing compaction of " << compacted_blocks.size()
             :                         << " redo delta blocks { " << compacted_blocks
             :                         << " } into block " << new_block_id;
> I think using strings::Substitute() would help to make it more readable.
Done


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/diskrowset.h
File src/kudu/tablet/diskrowset.h:

http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/diskrowset.h@220
PS2, Line 220: int64_t
> size() returns size_t; is there any specific reason to convert it into sign
To be consistent with the preexisting written_count (which I renamed to row_written_count), and in keeping with the GSG principal of using signed ints a lot (which I know you disagree with). See "On unsigned integers" at https://google.github.io/styleguide/cppguide.html#Integer_Types.


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

http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/tablet.cc@1487
PS2, Line 1487: gced_all_input
> If this variable is not used in the code below, consider getting rid of it 
Done


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/tablet.cc@1651
PS2, Line 1651: << op_name << " successful on " << drsw.rows_written_count()
              :                         << " rows " << "(" << drsw.drs_written_count() << " rowsets, "
              :                         << drsw.written_size() << " bytes)";
> Maybe, use strings::Substitute() to build this message?  That way it will b
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
Gerrit-Change-Number: 11367
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Aug 2018 17:10:50 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tablet] Improve logging of maintenance ops

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

Change subject: [tablet] Improve logging of maintenance ops
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
Gerrit-Change-Number: 11367
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Aug 2018 18:25:21 +0000
Gerrit-HasComments: No

[kudu-CR] Improve logging of maintenance ops

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

Change subject: Improve logging of maintenance ops
......................................................................


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_compaction.cc@296
PS2, Line 296: // Throwaway struct to combine stats from multiple delta stores.
             : struct MergedDeltaStats {
Do you expect to use this more extensively? Why not just a `string DeltaStoreStatsToString(vector<DeltaStore>)`?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
Gerrit-Change-Number: 11367
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 31 Aug 2018 05:15:41 +0000
Gerrit-HasComments: Yes

[kudu-CR] Improve logging of maintenance ops

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

Change subject: Improve logging of maintenance ops
......................................................................


Patch Set 2:

(13 comments)

looks good, just some nits

http://gerrit.cloudera.org:8080/#/c/11367/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11367/2//COMMIT_MSG@7
PS2, Line 7: Improve logging of maintenance ops
Could you update this to follow the pattern in most of other Kudu commmits:

[tablet] improve logging of maintenance ops

?


http://gerrit.cloudera.org:8080/#/c/11367/2//COMMIT_MSG@12
PS2, Line 12: MRS flush and rowset compaction logging now includes the number of new rowsets
nit: could you keep these lines no longer than 72 symbols?  Other lines with examples is OK to be as as, but here it would be nice to adhere to Kudu commit guidelines.


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

http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_compaction.cc@107
PS2, Line 107: push_back
nit: maybe, use emplace_back(std::move(...)) instead


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_compaction.cc@109
PS2, Line 109:  
nit: indent


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_compaction.cc@109
PS2, Line 109: push_back
ditto


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_compaction.cc@316
PS2, Line 316:  
nit: add const specifier


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_compaction.cc@330
PS2, Line 330: const shared_ptr<DeltaStore>&
nit: would 'const auto&' work here as well?


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_compaction.cc@341
PS2, Line 341: << "Finished major delta compaction of columns "
             :             << ColumnNamesToString() << ". Compacted " << included_stores_.size()
             :             << " delta files. Overall stats: " << mds.ToString();
nit: consider using strings::Substitute() here


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_stats.h
File src/kudu/tablet/delta_stats.h:

http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_stats.h@89
PS2, Line 89: int64_t UpdateCount() const;
Maybe, follow the style of already existing update_count_for_col_id() method and call this 

update_count()

or

update_count_total()

or

update_count_all_columns() ?


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

http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/delta_tracker.cc@408
PS2, Line 408:   LOG_WITH_PREFIX(INFO) << "Flushing compaction of " << compacted_blocks.size()
             :                         << " redo delta blocks { " << compacted_blocks
             :                         << " } into block " << new_block_id;
I think using strings::Substitute() would help to make it more readable.


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/diskrowset.h
File src/kudu/tablet/diskrowset.h:

http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/diskrowset.h@220
PS2, Line 220: int64_t
size() returns size_t; is there any specific reason to convert it into signed integer?


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

http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/tablet.cc@1487
PS2, Line 1487: gced_all_input
If this variable is not used in the code below, consider getting rid of it and just adding explanatory comment about drsw.rows_written_count() == 0 meaning.


http://gerrit.cloudera.org:8080/#/c/11367/2/src/kudu/tablet/tablet.cc@1651
PS2, Line 1651: << op_name << " successful on " << drsw.rows_written_count()
              :                         << " rows " << "(" << drsw.drs_written_count() << " rowsets, "
              :                         << drsw.written_size() << " bytes)";
Maybe, use strings::Substitute() to build this message?  That way it will be easier to read in the code.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
Gerrit-Change-Number: 11367
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 31 Aug 2018 04:52:08 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tablet] Improve logging of maintenance ops

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

Change subject: [tablet] Improve logging of maintenance ops
......................................................................

[tablet] Improve logging of maintenance ops

MRS flushes and rowset compactions
=================================

MRS flush and rowset compaction logging now includes the number of new
rowsets flushed.

Before:

I0830 14:54:24.353442 2830984064 tablet.cc:1651] T test_tablet_id P 78c16245dcd84048bf66debf1958c169: Flush successful on 100 rows (9272 bytes)

I0830 14:58:23.309068 2830984064 tablet.cc:1651] T test_tablet_id P 15853c32ec1d4f70b82aef90da2108a6: Compaction successful on 30 rows (4957 bytes)

After:

I0830 14:54:24.353442 2830984064 tablet.cc:1651] T test_tablet_id P 78c16245dcd84048bf66debf1958c169: Flush successful on 100 rows (1 rowsets, 9272 bytes)

I0830 14:58:23.309068 2830984064 tablet.cc:1651] T test_tablet_id P 15853c32ec1d4f70b82aef90da2108a6: Compaction successful on 30 rows (1 rowsets, 4957 bytes)

Major delta compactions
=======================

Major delta compaction had logging that was a little too verbose- for
each delta store compacted, it printed out a separate log message with
mutation counts. Instead, this patch makes the old more detailed output
appear only when verbose logging is on and at INFO level substitutes a
total count of each mutation type and a count of delta files compacted,
as part of the "Finished" message.

Before:

I0830 15:22:05.918965 2830984064 delta_compaction.cc:296] Starting major delta compaction for columns val1[int32 NOT NULL] val3[int32 NOT NULL] val4[string NOT NULL]
I0830 15:22:05.919018 2830984064 delta_compaction.cc:300] Preparing to major compact delta file: 0145461451762035 (ts range=[101, 150], delete_count=[0], reinsert_count=[0], update_counts_by_col_id=[11:50,13:50,14:50])
I0830 15:22:05.919056 2830984064 delta_compaction.cc:300] Preparing to major compact delta file: 0145461451762036 (ts range=[151, 200], delete_count=[0], reinsert_count=[0], update_counts_by_col_id=[11:50,13:50,14:50])
I0830 15:22:05.921931 2830984064 delta_compaction.cc:306] Finished major delta compaction of columns val1[int32 NOT NULL] val3[int32 NOT NULL] val4[string NOT NULL]

After:

I0830 15:28:02.737797 2830984064 delta_compaction.cc:326] Starting major delta compaction for columns val1[int32 NOT NULL], val3[int32 NOT NULL], val4[string NOT NULL]
I0830 15:28:02.740360 2830984064 delta_compaction.cc:341] Finished major delta compaction of columns val1[int32 NOT NULL], val3[int32 NOT NULL], val4[string NOT NULL]. Compacted 2 delta files. Overall stats: delete_count=0, reinsert_count=0, update_count=300

Minor delta compaction
======================

Now includes the number of stores compacted. I didn't see a test that
does minor delta compaction that I could pull bafe/after examples from.

Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
Reviewed-on: http://gerrit.cloudera.org:8080/11367
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_stats.cc
M src/kudu/tablet/delta_stats.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/tablet.cc
6 files changed, 64 insertions(+), 18 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
Gerrit-Change-Number: 11367
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tablet] Improve logging of maintenance ops

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

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

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

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

Change subject: [tablet] Improve logging of maintenance ops
......................................................................

[tablet] Improve logging of maintenance ops

MRS flushes and rowset compactions
=================================

MRS flush and rowset compaction logging now includes the number of new
rowsets flushed.

Before:

I0830 14:54:24.353442 2830984064 tablet.cc:1651] T test_tablet_id P 78c16245dcd84048bf66debf1958c169: Flush successful on 100 rows (9272 bytes)

I0830 14:58:23.309068 2830984064 tablet.cc:1651] T test_tablet_id P 15853c32ec1d4f70b82aef90da2108a6: Compaction successful on 30 rows (4957 bytes)

After:

I0830 14:54:24.353442 2830984064 tablet.cc:1651] T test_tablet_id P 78c16245dcd84048bf66debf1958c169: Flush successful on 100 rows (1 rowsets, 9272 bytes)

I0830 14:58:23.309068 2830984064 tablet.cc:1651] T test_tablet_id P 15853c32ec1d4f70b82aef90da2108a6: Compaction successful on 30 rows (1 rowsets, 4957 bytes)

Major delta compactions
=======================

Major delta compaction had logging that was a little too verbose- for
each delta store compacted, it printed out a separate log message with
mutation counts. Instead, this patch makes the old more detailed output
appear only when verbose logging is on and at INFO level substitutes a
total count of each mutation type and a count of delta files compacted,
as part of the "Finished" message.

Before:

I0830 15:22:05.918965 2830984064 delta_compaction.cc:296] Starting major delta compaction for columns val1[int32 NOT NULL] val3[int32 NOT NULL] val4[string NOT NULL]
I0830 15:22:05.919018 2830984064 delta_compaction.cc:300] Preparing to major compact delta file: 0145461451762035 (ts range=[101, 150], delete_count=[0], reinsert_count=[0], update_counts_by_col_id=[11:50,13:50,14:50])
I0830 15:22:05.919056 2830984064 delta_compaction.cc:300] Preparing to major compact delta file: 0145461451762036 (ts range=[151, 200], delete_count=[0], reinsert_count=[0], update_counts_by_col_id=[11:50,13:50,14:50])
I0830 15:22:05.921931 2830984064 delta_compaction.cc:306] Finished major delta compaction of columns val1[int32 NOT NULL] val3[int32 NOT NULL] val4[string NOT NULL]

After:

I0830 15:28:02.737797 2830984064 delta_compaction.cc:326] Starting major delta compaction for columns val1[int32 NOT NULL], val3[int32 NOT NULL], val4[string NOT NULL]
I0830 15:28:02.740360 2830984064 delta_compaction.cc:341] Finished major delta compaction of columns val1[int32 NOT NULL], val3[int32 NOT NULL], val4[string NOT NULL]. Compacted 2 delta files. Overall stats: delete_count=0, reinsert_count=0, update_count=300

Minor delta compaction
======================

Now includes the number of stores compacted. I didn't see a test that
does minor delta compaction that I could pull bafe/after examples from.

Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
---
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_stats.cc
M src/kudu/tablet/delta_stats.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/tablet.cc
6 files changed, 63 insertions(+), 17 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
Gerrit-Change-Number: 11367
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tablet] Improve logging of maintenance ops

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

Change subject: [tablet] Improve logging of maintenance ops
......................................................................


Patch Set 3:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11367/3/src/kudu/tablet/tablet.cc@1651
PS3, Line 1651:   LOG_WITH_PREFIX(INFO) << op_name << " successful on " << drsw.rows_written_count()
              :                         << " rows " << "(" << drsw.drs_written_count() << " rowsets, "
              :                         << drsw.written_size() << " bytes)";
              :   LOG_WITH_PREFIX(INFO) << Substitute("$0 successful on $1 rows ($2 rowsets, $3 bytes)",
              :                                       op_name,
              :                                       drsw.rows_written_count(),
              :                                       drsw.drs_written_count(),
              :                                       drsw.written_size());
Are these duplicates?  Seems like the first one should be removed.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
Gerrit-Change-Number: 11367
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Aug 2018 17:18:42 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tablet] Improve logging of maintenance ops

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

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

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

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

Change subject: [tablet] Improve logging of maintenance ops
......................................................................

[tablet] Improve logging of maintenance ops

MRS flushes and rowset compactions
=================================

MRS flush and rowset compaction logging now includes the number of new
rowsets flushed.

Before:

I0830 14:54:24.353442 2830984064 tablet.cc:1651] T test_tablet_id P 78c16245dcd84048bf66debf1958c169: Flush successful on 100 rows (9272 bytes)

I0830 14:58:23.309068 2830984064 tablet.cc:1651] T test_tablet_id P 15853c32ec1d4f70b82aef90da2108a6: Compaction successful on 30 rows (4957 bytes)

After:

I0830 14:54:24.353442 2830984064 tablet.cc:1651] T test_tablet_id P 78c16245dcd84048bf66debf1958c169: Flush successful on 100 rows (1 rowsets, 9272 bytes)

I0830 14:58:23.309068 2830984064 tablet.cc:1651] T test_tablet_id P 15853c32ec1d4f70b82aef90da2108a6: Compaction successful on 30 rows (1 rowsets, 4957 bytes)

Major delta compactions
=======================

Major delta compaction had logging that was a little too verbose- for
each delta store compacted, it printed out a separate log message with
mutation counts. Instead, this patch makes the old more detailed output
appear only when verbose logging is on and at INFO level substitutes a
total count of each mutation type and a count of delta files compacted,
as part of the "Finished" message.

Before:

I0830 15:22:05.918965 2830984064 delta_compaction.cc:296] Starting major delta compaction for columns val1[int32 NOT NULL] val3[int32 NOT NULL] val4[string NOT NULL]
I0830 15:22:05.919018 2830984064 delta_compaction.cc:300] Preparing to major compact delta file: 0145461451762035 (ts range=[101, 150], delete_count=[0], reinsert_count=[0], update_counts_by_col_id=[11:50,13:50,14:50])
I0830 15:22:05.919056 2830984064 delta_compaction.cc:300] Preparing to major compact delta file: 0145461451762036 (ts range=[151, 200], delete_count=[0], reinsert_count=[0], update_counts_by_col_id=[11:50,13:50,14:50])
I0830 15:22:05.921931 2830984064 delta_compaction.cc:306] Finished major delta compaction of columns val1[int32 NOT NULL] val3[int32 NOT NULL] val4[string NOT NULL]

After:

I0830 15:28:02.737797 2830984064 delta_compaction.cc:326] Starting major delta compaction for columns val1[int32 NOT NULL], val3[int32 NOT NULL], val4[string NOT NULL]
I0830 15:28:02.740360 2830984064 delta_compaction.cc:341] Finished major delta compaction of columns val1[int32 NOT NULL], val3[int32 NOT NULL], val4[string NOT NULL]. Compacted 2 delta files. Overall stats: delete_count=0, reinsert_count=0, update_count=300

Minor delta compaction
======================

Now includes the number of stores compacted. I didn't see a test that
does minor delta compaction that I could pull bafe/after examples from.

Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
---
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_stats.cc
M src/kudu/tablet/delta_stats.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/tablet.cc
6 files changed, 64 insertions(+), 18 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
Gerrit-Change-Number: 11367
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tablet] Improve logging of maintenance ops

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

Change subject: [tablet] Improve logging of maintenance ops
......................................................................


Patch Set 3:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11367/3/src/kudu/tablet/tablet.cc@1651
PS3, Line 1651:   LOG_WITH_PREFIX(INFO) << op_name << " successful on " << drsw.rows_written_count()
              :                         << " rows " << "(" << drsw.drs_written_count() << " rowsets, "
              :                         << drsw.written_size() << " bytes)";
              :   LOG_WITH_PREFIX(INFO) << Substitute("$0 successful on $1 rows ($2 rowsets, $3 bytes)",
              :                                       op_name,
              :                                       drsw.rows_written_count(),
              :                                       drsw.drs_written_count(),
              :                                       drsw.written_size());
> Are these duplicates?  Seems like the first one should be removed.
Whoops.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
Gerrit-Change-Number: 11367
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Aug 2018 18:07:10 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tablet] Improve logging of maintenance ops

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

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

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

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

Change subject: [tablet] Improve logging of maintenance ops
......................................................................

[tablet] Improve logging of maintenance ops

MRS flushes and rowset compactions
=================================

MRS flush and rowset compaction logging now includes the number of new
rowsets flushed.

Before:

I0830 14:54:24.353442 2830984064 tablet.cc:1651] T test_tablet_id P 78c16245dcd84048bf66debf1958c169: Flush successful on 100 rows (9272 bytes)

I0830 14:58:23.309068 2830984064 tablet.cc:1651] T test_tablet_id P 15853c32ec1d4f70b82aef90da2108a6: Compaction successful on 30 rows (4957 bytes)

After:

I0830 14:54:24.353442 2830984064 tablet.cc:1651] T test_tablet_id P 78c16245dcd84048bf66debf1958c169: Flush successful on 100 rows (1 rowsets, 9272 bytes)

I0830 14:58:23.309068 2830984064 tablet.cc:1651] T test_tablet_id P 15853c32ec1d4f70b82aef90da2108a6: Compaction successful on 30 rows (1 rowsets, 4957 bytes)

Major delta compactions
=======================

Major delta compaction had logging that was a little too verbose- for
each delta store compacted, it printed out a separate log message with
mutation counts. Instead, this patch makes the old more detailed output
appear only when verbose logging is on and at INFO level substitutes a
total count of each mutation type and a count of delta files compacted,
as part of the "Finished" message.

Before:

I0830 15:22:05.918965 2830984064 delta_compaction.cc:296] Starting major delta compaction for columns val1[int32 NOT NULL] val3[int32 NOT NULL] val4[string NOT NULL]
I0830 15:22:05.919018 2830984064 delta_compaction.cc:300] Preparing to major compact delta file: 0145461451762035 (ts range=[101, 150], delete_count=[0], reinsert_count=[0], update_counts_by_col_id=[11:50,13:50,14:50])
I0830 15:22:05.919056 2830984064 delta_compaction.cc:300] Preparing to major compact delta file: 0145461451762036 (ts range=[151, 200], delete_count=[0], reinsert_count=[0], update_counts_by_col_id=[11:50,13:50,14:50])
I0830 15:22:05.921931 2830984064 delta_compaction.cc:306] Finished major delta compaction of columns val1[int32 NOT NULL] val3[int32 NOT NULL] val4[string NOT NULL]

After:

I0830 15:28:02.737797 2830984064 delta_compaction.cc:326] Starting major delta compaction for columns val1[int32 NOT NULL], val3[int32 NOT NULL], val4[string NOT NULL]
I0830 15:28:02.740360 2830984064 delta_compaction.cc:341] Finished major delta compaction of columns val1[int32 NOT NULL], val3[int32 NOT NULL], val4[string NOT NULL]. Compacted 2 delta files. Overall stats: delete_count=0, reinsert_count=0, update_count=300

Minor delta compaction
======================

Now includes the number of stores compacted. I didn't see a test that
does minor delta compaction that I could pull bafe/after examples from.

Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
---
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_stats.cc
M src/kudu/tablet/delta_stats.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/tablet.cc
6 files changed, 66 insertions(+), 17 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I43883001c5a1c72ff1ca0c1bc84d24a8533e3891
Gerrit-Change-Number: 11367
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>