You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "ZhangYao (Code Review)" <ge...@cloudera.org> on 2019/06/17 08:31:06 UTC
[kudu-CR] KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
ZhangYao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13661
Change subject: KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
......................................................................
KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
---
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
2 files changed, 18 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/13661/1
--
To view, visit http://gerrit.cloudera.org:8080/13661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
Gerrit-Change-Number: 13661
Gerrit-PatchSet: 1
Gerrit-Owner: ZhangYao <tr...@gmail.com>
[kudu-CR] KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
Posted by "Yao Xu (Code Review)" <ge...@cloudera.org>.
Yao Xu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13661 )
Change subject: KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13661/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13661/1//COMMIT_MSG@8
PS1, Line 8:
Doc for this feature.
http://gerrit.cloudera.org:8080/#/c/13661/1/src/kudu/tablet/delta_store.cc
File src/kudu/tablet/delta_store.cc:
http://gerrit.cloudera.org:8080/#/c/13661/1/src/kudu/tablet/delta_store.cc@205
PS1, Line 205: if (may_have_deltas_) {
I think we should add some doc here to show that this variable represents whether there is has delta in the last batch.
--
To view, visit http://gerrit.cloudera.org:8080/13661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
Gerrit-Change-Number: 13661
Gerrit-PatchSet: 1
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Comment-Date: Mon, 17 Jun 2019 09:08:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13661 )
Change subject: KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
......................................................................
KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
Add a variable for lazy initialization to accelerate DeltaMemStore scan
for case of no matching deltas.
Rerun the benchmark of this patch with the original workload mentioned in
the JIRA (select * from my_wide_table where c1=<value that does not occur>).
Here are the top CPU consumers before and after:
Before:
+ 20.39% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaPreparer<kudu::tablet::DMSPreparerTraits>::Start(unsigned long, int)
+ 10.53% kudu-tserver kudu-tserver [.] kudu::tablet::CFileSet::Iterator::FinishBatch()
+ 8.64% kudu-tserver kudu-tserver [.] kudu::GetMemcmpableVarint64(kudu::Slice*, unsigned long*)
+ 7.35% kudu-tserver kudu-tserver [.] LZ4_decompress_fast
After:
+ 12.41% kudu-tserver kudu-tserver [.] kudu::tablet::CFileSet::Iterator::FinishBatch()
+ 10.74% kudu-tserver kudu-tserver [.] kudu::GetMemcmpableVarint64(kudu::Slice*, unsigned long*)
+ 9.94% kudu-tserver kudu-tserver [.] LZ4_decompress_fast
+ 7.54% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaFileIterator<(kudu::tablet::DeltaType)0>::AddDeltas(unsigned int, unsigned int)
+ 6.38% kudu-tserver kudu-tserver [.] bshuf_shuffle_bit_eightelem_AVX_avx2
+ 6.03% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaFileIterator<(kudu::tablet::DeltaType)0>::PrepareBatch(unsigned long, int)
...
+ 3.21% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaPreparer<kudu::tablet::DeltaFilePreparerTraits<(kudu::tablet::DeltaType)0> >::Start(unsigned long, int)
We can see DeltaPreparer::Start go from 20% CPU to 3% CPU.
Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
Reviewed-on: http://gerrit.cloudera.org:8080/13661
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
2 files changed, 29 insertions(+), 17 deletions(-)
Approvals:
Kudu Jenkins: Verified
Todd Lipcon: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/13661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
Gerrit-Change-Number: 13661
Gerrit-PatchSet: 4
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Reviewer: ZhangYao <tr...@gmail.com>
[kudu-CR] KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
Posted by "ZhangYao (Code Review)" <ge...@cloudera.org>.
ZhangYao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13661 )
Change subject: KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13661/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13661/2//COMMIT_MSG@9
PS2, Line 9: Add a variable for lazy initialization to accelerate DeltaMemStore scan
> nit: no need to indent here
Done
http://gerrit.cloudera.org:8080/#/c/13661/2//COMMIT_MSG@11
PS2, Line 11:
> I ran a benchmark of this patch with the original workload mentioned in the
Thanks a lot :). About why CFileSet::Iterator::FinishBatch CPU cost goes up, I will watch your jira(KUDU-2866).
http://gerrit.cloudera.org:8080/#/c/13661/2/src/kudu/tablet/delta_store.cc
File src/kudu/tablet/delta_store.cc:
http://gerrit.cloudera.org:8080/#/c/13661/2/src/kudu/tablet/delta_store.cc@214
PS2, Line 214: CHECK(deleted_.empty());
> nit: these lines are indented two extra spaces (we don't include the prepro
My fault. Done
--
To view, visit http://gerrit.cloudera.org:8080/13661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
Gerrit-Change-Number: 13661
Gerrit-PatchSet: 3
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Reviewer: ZhangYao <tr...@gmail.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 07:05:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13661 )
Change subject: KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13661/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13661/2//COMMIT_MSG@9
PS2, Line 9: Add a variable for lazy initialization to accelerate DeltaMemStore scan
nit: no need to indent here
http://gerrit.cloudera.org:8080/#/c/13661/2//COMMIT_MSG@11
PS2, Line 11:
I ran a benchmark of this patch with the original workload mentioned in the JIRA (select * from my_wide_table where c1=<value that does not occur>). Here are the top CPU consumers before and after:
Before:
+ 20.39% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaPreparer<kudu::tablet::DMSPreparerTraits>::Start(unsigned long, int)
+ 10.53% kudu-tserver kudu-tserver [.] kudu::tablet::CFileSet::Iterator::FinishBatch()
+ 8.64% kudu-tserver kudu-tserver [.] kudu::GetMemcmpableVarint64(kudu::Slice*, unsigned long*)
+ 7.35% kudu-tserver kudu-tserver [.] LZ4_decompress_fast
After:
+ 12.41% kudu-tserver kudu-tserver [.] kudu::tablet::CFileSet::Iterator::FinishBatch()
+ 10.74% kudu-tserver kudu-tserver [.] kudu::GetMemcmpableVarint64(kudu::Slice*, unsigned long*)
+ 9.94% kudu-tserver kudu-tserver [.] LZ4_decompress_fast
+ 7.54% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaFileIterator<(kudu::tablet::DeltaType)0>::AddDeltas(unsigned int, unsigned int)
+ 6.38% kudu-tserver kudu-tserver [.] bshuf_shuffle_bit_eightelem_AVX_avx2
+ 6.03% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaFileIterator<(kudu::tablet::DeltaType)0>::PrepareBatch(unsigned long, int)
...
+ 3.21% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaPreparer<kudu::tablet::DeltaFilePreparerTraits<(kudu::tablet::DeltaType)0> >::Start(unsigned long, int)
ie your patch made DeltaPreparer::Start go from 20% CPU to 3% CPU. Great! I'll work on a fix for CFileSet::Iterator::FinishBatch
Feel free to include the above in the commit message if you don't have time to run your own benchmark
http://gerrit.cloudera.org:8080/#/c/13661/2/src/kudu/tablet/delta_store.cc
File src/kudu/tablet/delta_store.cc:
http://gerrit.cloudera.org:8080/#/c/13661/2/src/kudu/tablet/delta_store.cc@214
PS2, Line 214: CHECK(deleted_.empty());
nit: these lines are indented two extra spaces (we don't include the preprocessor ifdef for indentation)
--
To view, visit http://gerrit.cloudera.org:8080/13661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
Gerrit-Change-Number: 13661
Gerrit-PatchSet: 2
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Reviewer: ZhangYao <tr...@gmail.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 06:33:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
Posted by "ZhangYao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Yao Xu, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13661
to look at the new patch set (#2).
Change subject: KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
......................................................................
KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
Add a variable for lazy initialization to accelerate DeltaMemStore scan
for case of no matching deltas.
Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
---
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
2 files changed, 29 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/13661/2
--
To view, visit http://gerrit.cloudera.org:8080/13661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
Gerrit-Change-Number: 13661
Gerrit-PatchSet: 2
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
[kudu-CR] KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
Posted by "ZhangYao (Code Review)" <ge...@cloudera.org>.
ZhangYao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13661 )
Change subject: KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13661/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13661/2//COMMIT_MSG@11
PS2, Line 11:
> The cost didn't go up -- just that, percentage-wise, it went from 10->12% b
Get that :)
--
To view, visit http://gerrit.cloudera.org:8080/13661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
Gerrit-Change-Number: 13661
Gerrit-PatchSet: 4
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Reviewer: ZhangYao <tr...@gmail.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 07:39:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13661 )
Change subject: KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13661/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13661/2//COMMIT_MSG@11
PS2, Line 11:
> Thanks a lot :). About why CFileSet::Iterator::FinishBatch CPU cost goes u
The cost didn't go up -- just that, percentage-wise, it went from 10->12% because the other costs went down. I put a patch here: https://gerrit.cloudera.org/c/13666/ and also about to post one to optimize our GetMemcmpableVarint64
--
To view, visit http://gerrit.cloudera.org:8080/13661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
Gerrit-Change-Number: 13661
Gerrit-PatchSet: 3
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Reviewer: ZhangYao <tr...@gmail.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 07:35:20 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13661 )
Change subject: KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13661/1/src/kudu/tablet/delta_store.cc
File src/kudu/tablet/delta_store.cc:
http://gerrit.cloudera.org:8080/#/c/13661/1/src/kudu/tablet/delta_store.cc@211
PS1, Line 211: }
I think it's worth adding here:
} else {
#ifndef NDEBUG
CHECK(deleted_.empty());
CHECK(reinserted_.empty());
for (const UpdatesForColumn& ufc : updates_by_col_) {
CHECK(ufc.empty());
}
#endif
}
so that if we accidentally miss a case here we would crash in a DEBUG build
http://gerrit.cloudera.org:8080/#/c/13661/1/src/kudu/tablet/delta_store.cc@212
PS1, Line 212: prepared_deltas_.clear();
this one can't go into the block above?
--
To view, visit http://gerrit.cloudera.org:8080/13661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
Gerrit-Change-Number: 13661
Gerrit-PatchSet: 1
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Comment-Date: Mon, 17 Jun 2019 16:50:53 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13661 )
Change subject: KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
Gerrit-Change-Number: 13661
Gerrit-PatchSet: 3
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Reviewer: ZhangYao <tr...@gmail.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 07:35:47 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
Posted by "ZhangYao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Yao Xu, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13661
to look at the new patch set (#3).
Change subject: KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
......................................................................
KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
Add a variable for lazy initialization to accelerate DeltaMemStore scan
for case of no matching deltas.
Rerun the benchmark of this patch with the original workload mentioned in
the JIRA (select * from my_wide_table where c1=<value that does not occur>).
Here are the top CPU consumers before and after:
Before:
+ 20.39% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaPreparer<kudu::tablet::DMSPreparerTraits>::Start(unsigned long, int)
+ 10.53% kudu-tserver kudu-tserver [.] kudu::tablet::CFileSet::Iterator::FinishBatch()
+ 8.64% kudu-tserver kudu-tserver [.] kudu::GetMemcmpableVarint64(kudu::Slice*, unsigned long*)
+ 7.35% kudu-tserver kudu-tserver [.] LZ4_decompress_fast
After:
+ 12.41% kudu-tserver kudu-tserver [.] kudu::tablet::CFileSet::Iterator::FinishBatch()
+ 10.74% kudu-tserver kudu-tserver [.] kudu::GetMemcmpableVarint64(kudu::Slice*, unsigned long*)
+ 9.94% kudu-tserver kudu-tserver [.] LZ4_decompress_fast
+ 7.54% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaFileIterator<(kudu::tablet::DeltaType)0>::AddDeltas(unsigned int, unsigned int)
+ 6.38% kudu-tserver kudu-tserver [.] bshuf_shuffle_bit_eightelem_AVX_avx2
+ 6.03% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaFileIterator<(kudu::tablet::DeltaType)0>::PrepareBatch(unsigned long, int)
...
+ 3.21% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaPreparer<kudu::tablet::DeltaFilePreparerTraits<(kudu::tablet::DeltaType)0> >::Start(unsigned long, int)
We can see DeltaPreparer::Start go from 20% CPU to 3% CPU.
Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
---
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
2 files changed, 29 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/13661/3
--
To view, visit http://gerrit.cloudera.org:8080/13661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
Gerrit-Change-Number: 13661
Gerrit-PatchSet: 3
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Reviewer: ZhangYao <tr...@gmail.com>
[kudu-CR] KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
Posted by "ZhangYao (Code Review)" <ge...@cloudera.org>.
ZhangYao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13661 )
Change subject: KUDU-2381: Optimize DeltaMemStore for case of no matching deltas.
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13661/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13661/1//COMMIT_MSG@8
PS1, Line 8:
> Doc for this feature.
Done
http://gerrit.cloudera.org:8080/#/c/13661/1/src/kudu/tablet/delta_store.cc
File src/kudu/tablet/delta_store.cc:
http://gerrit.cloudera.org:8080/#/c/13661/1/src/kudu/tablet/delta_store.cc@205
PS1, Line 205: // Lazy initialization.
> I think we should add some doc here to show that this variable represents w
Done
http://gerrit.cloudera.org:8080/#/c/13661/1/src/kudu/tablet/delta_store.cc@211
PS1, Line 211: reinserted_.clear();
> I think it's worth adding here:
Done
http://gerrit.cloudera.org:8080/#/c/13661/1/src/kudu/tablet/delta_store.cc@212
PS1, Line 212: } else {
> this one can't go into the block above?
Here prepared_deltas_ is used for PREPARE_FOR_COLLECT but update_by_col_ ?deleted_ and reinserted_ is used for PREPARE_FOR_APPLY. For lazy init updates_by_col_, if we put prepared_deltas_ above, when in PREPARE_FOR_COLLECT mode, we will also init updates_by_col_. Maybe it is a waste of time?
--
To view, visit http://gerrit.cloudera.org:8080/13661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b
Gerrit-Change-Number: 13661
Gerrit-PatchSet: 2
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Reviewer: ZhangYao <tr...@gmail.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 03:58:58 +0000
Gerrit-HasComments: Yes