Verified Commit f4650abf authored by Marc Vef's avatar Marc Vef
Browse files

Adding asserts, fixing comments, removing pass-by-value

parent 46be69b9

include/daemon/backend/data/chunk_storage.hpp

+1 −1
Original line number Diff line number Diff line
@@ -59,7 +59,7 @@ private: 
    void init_chunk_space(const std::string& file_path) const;



public:

    ChunkStorage(std::string path, size_t chunksize);

    ChunkStorage(std::string& path, size_t chunksize);



    void destroy_chunk_space(const std::string& file_path) const;

include/daemon/ops/data.hpp

+6 −6
Original line number Diff line number Diff line
@@ -64,14 +64,14 @@ class ChunkOperation { 


protected:



    std::string path_;

    const std::string path_;



    std::vector<ABT_task> abt_tasks_;

    std::vector<ABT_eventual> task_eventuals_;



    virtual void cancel_all_tasks();



    explicit ChunkOperation(std::string path);

    explicit ChunkOperation(const std::string& path);



    ChunkOperation(std::string path, size_t n);
@@ -94,9 +94,9 @@ private: 


public:



    explicit ChunkTruncateOperation(std::string path);

    explicit ChunkTruncateOperation(const std::string& path);



    ChunkTruncateOperation(std::string path, size_t n);

    ChunkTruncateOperation(const std::string& path, size_t n);



    void cancel_all_tasks() override;
@@ -123,7 +123,7 @@ private: 


public:



    ChunkWriteOperation(std::string path, size_t n);

    ChunkWriteOperation(const std::string& path, size_t n);



    void cancel_all_tasks() override;
@@ -163,7 +163,7 @@ public: 
        std::vector<uint64_t>* chunk_ids;

    };



    ChunkReadOperation(std::string path, size_t n);

    ChunkReadOperation(const std::string& path, size_t n);



    void cancel_all_tasks() override;

src/daemon/backend/data/chunk_storage.cpp

+16 −17
Original line number Diff line number Diff line
@@ -49,10 +49,10 @@ string ChunkStorage::get_chunk_path(const string& file_path, gkfs::rpc::chnk_id_ 
}



/**

 * Creates a chunk directories are all chunk files are placed in.

 * Creates a chunk directory that all chunk files are placed in.

 * The path to the real file will be used as the directory name

 * @param file_path

 * @returns 0 on success or errno on failure

 * @throws ChunkStorageException on error

 */

void ChunkStorage::init_chunk_space(const string& file_path) const {

    auto chunk_dir = absolute(get_chunks_dir(file_path));
@@ -72,8 +72,8 @@ void ChunkStorage::init_chunk_space(const string& file_path) const { 
 * @param chunksize

 * @throws ChunkStorageException

 */

ChunkStorage::ChunkStorage(string path, const size_t chunksize) :

        root_path_(std::move(path)),

ChunkStorage::ChunkStorage(string& path, const size_t chunksize) :

        root_path_(path),

        chunksize_(chunksize) {

    /* Initialize logger */

    log_ = spdlog::get(LOGGER_NAME);
@@ -107,7 +107,7 @@ void ChunkStorage::destroy_chunk_space(const string& file_path) const { 


/**

 * Writes a chunk file.

 * On failure returns a negative error code corresponding to `-errno`.

 * On failure throws ChunkStorageException with encapsulated error code

 *

 * Refer to https://www.gnu.org/software/libc/manual/html_node/I_002fO-Primitives.html for pwrite behavior

 *
@@ -152,7 +152,7 @@ ChunkStorage::write_chunk(const string& file_path, gkfs::rpc::chnk_id_t chunk_id 


/**

 * Read from a chunk file.

 * On failure returns a negative error code corresponding to `-errno`.

 * On failure throws ChunkStorageException with encapsulated error code

 *

 * Refer to https://www.gnu.org/software/libc/manual/html_node/I_002fO-Primitives.html for pread behavior

 * @param file_path
@@ -161,6 +161,7 @@ ChunkStorage::write_chunk(const string& file_path, gkfs::rpc::chnk_id_t chunk_id 
 * @param size

 * @param offset

 * @param eventual

 * @throws ChunkStorageException (caller will handle eventual signalling)

 */

ssize_t

ChunkStorage::read_chunk(const string& file_path, gkfs::rpc::chnk_id_t chunk_id, char* buf, size_t size,
@@ -224,10 +225,8 @@ ChunkStorage::read_chunk(const string& file_path, gkfs::rpc::chnk_id_t chunk_id, 
*

* If an error is encountered when removing a chunk file, the function will still remove all files and

* report the error afterwards with ChunkStorageException.

 *

 * @param file_path

 * @param chunk_start

 * @param chunk_end

 * @throws ChunkStorageException

 */

void ChunkStorage::trim_chunk_space(const string& file_path, gkfs::rpc::chnk_id_t chunk_start) {
@@ -237,7 +236,7 @@ void ChunkStorage::trim_chunk_space(const string& file_path, gkfs::rpc::chnk_id_ 
    auto err_flag = false;

    for (bfs::directory_iterator chunk_file(chunk_dir); chunk_file != end; ++chunk_file) {

        auto chunk_path = chunk_file->path();

        auto chunk_id = ::stoul(chunk_path.filename().c_str());

        auto chunk_id = std::stoul(chunk_path.filename().c_str());

        if (chunk_id >= chunk_start) {

            auto err = unlink(chunk_path.c_str());

            if (err == -1 && errno != ENOENT) {
@@ -262,7 +261,7 @@ void ChunkStorage::trim_chunk_space(const string& file_path, gkfs::rpc::chnk_id_ 
void ChunkStorage::truncate_chunk_file(const string& file_path, gkfs::rpc::chnk_id_t chunk_id, off_t length) {

    auto chunk_path = absolute(get_chunk_path(file_path, chunk_id));

    assert(length > 0 && static_cast<gkfs::rpc::chnk_id_t>(length) <= chunksize_);

    int ret = truncate64(chunk_path.c_str(), length);

    int ret = truncate(chunk_path.c_str(), length);

    if (ret == -1) {

        auto err_str = fmt::format("Failed to truncate chunk file. File: '{}', Error: '{}'", chunk_path,

                                   ::strerror(errno));

src/daemon/ops/data.cpp

+12 −9
Original line number Diff line number Diff line
@@ -46,9 +46,9 @@ void ChunkOperation::cancel_all_tasks() { 
    task_eventuals_.clear();

}



ChunkOperation::ChunkOperation(string path) : ChunkOperation(move(path), 1) {}

ChunkOperation::ChunkOperation(const string& path) : ChunkOperation(path, 1) {}



ChunkOperation::ChunkOperation(string path, size_t n) : path_(::move(path)) {

ChunkOperation::ChunkOperation(string path, size_t n) : path_(std::move(path)) {

    // Knowing n beforehand is important and cannot be dynamic. Otherwise eventuals cause seg faults

    abt_tasks_.resize(n);

    task_eventuals_.resize(n);
@@ -67,6 +67,7 @@ ChunkOperation::~ChunkOperation() { 
 * @return error<int> is put into eventual to signal that it finished

 */

void ChunkTruncateOperation::truncate_abt(void* _arg) {

    assert(_arg);

    // Unpack args

    auto* arg = static_cast<struct chunk_truncate_args*>(_arg);

    const string& path = *(arg->path);
@@ -93,9 +94,9 @@ void ChunkTruncateOperation::truncate_abt(void* _arg) { 
    ABT_eventual_set(arg->eventual, &err_response, sizeof(int));

}



ChunkTruncateOperation::ChunkTruncateOperation(string path) : ChunkTruncateOperation{move(path), 1} {}

ChunkTruncateOperation::ChunkTruncateOperation(const string& path) : ChunkTruncateOperation{path, 1} {}



ChunkTruncateOperation::ChunkTruncateOperation(string path, size_t n) : ChunkOperation{move(path), n} {

ChunkTruncateOperation::ChunkTruncateOperation(const string& path, size_t n) : ChunkOperation{path, n} {

    task_args_.resize(n);

}
@@ -171,6 +172,7 @@ int ChunkTruncateOperation::wait_for_tasks() { 
 * @return written_size<ssize_t> is put into eventual to signal that it finished

 */

void ChunkWriteOperation::write_file_abt(void* _arg) {

    assert(_arg);

    // Unpack args

    auto* arg = static_cast<struct chunk_write_args*>(_arg);

    const string& path = *(arg->path);
@@ -179,7 +181,7 @@ void ChunkWriteOperation::write_file_abt(void* _arg) { 
        wrote = GKFS_DATA->storage()->write_chunk(path, arg->chnk_id, arg->buf, arg->size, arg->off);

    } catch (const ChunkStorageException& err) {

        GKFS_DATA->spdlogger()->error("{}() {}", __func__, err.what());

        wrote = -(err.code().value());;

        wrote = -(err.code().value());

    } catch (const ::exception& err) {

        GKFS_DATA->spdlogger()->error("{}() Unexpected error writing chunk {} of file {}", __func__, arg->chnk_id,

                                      path);
@@ -188,7 +190,7 @@ void ChunkWriteOperation::write_file_abt(void* _arg) { 
    ABT_eventual_set(arg->eventual, &wrote, sizeof(ssize_t));

}



ChunkWriteOperation::ChunkWriteOperation(string path, size_t n) : ChunkOperation{move(path), n} {

ChunkWriteOperation::ChunkWriteOperation(const string& path, size_t n) : ChunkOperation{path, n} {

    task_args_.resize(n);

}
@@ -224,7 +226,7 @@ ChunkWriteOperation::write_async(size_t idx, const uint64_t chunk_id, const char 


    auto& task_arg = task_args_[idx];

    task_arg.path = &path_;

    task_arg.buf = bulk_buf_ptr; // write_file_abt will treat buf as const and will not modify it

    task_arg.buf = bulk_buf_ptr;

    task_arg.chnk_id = chunk_id;

    task_arg.size = size;

    task_arg.off = offset;
@@ -290,6 +292,7 @@ pair<int, size_t> ChunkWriteOperation::wait_for_tasks() { 
 * @return read_size<ssize_t> is put into eventual to signal that it finished

 */

void ChunkReadOperation::read_file_abt(void* _arg) {

    assert(_arg);

    //unpack args

    auto* arg = static_cast<struct chunk_read_args*>(_arg);

    const string& path = *(arg->path);
@@ -299,7 +302,7 @@ void ChunkReadOperation::read_file_abt(void* _arg) { 
        read = GKFS_DATA->storage()->read_chunk(path, arg->chnk_id, arg->buf, arg->size, arg->off);

    } catch (const ChunkStorageException& err) {

        GKFS_DATA->spdlogger()->error("{}() {}", __func__, err.what());

        read = -(err.code().value());;

        read = -(err.code().value());

    } catch (const ::exception& err) {

        GKFS_DATA->spdlogger()->error("{}() Unexpected error reading chunk {} of file {}", __func__, arg->chnk_id,

                                      path);
@@ -308,7 +311,7 @@ void ChunkReadOperation::read_file_abt(void* _arg) { 
    ABT_eventual_set(arg->eventual, &read, sizeof(ssize_t));

}



ChunkReadOperation::ChunkReadOperation(string path, size_t n) : ChunkOperation{move(path), n} {

ChunkReadOperation::ChunkReadOperation(const string& path, size_t n) : ChunkOperation{path, n} {

    task_args_.resize(n);

}