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/BlueFS: add bluefs_sync_write option #14510

Merged
merged 3 commits into from May 5, 2017

Conversation

Projects
None yet
4 participants
@liewegas
Member

liewegas commented Apr 13, 2017

No description provided.

@liewegas liewegas added the bluestore label Apr 13, 2017

@liewegas liewegas requested a review from ifed01 Apr 14, 2017

@@ -107,6 +107,10 @@ class BlockDevice {
uint64_t len,
char *buf,
bool buffered) = 0;
virtual int write(

This comment has been minimized.

@ifed01

ifed01 Apr 14, 2017

Contributor

This will break NVMEDevice build if any. Need implementation there too

@ifed01

ifed01 Apr 14, 2017

Contributor

This will break NVMEDevice build if any. Need implementation there too

Show outdated Hide outdated src/os/bluestore/BlueFS.cc
completed_ios.clear();
flush_bdev();
lock.lock();
if (!cct->_conf->bluefs_sync_write) {

This comment has been minimized.

@ifed01

ifed01 Apr 14, 2017

Contributor

copy-paste of a block in _flush_and_sync_log, make a single func?

@ifed01

ifed01 Apr 14, 2017

Contributor

copy-paste of a block in _flush_and_sync_log, make a single func?

Show outdated Hide outdated src/os/bluestore/KernelDevice.cc
@@ -525,6 +525,60 @@ void KernelDevice::aio_submit(IOContext *ioc)
}
}
int KernelDevice::write(

This comment has been minimized.

@ifed01

ifed01 Apr 14, 2017

Contributor

I'd suggest to reuse the sync path from aio_write code as much as possible here.... may be make a standalone internal func?

@ifed01

ifed01 Apr 14, 2017

Contributor

I'd suggest to reuse the sync path from aio_write code as much as possible here.... may be make a standalone internal func?

@ifed01

ifed01 approved these changes Apr 19, 2017

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Apr 20, 2017

Member

retest this please

Member

liewegas commented Apr 20, 2017

retest this please

@tchaikov

This comment has been minimized.

Show comment
Hide comment
@tchaikov

tchaikov May 2, 2017

Contributor

needs rebase

Contributor

tchaikov commented May 2, 2017

needs rebase

liewegas added some commits Apr 13, 2017

os/bluestore/KernelBlockDevice: sync write() method
Avoid aio machinery (and context switching) and do a simple
synchronous (O_DIRECT) write.

Signed-off-by: Sage Weil <sage@redhat.com>
os/bluestore/BlueFS: _write_super is always synchronous
Signed-off-by: Sage Weil <sage@redhat.com>
os/bluestore/BlueFS: add bluefs_sync_write option
If we have a fast device we can do our writes using synchronous
IO instead of aio.  Most of the time rocksdb is doing sync writes
anyway (write and then fsync from the same thread).

Note that this might not be the case when using the
bluestore_sync_submit_transaction mode... that probably should
not be combined with bluefs_sync_write.

Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas merged commit 7c8a2a1 into ceph:master May 5, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@liewegas liewegas deleted the liewegas:wip-bluefs-sync-write branch May 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment