You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Fengling Wang (Code Review)" <ge...@cloudera.org> on 2018/05/18 23:30:26 UTC
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Fengling Wang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10456
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
KUDU-2314 Add wal and metadata directories info to FsReport
I added wal and metadata directories info in BlockManagerOptions
just like what was done in FsManagerOptions, and refactored FBM,
LBM, and related tests so that the these two directories info can
be reflected on the FsReport.
Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
---
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/fs_report.cc
M src/kudu/fs/fs_report.h
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
8 files changed, 34 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/10456/1
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 1
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10456/1/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
http://gerrit.cloudera.org:8080/#/c/10456/1/src/kudu/fs/log_block_manager.cc@1825
PS1, Line 1825: // Record the wal and metadata directory paths
> If this is the right place to insert wal and md dirs, maybe we need to do s
I think it makes sense to handle this reporting at the FsManager layer instead, since it already knows about the WALs and metadata directories. Doing it in the block managers and piping these down here seems a bit odd since the block managers are kind of unrelated to WALs and metadata.
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 1
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 18 May 2018 23:59:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10456/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10456/4//COMMIT_MSG@7
PS4, Line 7: tadata directories info to FsReport
:
nit: a sample output of the new report would be nice
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 4
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 23 May 2018 22:31:39 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
KUDU-2314 Add wal and metadata directories info to FsReport
Sample Output:
I0523 15:33:13.167342 2372129664 fs_report.cc:352] FS layout report
--------------------
wal directory: /private/tmp/kudu/wal/master/0
metadata directory: /private/tmp/kudu/wal/master/0
1 data directories: /private/tmp/kudu/data/master/0/data
Total live blocks: 0
Total live bytes: 0
Total live bytes (after alignment): 0
Total number of LBM containers: 0 (0 full)
Did not check for missing blocks
Did not check for orphaned blocks
Did not check for full LBM containers with extra space
Did not check for incomplete LBM containers
Did not check for malformed LBM records
Did not check for misaligned LBM blocks
Did not check for partial LBM records
Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Reviewed-on: http://gerrit.cloudera.org:8080/10456
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_report.cc
M src/kudu/fs/fs_report.h
3 files changed, 19 insertions(+), 1 deletion(-)
Approvals:
Kudu Jenkins: Verified
Andrew Wong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 6
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc@416
PS2, Line 416: // Report wal and metadata directories
Nit: terminate with a period.
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc@417
PS2, Line 417: FsReport local_report;
Shouldn't this whole section just be something like this?
if (report) {
report->wal_dir = ...;
report->metadata_dir = ...;
}
Why the indirection via local_report? And why the call to LogAndCheckForFatalErrors?
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc@419
PS2, Line 419: GetConsensusMetadataDir
> Prob shouldn't be consensus metadata here?
Nope. That'll return the /cmeta subdirectory inside the metadata directory itself.
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_report.h
File src/kudu/fs/fs_report.h:
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_report.h@256
PS2, Line 256: // WAL directory
Nit: terminate with period. Below too.
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_report.cc
File src/kudu/fs/fs_report.cc:
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_report.cc@281
PS2, Line 281: void FsReport::MergeFrom(const FsReport& other) {
We should probably CHECK/DCHECK that both reports agree on the WAL/metadata directories.
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 2
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 22 May 2018 19:53:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Fengling Wang (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10456
to look at the new patch set (#2).
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
KUDU-2314 Add wal and metadata directories info to FsReport
Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
---
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_report.cc
M src/kudu/fs/fs_report.h
3 files changed, 19 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/10456/2
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 2
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10456/3/src/kudu/fs/fs_report.cc
File src/kudu/fs/fs_report.cc:
http://gerrit.cloudera.org:8080/#/c/10456/3/src/kudu/fs/fs_report.cc@304
PS3, Line 304: DCHECK_EQ(metadata_dir, other.metadata_dir);
: DCHECK_EQ(wal_dir, other.wal_dir);
Nit: let's do this earlier, at the beginning of MergeFrom.
As a general rule, if there's CHECK/DCHECK work to do, best to do it upon entry to the function, so that it also serves as a visual precondition for anyone who is reading through the code.
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 3
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 23 May 2018 22:04:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Fengling Wang (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10456
to look at the new patch set (#3).
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
KUDU-2314 Add wal and metadata directories info to FsReport
Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
---
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_report.cc
M src/kudu/fs/fs_report.h
3 files changed, 19 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/10456/3
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 3
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc@419
PS2, Line 419: GetConsensusMetadataDir
> Nope. That'll return the /cmeta subdirectory inside the metadata directory
You should be able to use `canonicalized_wal_fs_root_.path` and `canonicalized_metadata_fs_root_.path` for `report.wal_dir` and `report.metadata_dir` respectively
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_report.cc
File src/kudu/fs/fs_report.cc:
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_report.cc@309
PS2, Line 309: s += "wal directory: " + wal_dir + "\n";
: s += "metadata directory: " + metadata_dir + "\n";
Maybe change the logging a bit to reflect that this now includes more than the block manager. Maybe something like:
FS layout report
----------------
wal directory: ...
metadata directory: ...
N data directories: ...
<all block manager checks>
... or something?
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 2
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 22 May 2018 20:10:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc@417
PS2, Line 417: FsReport local_report;
> I followed the style from log_block_manager.cc. Maybe it's not necessary he
IIRC log_block_manager.cc does it that way because it uses the "local" report in Repair(), even if the user passed nullptr for 'report'. So it's not a particular style so much as driven by necessity.
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 2
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 22 May 2018 20:57:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10456/1/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
http://gerrit.cloudera.org:8080/#/c/10456/1/src/kudu/fs/log_block_manager.cc@1825
PS1, Line 1825: // Record the wal and metadata directory paths
> I think it makes sense to handle this reporting at the FsManager layer inst
Agreed with Andrew. The original intent behind FsReport was to allow any module from the FsManager on down to modify it. The idea is that the FsManager creates it, passes it down the stack in Open(), then prints the results. Along the way, the block manager might modify it. Or the directory manager. Or the FsManager itself, etc.
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 1
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 21 May 2018 17:14:05 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 5
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 23 May 2018 23:19:14 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Fengling Wang (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10456
to look at the new patch set (#5).
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
KUDU-2314 Add wal and metadata directories info to FsReport
Sample Output:
I0523 15:33:13.167342 2372129664 fs_report.cc:352] FS layout report
--------------------
wal directory: /private/tmp/kudu/wal/master/0
metadata directory: /private/tmp/kudu/wal/master/0
1 data directories: /private/tmp/kudu/data/master/0/data
Total live blocks: 0
Total live bytes: 0
Total live bytes (after alignment): 0
Total number of LBM containers: 0 (0 full)
Did not check for missing blocks
Did not check for orphaned blocks
Did not check for full LBM containers with extra space
Did not check for incomplete LBM containers
Did not check for malformed LBM records
Did not check for misaligned LBM blocks
Did not check for partial LBM records
Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
---
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_report.cc
M src/kudu/fs/fs_report.h
3 files changed, 19 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/10456/5
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 5
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Fengling Wang (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10456
to look at the new patch set (#4).
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
KUDU-2314 Add wal and metadata directories info to FsReport
Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
---
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_report.cc
M src/kudu/fs/fs_report.h
3 files changed, 19 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/10456/4
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 4
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Fengling Wang (Code Review)" <ge...@cloudera.org>.
Fengling Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 1:
(1 comment)
There are probably quite a few things left to do for this patch. But I'm not sure if I'm on the right track, so I pushed this minimum patch and I'm waiting for suggestions.
There's no dedicated test for now. And I verified the result by hand, only for FBM. (Cannot verify it on LBM cuz I'm using MAC)
Thanks!
http://gerrit.cloudera.org:8080/#/c/10456/1/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
http://gerrit.cloudera.org:8080/#/c/10456/1/src/kudu/fs/log_block_manager.cc@1825
PS1, Line 1825: // Record the wal and metadata directory paths
If this is the right place to insert wal and md dirs, maybe we need to do some path checking just like what's done in FsManager::Init? Same for FBM.
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 1
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 18 May 2018 23:44:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Fengling Wang (Code Review)" <ge...@cloudera.org>.
Fengling Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc@416
PS2, Line 416: // Report wal and metadata directories
> Nit: terminate with a period.
Done
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc@417
PS2, Line 417: FsReport local_report;
> IIRC log_block_manager.cc does it that way because it uses the "local" repo
I guess I misunderstood sth again. But in all the tests, the calls to this 'Open' function are with the 'report' param being null. If so, how can I ever show the wal and metadata dirs?
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc@419
PS2, Line 419: GetConsensusMetadataDir
> You should be able to use `canonicalized_wal_fs_root_.path` and `canonicali
I see. thx.
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_report.h
File src/kudu/fs/fs_report.h:
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_report.h@256
PS2, Line 256: // WAL directory
> Nit: terminate with period. Below too.
Done
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_report.cc
File src/kudu/fs/fs_report.cc:
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_report.cc@309
PS2, Line 309: s += "wal directory: " + wal_dir + "\n";
: s += "metadata directory: " + metadata_dir + "\n";
> Maybe change the logging a bit to reflect that this now includes more than
Done
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 2
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 22 May 2018 22:43:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Fengling Wang (Code Review)" <ge...@cloudera.org>.
Fengling Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc@419
PS2, Line 419: GetConsensusMetadataDir
Prob shouldn't be consensus metadata here?
http://gerrit.cloudera.org:8080/#/c/10456/1/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
http://gerrit.cloudera.org:8080/#/c/10456/1/src/kudu/fs/log_block_manager.cc@1825
PS1, Line 1825: // Either return or log the report.
> I think it makes sense to handle this reporting at the FsManager layer inst
Done
http://gerrit.cloudera.org:8080/#/c/10456/1/src/kudu/fs/log_block_manager.cc@1825
PS1, Line 1825: // Either return or log the report.
> Agreed with Andrew. The original intent behind FsReport was to allow any mo
Ah I see. Thanks. I misunderstood how the functions are used previously.
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 2
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 22 May 2018 19:43:13 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc@417
PS2, Line 417: FsReport local_report;
> I guess I misunderstood sth again. But in all the tests, the calls to this
Right, that's because the Open() function predates the reporting mechanism, so when I added it, it made more sense to default the report argument in Open() to nullptr to avoid having to update a ton of tests. If you look at log_block_manager-test.cc you should see some tests that use the report meaningfully.
In production, ServerBase::Init() calls Open() and then LogAndCheckForFatalError() afterwards.
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 2
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 23 May 2018 04:39:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 4
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 23 May 2018 22:14:10 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Fengling Wang (Code Review)" <ge...@cloudera.org>.
Fengling Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10456/3/src/kudu/fs/fs_report.cc
File src/kudu/fs/fs_report.cc:
http://gerrit.cloudera.org:8080/#/c/10456/3/src/kudu/fs/fs_report.cc@304
PS3, Line 304: DCHECK_EQ(metadata_dir, other.metadata_dir);
: DCHECK_EQ(wal_dir, other.wal_dir);
> Nit: let's do this earlier, at the beginning of MergeFrom.
I see. Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 3
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 23 May 2018 22:08:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Fengling Wang (Code Review)" <ge...@cloudera.org>.
Fengling Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc@417
PS2, Line 417: FsReport local_report;
> Shouldn't this whole section just be something like this?
I followed the style from log_block_manager.cc. Maybe it's not necessary here?
http://gerrit.cloudera.org:8080/#/c/10456/2/src/kudu/fs/fs_manager.cc@419
PS2, Line 419: GetConsensusMetadataDir
> Nope. That'll return the /cmeta subdirectory inside the metadata directory
There's no direct function that gives me back the metadata dir. Should I create one?
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 2
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 22 May 2018 20:08:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2314 Add wal and metadata directories info to FsReport
Posted by "Fengling Wang (Code Review)" <ge...@cloudera.org>.
Fengling Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/10456 )
Change subject: KUDU-2314 Add wal and metadata directories info to FsReport
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10456/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10456/4//COMMIT_MSG@7
PS4, Line 7: tadata directories info to FsReport
:
> nit: a sample output of the new report would be nice
Done
--
To view, visit http://gerrit.cloudera.org:8080/10456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1fd362582ea84696db2aa64a83c6d3e86c78ae
Gerrit-Change-Number: 10456
Gerrit-PatchSet: 4
Gerrit-Owner: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 23 May 2018 22:55:50 +0000
Gerrit-HasComments: Yes