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.

Sign up for GitHub

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

Jump to bottom

Add F_BARRIERFSYNC for Sync operations on MacOS #319

Merged
merged 1 commit into from Feb 7, 2023
Merged

Add F_BARRIERFSYNC for Sync operations on MacOS #319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 19 additions & 3 deletions CMakeLists.txt
View file
Open in desktop
Expand Up @@ -587,9 +587,25 @@ if(HAVE_AUXV_GETAUXVAL)
add_definitions(-DROCKSDB_AUXV_GETAUXVAL_PRESENT)
endif()

check_cxx_symbol_exists(F_FULLFSYNC "fcntl.h" HAVE_FULLFSYNC)
if(HAVE_FULLFSYNC)
add_definitions(-DHAVE_FULLFSYNC)
set(FSYNC_MODE AUTO CACHE STRING "Enable RTTI in builds")
set_property(CACHE FSYNC_MODE PROPERTY STRINGS AUTO FULL BARRIER OFF)
if(NOT FSYNC_MODE STREQUAL "OFF")
if (NOT FSYNC_MODE STREQUAL "BARRIER")
check_cxx_symbol_exists(F_FULLFSYNC "fcntl.h" HAVE_FULLFSYNC)
if(HAVE_FULLFSYNC)
add_definitions(-DHAVE_FULLFSYNC)
elseif(FSYNC_MODE STREQUAL "FULL")
message(FATAL_ERROR "FSYNC_MODE is FULL, but unable to compile with F_FULLFSYNC")
endif()
endif()
if (NOT FSYNC_MODE STREQUAL "FULL")
check_cxx_symbol_exists(F_BARRIERFSYNC "fcntl.h" HAVE_BARRIERFSYNC)
if(HAVE_BARRIERFSYNC)
add_definitions(-DHAVE_BARRIERFSYNC)
elseif(FSYNC_MODE STREQUAL "BARRIER")
message(FATAL_ERROR "FSYNC_MODE is , but unable to compile with F_BARRIERFSYNC")
endif()
endif()
endif()

include_directories(${PROJECT_SOURCE_DIR})
Expand Down
10 changes: 10 additions & 0 deletions Makefile
View file
Open in desktop
Expand Up @@ -199,6 +199,15 @@ ifeq ($(COERCE_CONTEXT_SWITCH), 1)
OPT += -DCOERCE_CONTEXT_SWITCH
endif

# Controls the mode and switches for sync and fsync
# Valid modes are:
# - FULL: Use F_FULLFSYNC for both sync and fsync
# - BARRIER: Use F_BARRIERFSYNC for both sync and fsync
# - AUTO: Detect what is available. Favor barrier for sync, full for fsync
# (if available)
# - OFF: Use fdatasync and fsync
FSYNC_MODE ?= AUTO

#-----------------------------------------------
include src.mk

Expand Down Expand Up @@ -275,6 +284,7 @@ dummy := $(shell (export CXXFLAGS="$(EXTRA_CXXFLAGS)"; \
export LIB_MODE="$(LIB_MODE)"; \
export ROCKSDB_CXX_STANDARD="$(ROCKSDB_CXX_STANDARD)"; \
export USE_FOLLY="$(USE_FOLLY)"; \
export FSYNC_MODE="$(FSYNC_MODE)"; \
"$(CURDIR)/build_tools/build_detect_platform" "$(CURDIR)/make_config.mk"))

endif
Expand Down
30 changes: 27 additions & 3 deletions build_tools/build_detect_platform
View file
Open in desktop
Expand Up @@ -816,15 +816,39 @@ EOF
fi

# check for F_FULLFSYNC
$CXX $PLATFORM_CXXFALGS -x c++ - -o test.o 2>/dev/null <<EOF
if [ "$FSYNC_MODE" != "OFF" ]; then
if [ "$FSYNC_MODE" != "FULL" ]; then
$CXX $PLATFORM_CXXFLAGS -x c++ - -o test.o 2>/dev/null <<EOF
#include <fcntl.h>
int main() {
fcntl(0, F_BARRIERFSYNC);
return 0;
}
EOF
if [ "$?" = 0 ]; then
COMMON_FLAGS="$COMMON_FLAGS -DHAVE_BARRIERFSYNC"
elif [ "$FSYNC_MODE" == "BARRIER" ]; then
echo "Cannot compile with FSYNC_MODE " $FSYNC_MODE >&2
exit 1
fi
fi


if [ "$FSYNC_MODE" != "BARRIER" ]; then
$CXX $PLATFORM_CXXFLAGS -x c++ - -o test.o 2>/dev/null <<EOF
#include <fcntl.h>
int main() {
fcntl(0, F_FULLFSYNC);
return 0;
}
EOF
if [ "$?" = 0 ]; then
COMMON_FLAGS="$COMMON_FLAGS -DHAVE_FULLFSYNC"
if [ "$?" = 0 ]; then
COMMON_FLAGS="$COMMON_FLAGS -DHAVE_FULLFSYNC"
elif [ "$FSYNC_MODE" == "FULL" ]; then
echo "Cannot compile with FSYNC_MODE " $FSYNC_MODE >&2
exit 1
fi
fi
fi

rm -f test.o test_dl.o
Expand Down
128 changes: 57 additions & 71 deletions env/io_posix.cc
View file
Open in desktop
Expand Up @@ -102,6 +102,47 @@ int Madvise(void* addr, size_t len, int advice) {
}

namespace {
IOStatus PosixSync(int fd, const std::string& file_name,
const char* file_type) {
#if defined(HAVE_BARRIERFSYNC)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is code is the same as in the PosixFSync. do a function PosixSync that as parameter you pass if it is fsync or not and then split to the different code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about making one routine for sync/fsync, passing in an argument. But the code is NOT the same:

  • In the non-Mac case, Sync uses fdatasync and FSync uses fsync
  • In the Mac case, Sync prefers BARRIER and FULL prefers FSync but falls back to the other

I thought about merging them, but the code actually ends up more complicated. If you have a code layout that is simpler, I will take it but I could not come up with one.

Copy link
Contributor

@ayulas ayulas Jan 10, 2023 •
edited

Choose a reason for hiding this comment

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

forget. its not worth the complications. go ahead

if (::fcntl(fd, F_BARRIERFSYNC) < 0) {
std::string message = "while fcntl(F_BARRIERFSYNC) ";
return IOError(message + file_type, file_name, errno);
}
#elif defined(HAVE_FULLFSYNC)
if (::fcntl(fd, F_FULLFSYNC) < 0) {
std::string message = "while fcntl(F_FULLFSYNC) ";
return IOError(message + file_type, file_name, errno);
}
#else // HAVE_FULLFSYNC
if (fdatasync(fd) < 0) {
std::string message = "While fdatasync ";
return IOError(message + file_type, file_name, errno);
}
#endif // HAVE_FULLFSYNC
return IOStatus::OK();
}

IOStatus PosixFSync(int fd, const std::string& file_name,
const char* file_type) {
#if defined(HAVE_FULLFSYNC)
Copy link
Contributor

Choose a reason for hiding this comment

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

the same code as above

if (::fcntl(fd, F_FULLFSYNC) < 0) {
std::string message = "while fcntl(F_FULLSYNC) ";
return IOError(message + file_type, file_name, errno);
}
#elif defined(HAVE_BARRIERFSYNC)
if (::fcntl(fd, F_BARRIERFSYNC) < 0) {
std::string message = "while fcntl(F_BARRIERFSYNC) ";
return IOError(message + file_type, file_name, errno);
}
#else // HAVE_FULLFSYNC
if (fsync(fd) < 0) {
std::string message = "While fsync ";
return IOError(message + file_type, file_name, errno);
}
#endif // HAVE_FULLFSYNC
return IOStatus::OK();
}

// On MacOS (and probably *BSD), the posix write and pwrite calls do not support
// buffers larger than 2^31-1 bytes. These two wrappers fix this issue by
Expand Down Expand Up @@ -1182,35 +1223,25 @@ IOStatus PosixMmapFile::Flush(const IOOptions& /*opts*/,

IOStatus PosixMmapFile::Sync(const IOOptions& /*opts*/,
IODebugContext* /*dbg*/) {
#ifdef HAVE_FULLFSYNC
if (::fcntl(fd_, F_FULLFSYNC) < 0) {
return IOError("while fcntl(F_FULLSYNC) mmapped file", filename_, errno);
}
#else // HAVE_FULLFSYNC
if (fdatasync(fd_) < 0) {
return IOError("While fdatasync mmapped file", filename_, errno);
IOStatus s = PosixSync(fd_, filename_, "mmapped file");
Copy link
Contributor

Choose a reason for hiding this comment

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

need to call generic PosixSync with fsync false

if (!s.ok()) {
return s;
} else {
return Msync();
}
#endif // HAVE_FULLFSYNC

return Msync();
}

/**
* Flush data as well as metadata to stable storage.
*/
IOStatus PosixMmapFile::Fsync(const IOOptions& /*opts*/,
IODebugContext* /*dbg*/) {
#ifdef HAVE_FULLFSYNC
if (::fcntl(fd_, F_FULLFSYNC) < 0) {
return IOError("While fcntl(F_FULLSYNC) on mmaped file", filename_, errno);
}
#else // HAVE_FULLFSYNC
if (fsync(fd_) < 0) {
return IOError("While fsync mmaped file", filename_, errno);
auto s = PosixFSync(fd_, filename_, "mmapped file");
Copy link
Contributor

Choose a reason for hiding this comment

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

need to call generic PosixSync with fsync true

if (!s.ok()) {
return s;
} else {
return Msync();
}
#endif // HAVE_FULLFSYNC

return Msync();
}

/**
Expand Down Expand Up @@ -1400,30 +1431,12 @@ IOStatus PosixWritableFile::Flush(const IOOptions& /*opts*/,

IOStatus PosixWritableFile::Sync(const IOOptions& /*opts*/,
IODebugContext* /*dbg*/) {
#ifdef HAVE_FULLFSYNC
if (::fcntl(fd_, F_FULLFSYNC) < 0) {
return IOError("while fcntl(F_FULLFSYNC)", filename_, errno);
}
#else // HAVE_FULLFSYNC
if (fdatasync(fd_) < 0) {
return IOError("While fdatasync", filename_, errno);
}
#endif // HAVE_FULLFSYNC
return IOStatus::OK();
return PosixSync(fd_, filename_, "");
}

IOStatus PosixWritableFile::Fsync(const IOOptions& /*opts*/,
IODebugContext* /*dbg*/) {
#ifdef HAVE_FULLFSYNC
if (::fcntl(fd_, F_FULLFSYNC) < 0) {
return IOError("while fcntl(F_FULLFSYNC)", filename_, errno);
}
#else // HAVE_FULLFSYNC
if (fsync(fd_) < 0) {
return IOError("While fsync", filename_, errno);
}
#endif // HAVE_FULLFSYNC
return IOStatus::OK();
return PosixFSync(fd_, filename_, "");
}

bool PosixWritableFile::IsSyncThreadSafe() const { return true; }
Expand Down Expand Up @@ -1595,30 +1608,12 @@ IOStatus PosixRandomRWFile::Flush(const IOOptions& /*opts*/,

IOStatus PosixRandomRWFile::Sync(const IOOptions& /*opts*/,
IODebugContext* /*dbg*/) {
#ifdef HAVE_FULLFSYNC
if (::fcntl(fd_, F_FULLFSYNC) < 0) {
return IOError("while fcntl(F_FULLFSYNC) random rw file", filename_, errno);
}
#else // HAVE_FULLFSYNC
if (fdatasync(fd_) < 0) {
return IOError("While fdatasync random read/write file", filename_, errno);
}
#endif // HAVE_FULLFSYNC
return IOStatus::OK();
return PosixSync(fd_, filename_, "random read/write file");
}

IOStatus PosixRandomRWFile::Fsync(const IOOptions& /*opts*/,
IODebugContext* /*dbg*/) {
#ifdef HAVE_FULLFSYNC
if (::fcntl(fd_, F_FULLFSYNC) < 0) {
return IOError("While fcntl(F_FULLSYNC) random rw file", filename_, errno);
}
#else // HAVE_FULLFSYNC
if (fsync(fd_) < 0) {
return IOError("While fsync random read/write file", filename_, errno);
}
#endif // HAVE_FULLFSYNC
return IOStatus::OK();
return PosixFSync(fd_, filename_, "random read/write file");
}

IOStatus PosixRandomRWFile::Close(const IOOptions& /*opts*/,
Expand Down Expand Up @@ -1713,18 +1708,9 @@ IOStatus PosixDirectory::FsyncWithDirOptions(
// skip fsync/fcntl when fd_ == -1 since this file descriptor has been closed
// in either the de-construction or the close function, data must have been
// fsync-ed before de-construction and close is called
#ifdef HAVE_FULLFSYNC
// btrfs is a Linux file system, while currently F_FULLFSYNC is available on
// Mac OS.
assert(!is_btrfs_);
if (fd_ != -1 && ::fcntl(fd_, F_FULLFSYNC) < 0) {
return IOError("while fcntl(F_FULLFSYNC)", "a directory", errno);
if (fd_ != -1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

same as my comment above

s = PosixFSync(fd_, "", "a directory");
}
#else // HAVE_FULLFSYNC
if (fd_ != -1 && fsync(fd_) == -1) {
s = IOError("While fsync", "a directory", errno);
}
#endif // HAVE_FULLFSYNC
#endif // OS_AIX
return s;
}
Expand Down