You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "helifu (Code Review)" <ge...@cloudera.org> on 2019/07/09 12:42:13 UTC
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
helifu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13821
Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update
......................................................................
KUDU-2855 Lazy-create DeltaMemStore on first update
This patch supports lazy-create DeltaMemStrore on first update to
save memory and to fast-path out any DMS-related code. The created
DeltaMemStore will be destroyed on the following flush.
Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset-test.cc
3 files changed, 79 insertions(+), 41 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/13821/1
--
To view, visit http://gerrit.cloudera.org:8080/13821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
Gerrit-Change-Number: 13821
Gerrit-PatchSet: 1
Gerrit-Owner: helifu <hz...@corp.netease.com>
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13821 )
Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13821/3/src/kudu/tablet/delta_tracker.h
File src/kudu/tablet/delta_tracker.h:
http://gerrit.cloudera.org:8080/#/c/13821/3/src/kudu/tablet/delta_tracker.h@374
PS3, Line 374: // Number of deleted rows for a DMS that is currently being flushed.
Nit: what changed here?
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h
File src/kudu/tablet/delta_tracker.h:
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@336
PS2, Line 336: const scoped_refptr<log::LogAnchorRegistry> log_anchor_registry_;
> The "log_anchor_registry_" will be a wild pointer if we use a raw point her
I don't understand your explanation. When log_anchor_registry_ was a raw pointer, it pointed to the scoped_refptr owned by Tablet. That's safe because we can't destroy Tablet without first destroying all of its DiskRowSets and DeltaTrackers. This is a fundamental tablet lifecycle property that doesn't change even when DMSes are lazily constructed.
It's frustrating that LogAnchorRegistry isn't treated consistently as a shared object, but I think that's partly done for performance reasons. When Foo has a scoped_refptr<Bar> instead of a Bar*, Bar's reference count is incremented when Foo is constructed and decremented when Foo is destructed. For LogAnchorRegistry, we know that a Tablet will outlive all of its sub-objects, and because there can be many of those sub-objects (i.e. a handful per rowset), storing LogAnchorRegistry* in them means avoiding excessive increments and decrements without sacrificing safety.
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc
File src/kudu/tablet/delta_tracker.cc:
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@78
PS2, Line 78: DEFINE_bool(dms_lazy_create, true,
: "Allow lazily creating of DeltaMemStore");
: TAG_FLAG(dms_lazy_create, advanced
> Should I keep this gflag or just discard it? If yes, maybe some test cases
I'd discard it; it doesn't seem important enough to expose.
--
To view, visit http://gerrit.cloudera.org:8080/13821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
Gerrit-Change-Number: 13821
Gerrit-PatchSet: 3
Gerrit-Owner: helifu <hz...@corp.netease.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 10 Jul 2019 15:58:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13821 )
Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update
......................................................................
Patch Set 2:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/13821/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13821/2//COMMIT_MSG@9
PS2, Line 9: DeltaMemStrore
DeltaMemStore
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h
File src/kudu/tablet/delta_tracker.h:
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@336
PS2, Line 336: const scoped_refptr<log::LogAnchorRegistry> log_anchor_registry_;
Why can't this remain a raw pointer?
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@352
PS2, Line 352: dms_exist_
Nit: dms_exists_
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@356
PS2, Line 356: // - Mutators take this lock in exclusive mode while dms_ is not initialized.
"Mutators take this lock in exclusive mode if they need to create a new DMS, and shared mode otherwise."
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc
File src/kudu/tablet/delta_tracker.cc:
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@78
PS2, Line 78: DEFINE_bool(dms_lazy_create, true,
: "Allow lazily creating of DeltaMemStore");
: TAG_FLAG(dms_lazy_create, hidden);
What's the purpose of this if it's hidden?
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@169
PS2, Line 169: Status DeltaTracker::CreateAndInitDMSUnlocked(const fs::IOContext* io_context) {
This should only be done with component_lock_ held in exclusive mode, right? could you DCHECK on that? And take the lock before L162?
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@601
PS2, Line 601: !dms_->Empty()
This seems like a semantic change in that previously CollectStores returned the DMS even if it was empty. Is it safe?
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@659
PS2, Line 659: Status s = Status::OK();
This is the default value of a Status.
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@673
PS2, Line 673: operations of the above component_lock_
"critical sections defined by component_lock_"
--
To view, visit http://gerrit.cloudera.org:8080/13821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
Gerrit-Change-Number: 13821
Gerrit-PatchSet: 2
Gerrit-Owner: helifu <hz...@corp.netease.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 10 Jul 2019 03:58:13 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13821 )
Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update
......................................................................
KUDU-2855 Lazy-create DeltaMemStore on first update
This patch supports lazy-create DeltaMemStore on first update to
save memory and to fast-path out any DMS-related code. The created
DeltaMemStore will be destroyed on the following flush.
Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
Reviewed-on: http://gerrit.cloudera.org:8080/13821
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
4 files changed, 72 insertions(+), 43 deletions(-)
Approvals:
Kudu Jenkins: Verified
Adar Dembo: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/13821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
Gerrit-Change-Number: 13821
Gerrit-PatchSet: 5
Gerrit-Owner: helifu <hz...@corp.netease.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Posted by "helifu (Code Review)" <ge...@cloudera.org>.
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13821 )
Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h
File src/kudu/tablet/delta_tracker.h:
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@336
PS2, Line 336: const scoped_refptr<log::LogAnchorRegistry> log_anchor_registry_;
> Those test-only cases look like lazy programming. Let's fix them, and then
No, I will fix them. ^_^
By the way, a raw pointer here has a potential risk for the next users or test cases if he or she doesn't know how log_anchor_registry_ works.
--
To view, visit http://gerrit.cloudera.org:8080/13821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
Gerrit-Change-Number: 13821
Gerrit-PatchSet: 3
Gerrit-Owner: helifu <hz...@corp.netease.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 10 Jul 2019 22:27:17 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Posted by "helifu (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13821
to look at the new patch set (#2).
Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update
......................................................................
KUDU-2855 Lazy-create DeltaMemStore on first update
This patch supports lazy-create DeltaMemStrore on first update to
save memory and to fast-path out any DMS-related code. The created
DeltaMemStore will be destroyed on the following flush.
Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset-test.cc
3 files changed, 82 insertions(+), 46 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/13821/2
--
To view, visit http://gerrit.cloudera.org:8080/13821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
Gerrit-Change-Number: 13821
Gerrit-PatchSet: 2
Gerrit-Owner: helifu <hz...@corp.netease.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13821 )
Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
Gerrit-Change-Number: 13821
Gerrit-PatchSet: 4
Gerrit-Owner: helifu <hz...@corp.netease.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Thu, 11 Jul 2019 04:58:44 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Posted by "helifu (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13821
to look at the new patch set (#3).
Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update
......................................................................
KUDU-2855 Lazy-create DeltaMemStore on first update
This patch supports lazy-create DeltaMemStore on first update to
save memory and to fast-path out any DMS-related code. The created
DeltaMemStore will be destroyed on the following flush.
Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset-test.cc
3 files changed, 87 insertions(+), 47 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/13821/3
--
To view, visit http://gerrit.cloudera.org:8080/13821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
Gerrit-Change-Number: 13821
Gerrit-PatchSet: 3
Gerrit-Owner: helifu <hz...@corp.netease.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Posted by "helifu (Code Review)" <ge...@cloudera.org>.
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13821 )
Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13821/3/src/kudu/tablet/delta_tracker.h
File src/kudu/tablet/delta_tracker.h:
http://gerrit.cloudera.org:8080/#/c/13821/3/src/kudu/tablet/delta_tracker.h@374
PS3, Line 374: // Number of deleted rows for a DMS that is currently being flushed.
> Nit: what changed here?
There is a ^M at the end.
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h
File src/kudu/tablet/delta_tracker.h:
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@336
PS2, Line 336: const scoped_refptr<log::LogAnchorRegistry> log_anchor_registry_;
> I don't understand your explanation. When log_anchor_registry_ was a raw po
Yes, when log_anchor_registry_ was a raw pointer, it pointed to the scoped_refptr owned by Tablet. But, in test cases, this raw pointer is not very safe to use:
https://github.com/apache/kudu/blob/3cbc0d4fbe295748d6ffdf1e5e7edeaf94ef0911/src/kudu/tablet/diskrowset-test-base.h#L330
https://github.com/apache/kudu/blob/09e089bfafb9a1fa2099bd43cd0bd786dad0e771/src/kudu/tablet/diskrowset-test.cc#L759
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc
File src/kudu/tablet/delta_tracker.cc:
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@78
PS2, Line 78: DEFINE_bool(dms_lazy_create, true,
: "Allow lazily creating of DeltaMemStore");
: TAG_FLAG(dms_lazy_create, advanced
> I'd discard it; it doesn't seem important enough to expose.
ok.
--
To view, visit http://gerrit.cloudera.org:8080/13821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
Gerrit-Change-Number: 13821
Gerrit-PatchSet: 3
Gerrit-Owner: helifu <hz...@corp.netease.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 10 Jul 2019 22:05:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Posted by "helifu (Code Review)" <ge...@cloudera.org>.
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13821 )
Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update
......................................................................
Patch Set 2:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h
File src/kudu/tablet/delta_tracker.h:
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@336
PS2, Line 336: const scoped_refptr<log::LogAnchorRegistry> log_anchor_registry_;
> Why can't this remain a raw pointer?
The "log_anchor_registry_" will be a wild pointer if we use a raw point here because it will be release by "registry_" from class MinLogIndexAnchorer.
For example, TestRowSet.TestCompactStores from diskrowset-test.cc:
1) the object "log::LogAnchorRegistry" is a raw pointer: https://github.com/apache/kudu/blob/3cbc0d4fbe295748d6ffdf1e5e7edeaf94ef0911/src/kudu/tablet/diskrowset-test-base.h#L330
2) the raw pointer will be released by "registry_" from class MinLogIndexAnchorer
3) then the next creation for DMS is dangerous.
==================
On the other side, yes, we can use a raw pointer indeed. But I think it's better to use a ref_ptr because we can't ensure that every caller retains its lifecycle.
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@352
PS2, Line 352: dms_exist_
> Nit: dms_exists_
Done
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@356
PS2, Line 356: // - Mutators take this lock in exclusive mode while dms_ is not initialized.
> "Mutators take this lock in exclusive mode if they need to create a new DMS
Done
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc
File src/kudu/tablet/delta_tracker.cc:
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@78
PS2, Line 78: DEFINE_bool(dms_lazy_create, true,
: "Allow lazily creating of DeltaMemStore");
: TAG_FLAG(dms_lazy_create, hidden);
> What's the purpose of this if it's hidden?
Should I keep this gflag or just discard it? If yes, maybe some test cases to cover true or false are necessary.
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@169
PS2, Line 169: Status DeltaTracker::CreateAndInitDMSUnlocked(const fs::IOContext* io_context) {
> This should only be done with component_lock_ held in exclusive mode, right
Done
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@601
PS2, Line 601: !dms_->Empty()
> This seems like a semantic change in that previously CollectStores returned
Emmm, the DMS will be not returned when dms_exists_ is false either.
And "!dms_->Empty" can help to avoid returning a empty DMS, though that is unlikely to happen.
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@659
PS2, Line 659: Status s = Status::OK();
> This is the default value of a Status.
Done
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@673
PS2, Line 673: operations of the above component_lock_
> "critical sections defined by component_lock_"
Done
--
To view, visit http://gerrit.cloudera.org:8080/13821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
Gerrit-Change-Number: 13821
Gerrit-PatchSet: 2
Gerrit-Owner: helifu <hz...@corp.netease.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 10 Jul 2019 11:40:33 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Posted by "helifu (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13821
to look at the new patch set (#4).
Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update
......................................................................
KUDU-2855 Lazy-create DeltaMemStore on first update
This patch supports lazy-create DeltaMemStore on first update to
save memory and to fast-path out any DMS-related code. The created
DeltaMemStore will be destroyed on the following flush.
Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
4 files changed, 72 insertions(+), 43 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/13821/4
--
To view, visit http://gerrit.cloudera.org:8080/13821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
Gerrit-Change-Number: 13821
Gerrit-PatchSet: 4
Gerrit-Owner: helifu <hz...@corp.netease.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13821 )
Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h
File src/kudu/tablet/delta_tracker.h:
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@336
PS2, Line 336: const scoped_refptr<log::LogAnchorRegistry> log_anchor_registry_;
> Yes, when log_anchor_registry_ was a raw pointer, it pointed to the scoped_
Those test-only cases look like lazy programming. Let's fix them, and then a raw pointer should be 100% safe here?
--
To view, visit http://gerrit.cloudera.org:8080/13821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
Gerrit-Change-Number: 13821
Gerrit-PatchSet: 3
Gerrit-Owner: helifu <hz...@corp.netease.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 10 Jul 2019 22:17:26 +0000
Gerrit-HasComments: Yes