You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2017/05/09 06:55:29 UTC
[kudu-CR] KUDU-1549: delete dead LBM containers at startup
Hello David Ribeiro Alves, Todd Lipcon,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/6824
to review the following change.
Change subject: KUDU-1549: delete dead LBM containers at startup
......................................................................
KUDU-1549: delete dead LBM containers at startup
Full containers with no live blocks are "dead" in that they will never be
used for any block operations. These containers are safe to delete, and
though we lose some forensic information in doing so, we'll wind up
processing less container metadata in the next startup.
This patch adds a quick and dirty implementation of dead container deletion
at startup. It would be nice to also do it in real time (perhaps as a
maintenance manager operation), but that requires containers to have shared
ownership, a lifecycle change I'm not keen on making right now.
Change-Id: I73a903092cf89508b7ba97fde3a2d6e691161b4c
---
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
3 files changed, 156 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/6824/1
--
To view, visit http://gerrit.cloudera.org:8080/6824
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I73a903092cf89508b7ba97fde3a2d6e691161b4c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1549: delete dead LBM containers at startup
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: KUDU-1549: delete dead LBM containers at startup
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/6824
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I73a903092cf89508b7ba97fde3a2d6e691161b4c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-1549: delete dead LBM containers at startup
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-1549: delete dead LBM containers at startup
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6824/1//COMMIT_MSG
Commit Message:
PS1, Line 11: we lose some forensic information in doing so
> such as? I mean it'd be good for this message to spell out the rough types/
Done
PS1, Line 15: It would be nice to also do it in real time (perhaps as a
: maintenance manager operation), but that requires containers to have shared
: ownership, a lifecycle change I'm not keen on making right now.
> getting a bit worried that doing proper LBM maintenance will depend on rest
True. It's just a lot more complicated to do some of this stuff in real time (both in deciding when to do it and in actually doing it), so I think this is a reasonable first step for the time being.
http://gerrit.cloudera.org:8080/#/c/6824/1/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
Line 1677: CHECK(to_delete->full());
> add an error message.
Done
--
To view, visit http://gerrit.cloudera.org:8080/6824
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I73a903092cf89508b7ba97fde3a2d6e691161b4c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1549: delete dead LBM containers at startup
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello David Ribeiro Alves, Todd Lipcon, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6824
to look at the new patch set (#2).
Change subject: KUDU-1549: delete dead LBM containers at startup
......................................................................
KUDU-1549: delete dead LBM containers at startup
Full containers with no live blocks are "dead" in that they will never be
used for any block operations. These containers are safe to delete, and
though we lose some forensic information (e.g. block X was created at time Y
and deleted at time Z) in doing so, we'll wind up processing less container
metadata in the next startup.
This patch adds a quick and dirty implementation of dead container deletion
at startup. It would be nice to also do it in real time (perhaps as a
maintenance manager operation), but that requires containers to have shared
ownership, a lifecycle change I'm not keen on making right now.
Change-Id: I73a903092cf89508b7ba97fde3a2d6e691161b4c
---
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
3 files changed, 157 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/6824/2
--
To view, visit http://gerrit.cloudera.org:8080/6824
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I73a903092cf89508b7ba97fde3a2d6e691161b4c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1549: delete dead LBM containers at startup
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged.
Change subject: KUDU-1549: delete dead LBM containers at startup
......................................................................
KUDU-1549: delete dead LBM containers at startup
Full containers with no live blocks are "dead" in that they will never be
used for any block operations. These containers are safe to delete, and
though we lose some forensic information (e.g. block X was created at time Y
and deleted at time Z) in doing so, we'll wind up processing less container
metadata in the next startup.
This patch adds a quick and dirty implementation of dead container deletion
at startup. It would be nice to also do it in real time (perhaps as a
maintenance manager operation), but that requires containers to have shared
ownership, a lifecycle change I'm not keen on making right now.
Change-Id: I73a903092cf89508b7ba97fde3a2d6e691161b4c
Reviewed-on: http://gerrit.cloudera.org:8080/6824
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <da...@gmail.com>
---
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
3 files changed, 157 insertions(+), 20 deletions(-)
Approvals:
David Ribeiro Alves: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/6824
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I73a903092cf89508b7ba97fde3a2d6e691161b4c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1549: delete dead LBM containers at startup
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: KUDU-1549: delete dead LBM containers at startup
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6824/1//COMMIT_MSG
Commit Message:
PS1, Line 11: we lose some forensic information in doing so
such as? I mean it'd be good for this message to spell out the rough types/uses of the lost info
PS1, Line 15: It would be nice to also do it in real time (perhaps as a
: maintenance manager operation), but that requires containers to have shared
: ownership, a lifecycle change I'm not keen on making right now.
getting a bit worried that doing proper LBM maintenance will depend on restarting a server. Not a concern of this patch per se, but something to keep in mind. Likely most users are not expecting to restart the cluster except for the very occasional upgrade.
http://gerrit.cloudera.org:8080/#/c/6824/1/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
Line 1677: CHECK(to_delete->full());
add an error message.
--
To view, visit http://gerrit.cloudera.org:8080/6824
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I73a903092cf89508b7ba97fde3a2d6e691161b4c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1549: delete dead LBM containers at startup
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1549: delete dead LBM containers at startup
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/6824
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I73a903092cf89508b7ba97fde3a2d6e691161b4c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No