Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os/bluestore/bluestore_tool: add log-dump command to dump bluefs's log #18535

Merged
merged 1 commit into from Oct 31, 2017
Merged

os/bluestore/bluestore_tool: add log-dump command to dump bluefs's log #18535

merged 1 commit into from Oct 31, 2017

Conversation

yanghonggang
Copy link
Contributor

@yanghonggang yanghonggang commented Oct 25, 2017

./bin/ceph-bluestore-tool --command bluefs-log-dump --path dev/osd0/
...
log_dump log_fnode file(ino 1 size 0x1000 mtime 0.000000 bdev 0 extents [0:0x100000+400000])
log_dump 0x0: txn(seq 1 len 0x37 crc 0x3d91dc32)
log_dump 0x0: op_init
log_dump 0x0: op_alloc_add 0:0x1000~3e7ff000
...

Signed-off-by: Yang Honggang joseph.yang@xtaotech.com

log_file->fnode = super.log_fnode;
cout << __func__ << " log_fnode " << super.log_fnode << std::endl;

BlueFS::FileReader *log_reader = new BlueFS::FileReader(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we try to reuse BlueFS::_replay(bool noop) instead of duplicating it here? Otherwise it is going to be very fragile..

@yanghonggang
Copy link
Contributor Author

@xiexingguo The implementation is improved followed your suggestion.
Please help to review.

Copy link
Member

@xiexingguo xiexingguo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@ifed01
Copy link
Contributor

ifed01 commented Oct 26, 2017

In general this looks good, just one concern though. Function BlueFS::_replay() called from mount() one will dump the log to ostream irrespective of the configured debug level. Hence delaying the mount and wasting (temporary) the RAM. May be it makes sense to add some checks to bypass that useless dumping when unneeded.

@yanghonggang
Copy link
Contributor Author

You are right. When log file is very large, it will be a problem.
I have to add ugly if else statements to reuse _replay().
Any good idea? @ifed01 @xiexingguo

@varadakari
Copy link
Contributor

varadakari commented Oct 27, 2017

dump it only when it is not replayed? And how about, write to a file, instead of std::out? If the file ref is present, then write to that file, else follow the dout(), which is needed for any debugging.

@yanghonggang
Copy link
Contributor Author

@varadakari When to_stdout is true, dump to std out, else keep the original logic.

@varadakari
Copy link
Contributor

@yanghonggang dump to stdout only noop is false? that means no replay is in progress?

@yanghonggang
Copy link
Contributor Author

noop is used to control whether to replay the log(true means no). our tool only dump log's content, we should not replay the log.

@varadakari
Copy link
Contributor

@yanghonggang what i mean write to std::out only when noop == false and std_out == true.

@yanghonggang
Copy link
Contributor Author

@varadakari

what i mean write to std::out only when noop == false and std_out == true.

What's the benefit of doing that?

@yanghonggang
Copy link
Contributor Author

@ifed01 @xiexingguo Please help to review

@@ -525,6 +553,8 @@ int main(int argc, char **argv)
}
fs->umount();
delete fs;
} else if (action == "log-dump") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bluefs-log-dump? since other bluefs stuff is prefixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

<< ": " << t << std::endl;
} else {
dout(10) << __func__ << " 0x" << std::hex << pos << std::dec
<< ": " << t << dendl;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's leave the dout unconditional so it's just if (to_stdout) std::cout << ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

…fs's log

./bin/ceph-bluestore-tool --command bluefs-log-dump --path dev/osd0/
...
0x1000: txn(seq 2 len 0xd7 crc 0x306e389b)
0x1000:  op_dir_create db
0x1000:  op_dir_create db.wal
0x1000:  op_dir_create db.slow
0x1000:  op_file_update  file

Signed-off-by: Yang Honggang <joseph.yang@xtaotech.com>
@yanghonggang
Copy link
Contributor Author

@tchaikov tchaikov merged commit 529f036 into ceph:master Oct 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants