Skip to content

Commit

Permalink
Convert RaidFileWrite to use FileStream to write stripes
Browse files Browse the repository at this point in the history 
Enable exclusive locking of stripe files on all platforms, not just Windows and
O_EXLOCK.
  • Loading branch information
@qris
qris committed Dec 17, 2017
1 parent 4a83ecc commit d671450
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 84 deletions.
3 changes: 1 addition & 2 deletions lib/common/CommonException.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,4 @@ InvalidConfiguration 52 Some required values are missing or incorrect in the c
IOStreamTimedOut 53 A network operation timed out
SignalReceived 54 The function call returned because the process received a signal
NamedLockFailed 55 Failed to acquire a named lock
FileLockFailed 56 Failed to acquire a lock on a file for an unexpected reason
FileLockingConflict 57 The file is already locked by another process
FileLockingConflict 56 The file is already locked by another process
99 changes: 52 additions & 47 deletions lib/common/FileStream.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,21 @@
// --------------------------------------------------------------------------

#include "Box.h"

#include <errno.h>

#ifdef HAVE_FCNTL_G
#include <fcntl.h>
#endif

#ifdef HAVE_SYS_FILE_H
#include <sys/file.h>
#endif

#include "FileStream.h"
#include "CommonException.h"
#include "Logging.h"

#include <errno.h>

#include "MemLeakFindOn.h"

// --------------------------------------------------------------------------
Expand All @@ -24,11 +33,11 @@
// Created: 2003/07/31
//
// --------------------------------------------------------------------------
FileStream::FileStream(const std::string& rFilename, int flags, int mode,
FileStream::FileStream(const std::string& mFileName, int flags, int mode,
lock_mode_t lock_mode)
: mOSFileHandle(INVALID_FILE),
mIsEOF(false),
mFileName(rFilename)
mFileName(mFileName)
{
OpenFile(flags, mode, lock_mode);
}
Expand Down Expand Up @@ -144,63 +153,59 @@ void FileStream::OpenFile(int flags, int mode, lock_mode_t lock_mode)
}
}

try
{
bool lock_failed = false;
bool lock_failed = false;

#ifdef BOX_LOCK_TYPE_FLOCK
BOX_TRACE("Trying to lock " << rFilename << " " << lock_message);
if(::flock(fd, (lock_mode == SHARED ? LOCK_SH : LOCK_EX) | LOCK_NB) != 0)
BOX_TRACE("Trying to lock " << mFileName << " " << lock_message);
if(::flock(mOSFileHandle, (lock_mode == SHARED ? LOCK_SH : LOCK_EX) | LOCK_NB) != 0)
{
Close();

if(errno == EWOULDBLOCK)
{
if(errno == EWOULDBLOCK)
{
lock_failed = true;
}
else
{
THROW_SYS_FILE_ERROR("Failed to lock lockfile " << lock_method_name,
mFileName, CommonException, OSFileError);
}
lock_failed = true;
}
else
{
THROW_SYS_FILE_ERROR("Failed to lock lockfile " << lock_method_name,
mFileName, CommonException, OSFileError);
}
}
#elif defined BOX_LOCK_TYPE_F_SETLK || defined BOX_LOCK_TYPE_F_OFD_SETLK
struct flock desc;
desc.l_type = (lock_mode == SHARED ? F_RDLCK : F_WRLCK);
desc.l_whence = SEEK_SET;
desc.l_start = 0;
desc.l_len = 0;
desc.l_pid = 0;
BOX_TRACE("Trying to lock " << mFileName << " " << lock_message);
struct flock desc;
desc.l_type = (lock_mode == SHARED ? F_RDLCK : F_WRLCK);
desc.l_whence = SEEK_SET;
desc.l_start = 0;
desc.l_len = 0;
desc.l_pid = 0;
BOX_TRACE("Trying to lock " << mFileName << " " << lock_message);
# if defined BOX_LOCK_TYPE_F_OFD_SETLK
if(::fcntl(fd, F_OFD_SETLK, &desc) != 0)
if(::fcntl(mOSFileHandle, F_OFD_SETLK, &desc) != 0)
# else // BOX_LOCK_TYPE_F_SETLK
if(::fcntl(fd, F_SETLK, &desc) != 0)
if(::fcntl(mOSFileHandle, F_SETLK, &desc) != 0)
# endif
{
Close();

if(errno == EAGAIN)
{
if(errno == EAGAIN)
{
lock_failed = true;
}
else
{
THROW_SYS_FILE_ERROR("Failed to lock lockfile " << lock_method_name,
mFileName, CommonException, OSFileError);
}
lock_failed = true;
}
#endif

if(lock_failed)
else
{
// We failed to lock the file, which means that it's locked by someone else.
// Need to throw a specific exception that's expected by NamedLock, which
// should just return false in this case (and only this case).
THROW_EXCEPTION_MESSAGE(CommonException, FileLockingConflict,
BOX_FILE_MESSAGE(mFileName, "File already locked by another process"));
THROW_SYS_FILE_ERROR("Failed to lock lockfile " << lock_method_name,
mFileName, CommonException, OSFileError);
}
}
catch(BoxException &e)
#endif

if(lock_failed)
{
THROW_FILE_ERROR("Failed to lock file: " << e.what(), mFileName,
CommonException, FileLockFailed);
// We failed to lock the file, which means that it's locked by someone else.
// Need to throw a specific exception that's expected by NamedLock, which
// should just return false in this case (and only this case).
THROW_EXCEPTION_MESSAGE(CommonException, FileLockingConflict,
BOX_FILE_MESSAGE(mFileName, "File already locked by another process"));
}
}

Expand Down
11 changes: 0 additions & 11 deletions lib/common/NamedLock.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,6 @@

#include "Box.h"

#include <fcntl.h>
#include <errno.h>

#ifdef HAVE_UNISTD_H
#include <unistd.h>
#endif

#ifdef HAVE_FLOCK
#include <sys/file.h>
#endif

#include "CommonException.h"
#include "NamedLock.h"
#include "Utils.h"
Expand Down
42 changes: 18 additions & 24 deletions lib/raidfile/RaidFileWrite.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,15 +527,12 @@ void RaidFileWrite::TransformToRaidStorage()
// Then open them all for writing (in strict order)
try
{
#if HAVE_DECL_O_EXLOCK
FileHandleGuard<(O_WRONLY | O_CREAT | O_EXCL | O_EXLOCK | O_BINARY)> stripe1(stripe1FilenameW.c_str());
FileHandleGuard<(O_WRONLY | O_CREAT | O_EXCL | O_EXLOCK | O_BINARY)> stripe2(stripe2FilenameW.c_str());
FileHandleGuard<(O_WRONLY | O_CREAT | O_EXCL | O_EXLOCK | O_BINARY)> parity(parityFilenameW.c_str());
#else
FileHandleGuard<(O_WRONLY | O_CREAT | O_EXCL | O_BINARY)> stripe1(stripe1FilenameW.c_str());
FileHandleGuard<(O_WRONLY | O_CREAT | O_EXCL | O_BINARY)> stripe2(stripe2FilenameW.c_str());
FileHandleGuard<(O_WRONLY | O_CREAT | O_EXCL | O_BINARY)> parity(parityFilenameW.c_str());
#endif
FileStream stripe1(stripe1FilenameW, O_WRONLY | O_CREAT | O_EXCL | O_BINARY, 0755,
FileStream::EXCLUSIVE);
FileStream stripe2(stripe2FilenameW, O_WRONLY | O_CREAT | O_EXCL | O_BINARY, 0755,
FileStream::EXCLUSIVE);
FileStream parity(parityFilenameW, O_WRONLY | O_CREAT | O_EXCL | O_BINARY, 0755,
FileStream::EXCLUSIVE);

// Then... read in data...
int bytesRead = -1;
Expand Down Expand Up @@ -615,10 +612,7 @@ void RaidFileWrite::TransformToRaidStorage()
}

// Write block
if(::write(parity, parityBuffer, parityWriteSize) != parityWriteSize)
{
THROW_EXCEPTION(RaidFileException, OSError)
}
parity.Write(parityBuffer, parityWriteSize);
}

// Write stripes
Expand All @@ -629,11 +623,14 @@ void RaidFileWrite::TransformToRaidStorage()
int toWrite = (l == (blocksToDo - 1))
?(bytesRead - ((blocksToDo-1)*blockSize))
:blockSize;
if(::write(((l&1)==0)?stripe1:stripe2, writeFrom, toWrite) != toWrite)
if((l & 1) == 0)
{
stripe1.Write(writeFrom, toWrite);
}
else
{
THROW_SYS_FILE_ERROR("Failed to write to permanent RAID file",
writeFilename, RaidFileException, OSError);
}
stripe2.Write(writeFrom, toWrite);
}

// Next block
writeFrom += blockSize;
Expand All @@ -642,6 +639,7 @@ void RaidFileWrite::TransformToRaidStorage()
// Count of blocks done
blocksDone += blocksToDo;
}

// Error on read?
if(bytesRead == -1)
{
Expand All @@ -661,12 +659,8 @@ void RaidFileWrite::TransformToRaidStorage()
{
ASSERT(sizeof(writeFileStat.st_size) <= sizeof(RaidFileRead::FileSizeType));
RaidFileRead::FileSizeType sw = box_hton64(writeFileStat.st_size);
ASSERT((::lseek(parity, 0, SEEK_CUR) % blockSize) == 0);
if(::write(parity, &sw, sizeof(sw)) != sizeof(sw))
{
THROW_SYS_FILE_ERROR("Failed to write to file", writeFilename,
RaidFileException, OSError);
}
ASSERT((parity.GetPosition() % blockSize) == 0);
parity.Write(&sw, sizeof(sw));
}

// Then close the written files (note in reverse order of opening)
Expand All @@ -675,7 +669,7 @@ void RaidFileWrite::TransformToRaidStorage()
stripe1.Close();

#ifdef WIN32
// Must delete before renaming
// Must delete any existing file before renaming over it
#define CHECK_UNLINK(file) \
{ \
if (EMU_UNLINK(file) != 0 && errno != ENOENT) \
Expand Down

0 comments on commit d671450

Please sign in to comment.