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 2017/05/11 01:01:09 UTC
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Will Berkeley has uploaded a new change for review.
http://gerrit.cloudera.org:8080/6850
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
KUDU-2001 Add UNDO size to tablet on-disk size
To make the on-disk size metric more accurate, this
patch makes DeltaTracker::EstimateOnDiskSize() count
UNDO deltas and adds a new method
DeltaTracker::EstimateOnDiskSizeForCompaction() that
retains the original behavior, which only counted
REDOs and was used for compaction scoring.
Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/tablet.cc
5 files changed, 25 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/50/6850/1
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/6850/3/src/kudu/tablet/delta_tracker.h
File src/kudu/tablet/delta_tracker.h:
PS3, Line 219: .
of all delta blocks.
PS3, Line 222: eligible for compaction.
: // This estimate includes REDO deltas, but not UNDO deltas
of REDO deltas
PS3, Line 224: EstimateOnDiskSizeForCompaction
Rename to EstimateRedoDeltaOnDiskSize()
http://gerrit.cloudera.org:8080/#/c/6850/3/src/kudu/tablet/diskrowset.h
File src/kudu/tablet/diskrowset.h:
PS3, Line 336: EstimateDeltaDiskSizeForCompaction
I think a better name would be EstimateRedoDeltaDiskSize()
Line 339: // TODO(wdberkeley) Offer a version that has the real total disk space usage.
Add: See KUDU-1755.
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 2:
> Build Failed
>
> http://104.196.14.100/job/kudu-gerrit/7839/ : FAILURE
Known, unrelated failure.
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 7:
> KUDU-1736
I mean the test failure is KUDU-1736.
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 5:
> Huh. It occurs to me that this needs tests. :)
Done.
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has uploaded a new patch set (#2).
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
KUDU-2001 Add UNDO size to tablet on-disk size
To make the on-disk size metric more accurate, this
patch makes DeltaTracker::EstimateOnDiskSize() count
UNDO deltas and adds a new method
DeltaTracker::EstimateOnDiskSizeForCompaction() that
retains the original behavior, which only counts
REDOs and is used for compaction scoring.
Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/tablet.cc
5 files changed, 25 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/50/6850/2
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6850/5/src/kudu/tablet/diskrowset.cc
File src/kudu/tablet/diskrowset.cc:
Line 692: return base_data_->EstimateOnDiskSize() + delta_tracker_->EstimateOnDiskSize();
> You're right about this. That needs to be adjusted as well. There should al
I'm not too surprised that no tests failed -- we tend not to get too white-boxy with the tests making super-specific assertions about number of bytes, since those tend to change a lot with other unrelated changes. The worry is more about the "emergent behavior" of a real workload, which is harder to see in a test environment, since we currently don't really have any "ultra-long" type tests with many GB of data involved running precommit.
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 5: Code-Review+1
lgtm modulo the commit message, will defer to Todd for final approval
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6850/2/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:
Line 152: using kudu::consensus::OpId;
> warning: using decl 'OpId' is unused [misc-unused-using-decls]
Done
Line 153: using kudu::consensus::MaximumOpId;
> warning: using decl 'MaximumOpId' is unused [misc-unused-using-decls]
Done
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6850/5//COMMIT_MSG
Commit Message:
Line 12: DeltaTracker::EstimateOnDiskSizeForCompaction() that
> this got renamed in the latest rev?
Yeah, renamed to EstimateRedoDeltaOnDiskSize()
Also yeah, for Git the typical wrap length is 72 chars per https://stackoverflow.com/questions/2290016/git-commit-messages-50-72-formatting
http://gerrit.cloudera.org:8080/#/c/6850/5/src/kudu/tablet/diskrowset.cc
File src/kudu/tablet/diskrowset.cc:
Line 692: return base_data_->EstimateOnDiskSize() + delta_tracker_->EstimateOnDiskSize();
> since this now includes the UNDO deltas, whereas before it didn't, will thi
The major delta compaction estimate is below on L742 which now calls into L683 here so this looks right to me. I searched the code base for other uses of EstimateOnDiskSize() and I didn't find anything that could affect compaction policy.
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6850/1/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:
Line 151: using base::subtle::Barrier_AtomicIncrement;
> warning: using decl 'Barrier_AtomicIncrement' is unused [misc-unused-using-
Done
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6850
to look at the new patch set (#3).
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
KUDU-2001 Add UNDO size to tablet on-disk size
To make the on-disk size metric more accurate, this
patch makes DeltaTracker::EstimateOnDiskSize() count
UNDO deltas and adds a new method
DeltaTracker::EstimateOnDiskSizeForCompaction() that
retains the original behavior, which only counts
REDOs and is used for compaction scoring.
Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/tablet.cc
5 files changed, 25 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/50/6850/3
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2001 Refactor rowset size estimates
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-2001 Refactor rowset size estimates
......................................................................
Patch Set 8: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 4: -Code-Review
Huh. It occurs to me that this needs tests. :)
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6850/5/src/kudu/tablet/diskrowset.cc
File src/kudu/tablet/diskrowset.cc:
Line 692: return base_data_->EstimateOnDiskSize() + delta_tracker_->EstimateOnDiskSize();
> The path I'm concerned about is RowSetInfo::RowSetInfo() on rowset_info.cc:
You're right about this. That needs to be adjusted as well. There should also be some tests that should've noticed the unintentional change and failed, probably, so I'll look to add that.
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2001 Refactor rowset size estimates
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Todd Lipcon, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6850
to look at the new patch set (#8).
Change subject: KUDU-2001 Refactor rowset size estimates
......................................................................
KUDU-2001 Refactor rowset size estimates
Currently, Rowset::EstimateOnDiskSize() serves two purposes:
1. An estimate of the total size of the rowset, which is
exposed when rolled into the tablet's on-disk size metric.
2. An estimate of the benefit of compaction.
These two purposes conflicted-- the compaction size counts only
base data and redo deltas that are relevant for compaction, so
e.g. undo deltas are omitted from the estimate.
This patch separates these two purposes. EstimateOnDiskSize()
remains the method for purpose #1, while a new method
EstimateCompactionSize() is introduced for purpose #2.
EstimateOnDiskSize now includes undo deltas, and so is more
accurate than before (however, there's more work to do: see
KUDU-1755).
There should be no changes to compaction policy as a result of
this patch.
Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_info.cc
M src/kudu/tablet/tablet.cc
12 files changed, 101 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/50/6850/8
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6850/5//COMMIT_MSG
Commit Message:
Line 12: DeltaTracker::EstimateOnDiskSizeForCompaction() that
> Yeah, renamed to EstimateRedoDeltaOnDiskSize()
Done
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6850
to look at the new patch set (#5).
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
KUDU-2001 Add UNDO size to tablet on-disk size
To make the on-disk size metric more accurate, this
patch makes DeltaTracker::EstimateOnDiskSize() count
UNDO deltas and adds a new method
DeltaTracker::EstimateOnDiskSizeForCompaction() that
retains the original behavior, which only counts
REDOs and is used for compaction scoring.
Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/tablet.cc
6 files changed, 63 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/50/6850/5
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2001 Refactor rowset size estimates
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has submitted this change and it was merged.
Change subject: KUDU-2001 Refactor rowset size estimates
......................................................................
KUDU-2001 Refactor rowset size estimates
Currently, Rowset::EstimateOnDiskSize() serves two purposes:
1. An estimate of the total size of the rowset, which is
exposed when rolled into the tablet's on-disk size metric.
2. An estimate of the benefit of compaction.
These two purposes conflicted-- the compaction size counts only
base data and redo deltas that are relevant for compaction, so
e.g. undo deltas are omitted from the estimate.
This patch separates these two purposes. EstimateOnDiskSize()
remains the method for purpose #1, while a new method
EstimateCompactionSize() is introduced for purpose #2.
EstimateOnDiskSize now includes undo deltas, and so is more
accurate than before (however, there's more work to do: see
KUDU-1755).
There should be no changes to compaction policy as a result of
this patch.
Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Reviewed-on: http://gerrit.cloudera.org:8080/6850
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_info.cc
M src/kudu/tablet/tablet.cc
12 files changed, 101 insertions(+), 15 deletions(-)
Approvals:
Todd Lipcon: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6850/5/src/kudu/tablet/diskrowset.cc
File src/kudu/tablet/diskrowset.cc:
Line 692: return base_data_->EstimateOnDiskSize() + delta_tracker_->EstimateOnDiskSize();
> This has no effect on compaction policy. EstimateDeltaOnDiskSize is renamed
The path I'm concerned about is RowSetInfo::RowSetInfo() on rowset_info.cc:257, which calls rs->EstimateOnDiskSize(). Prior to this patch, it wouldn't include UNDO, but now it does, right?
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 7:
KUDU-1736
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/6850/3/src/kudu/tablet/delta_tracker.h
File src/kudu/tablet/delta_tracker.h:
PS3, Line 219: .
> of all delta blocks.
Done
PS3, Line 222: eligible for compaction.
: // This estimate includes REDO deltas, but not UNDO deltas
> of REDO deltas
Done
PS3, Line 224: EstimateOnDiskSizeForCompaction
> Rename to EstimateRedoDeltaOnDiskSize()
Done
http://gerrit.cloudera.org:8080/#/c/6850/3/src/kudu/tablet/diskrowset.h
File src/kudu/tablet/diskrowset.h:
PS3, Line 336: EstimateDeltaDiskSizeForCompaction
> I think a better name would be EstimateRedoDeltaDiskSize()
Done
Line 339: // TODO(wdberkeley) Offer a version that has the real total disk space usage.
> Add: See KUDU-1755.
Done
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Todd Lipcon, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6850
to look at the new patch set (#6).
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
KUDU-2001 Add UNDO size to tablet on-disk size
To make the on-disk size metric more accurate, this patch makes
DeltaTracker::EstimateOnDiskSize() count UNDO deltas and adds a
new method DeltaTracker::EstimateRedoDeltaOnDiskSize() that
retains the original behavior. EstimateRedoDeltaOnDiskSize()
is now the function used in compaction policy, so the behavior
of compaction policy is not changed by this patch.
Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/tablet.cc
6 files changed, 63 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/50/6850/6
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 5: Code-Review-1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6850/5//COMMIT_MSG
Commit Message:
Line 12: DeltaTracker::EstimateOnDiskSizeForCompaction() that
this got renamed in the latest rev?
nit: can you re-wrap this paragraph? seems like it's wrapped really small
http://gerrit.cloudera.org:8080/#/c/6850/5/src/kudu/tablet/diskrowset.cc
File src/kudu/tablet/diskrowset.cc:
Line 692: return base_data_->EstimateOnDiskSize() + delta_tracker_->EstimateOnDiskSize();
since this now includes the UNDO deltas, whereas before it didn't, will this have a potential effect on compaction policy?
We need to be quite careful with such a change to make sure it doesn't have unintended consequences. (eg what if a DRS including its undos is now larger than the whole compaction budget, but those UNDOs are non-GCable?)
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6850/5/src/kudu/tablet/diskrowset.cc
File src/kudu/tablet/diskrowset.cc:
Line 692: return base_data_->EstimateOnDiskSize() + delta_tracker_->EstimateOnDiskSize();
> since this now includes the UNDO deltas, whereas before it didn't, will thi
This has no effect on compaction policy. EstimateDeltaOnDiskSize is renamed to EstimateRedoDeltaOnDiskSize, since it only counts redos, and it keeps the same implementation except it now calls EstimateRedoDeltaOnDiskSize, which is just a rename of DeltaTracker's EstimateOnDiskSize, since it only counted redo deltas. The original names are used for new functions which count undos as well as redos, so they are more properly a full on-disk size estimate, and they'll be surfaced to metrics instead of the redo-only number.
Like Mike, I checked the codebase for other uses of these functions and I'm sure there's no other effects.
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6850
to look at the new patch set (#4).
Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
......................................................................
KUDU-2001 Add UNDO size to tablet on-disk size
To make the on-disk size metric more accurate, this
patch makes DeltaTracker::EstimateOnDiskSize() count
UNDO deltas and adds a new method
DeltaTracker::EstimateOnDiskSizeForCompaction() that
retains the original behavior, which only counts
REDOs and is used for compaction scoring.
Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/tablet.cc
5 files changed, 23 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/50/6850/4
--
To view, visit http://gerrit.cloudera.org:8080/6850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>