You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2018/07/31 17:58:26 UTC

[kudu-CR] Expose the block manager in ReadableBlocks

Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11091


Change subject: Expose the block manager in ReadableBlocks
......................................................................

Expose the block manager in ReadableBlocks

This is a preliminary patch that could be useful in
the fix for KUDU-2469.

Change-Id: Icc346177b29cda7a49a83cedfbd5fec2962d330f
---
M src/kudu/fs/block_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/fs-test-util.h
M src/kudu/fs/log_block_manager.cc
4 files changed, 19 insertions(+), 0 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icc346177b29cda7a49a83cedfbd5fec2962d330f
Gerrit-Change-Number: 11091
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>

[kudu-CR] Expose the block manager in ReadableBlocks

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

Change subject: Expose the block manager in ReadableBlocks
......................................................................


Patch Set 1: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc346177b29cda7a49a83cedfbd5fec2962d330f
Gerrit-Change-Number: 11091
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 31 Jul 2018 21:15:46 +0000
Gerrit-HasComments: No

[kudu-CR] Expose the block manager in ReadableBlocks

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

Change subject: Expose the block manager in ReadableBlocks
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11091/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11091/1//COMMIT_MSG@9
PS1, Line 9: This is a preliminary patch that could be useful in
           : the fix for KUDU-2469.
> It's hard to imagine how you'd have access to the ReadableBlock but not to 
One example is that CFileReaders take ownership of ReadableBlocks and have no knowledge of the rest of the FS layer.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc346177b29cda7a49a83cedfbd5fec2962d330f
Gerrit-Change-Number: 11091
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 31 Jul 2018 18:30:45 +0000
Gerrit-HasComments: Yes

[kudu-CR] Expose the block manager in ReadableBlocks

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

Change subject: Expose the block manager in ReadableBlocks
......................................................................


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc346177b29cda7a49a83cedfbd5fec2962d330f
Gerrit-Change-Number: 11091
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 31 Jul 2018 19:12:19 +0000
Gerrit-HasComments: No

[kudu-CR] Expose the block manager in ReadableBlocks

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

Change subject: Expose the block manager in ReadableBlocks
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11091/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11091/1//COMMIT_MSG@9
PS1, Line 9: This is a preliminary patch that could be useful in
           : the fix for KUDU-2469.
> It is already. This is a mirror of that implementation.
Ugh, sorry for missing that.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc346177b29cda7a49a83cedfbd5fec2962d330f
Gerrit-Change-Number: 11091
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 31 Jul 2018 19:12:17 +0000
Gerrit-HasComments: Yes

[kudu-CR] Expose the block manager in ReadableBlocks

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

Change subject: Expose the block manager in ReadableBlocks
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11091/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11091/1//COMMIT_MSG@9
PS1, Line 9: This is a preliminary patch that could be useful in
           : the fix for KUDU-2469.
It's hard to imagine how you'd have access to the ReadableBlock but not to the block manager itself (which I think you can get from the FsManager). Could you provide an example?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc346177b29cda7a49a83cedfbd5fec2962d330f
Gerrit-Change-Number: 11091
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 31 Jul 2018 18:23:42 +0000
Gerrit-HasComments: Yes

[kudu-CR] Expose the block manager in ReadableBlocks

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

Change subject: Expose the block manager in ReadableBlocks
......................................................................

Expose the block manager in ReadableBlocks

This is a preliminary patch that could be useful in
the fix for KUDU-2469.

Change-Id: Icc346177b29cda7a49a83cedfbd5fec2962d330f
Reviewed-on: http://gerrit.cloudera.org:8080/11091
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Grant Henke <gr...@apache.org>
---
M src/kudu/fs/block_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/fs-test-util.h
M src/kudu/fs/log_block_manager.cc
4 files changed, 19 insertions(+), 0 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Grant Henke: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icc346177b29cda7a49a83cedfbd5fec2962d330f
Gerrit-Change-Number: 11091
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] Expose the block manager in ReadableBlocks

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.

Change subject: Expose the block manager in ReadableBlocks
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/11091
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Icc346177b29cda7a49a83cedfbd5fec2962d330f
Gerrit-Change-Number: 11091
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] Expose the block manager in ReadableBlocks

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

Change subject: Expose the block manager in ReadableBlocks
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11091/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11091/1//COMMIT_MSG@9
PS1, Line 9: This is a preliminary patch that could be useful in
           : the fix for KUDU-2469.
> Hmm, OK. Can we expose it in WritableBlocks too then?
It is already. This is a mirror of that implementation.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc346177b29cda7a49a83cedfbd5fec2962d330f
Gerrit-Change-Number: 11091
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 31 Jul 2018 18:46:55 +0000
Gerrit-HasComments: Yes

[kudu-CR] Expose the block manager in ReadableBlocks

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

Change subject: Expose the block manager in ReadableBlocks
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11091/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11091/1//COMMIT_MSG@9
PS1, Line 9: This is a preliminary patch that could be useful in
           : the fix for KUDU-2469.
> One example is that CFileReaders take ownership of ReadableBlocks and have 
Hmm, OK. Can we expose it in WritableBlocks too then?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc346177b29cda7a49a83cedfbd5fec2962d330f
Gerrit-Change-Number: 11091
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 31 Jul 2018 18:43:00 +0000
Gerrit-HasComments: Yes