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

Introduce log writer thread #1146

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion production/db/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ endif()
add_library(gaia_db_persistence STATIC
src/log_file.cpp
src/async_disk_writer.cpp
src/async_write_batch.cpp)
src/async_write_batch.cpp
src/log_io.cpp)
configure_gaia_target(gaia_db_persistence)
target_include_directories(gaia_db_persistence PRIVATE
"${GAIA_DB_CORE_PUBLIC_INCLUDES}"
Expand Down
20 changes: 10 additions & 10 deletions production/db/core/inc/async_disk_writer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ namespace persistence
class async_disk_writer_t
{
public:
async_disk_writer_t(int validate_flushed_batch_efd, int signal_checkpoint_efd);
async_disk_writer_t(int validate_flushed_batch_eventfd, int signal_checkpoint_eventfd);

~async_disk_writer_t();

Expand Down Expand Up @@ -98,43 +98,43 @@ class async_disk_writer_t
/**
* Copy any temporary writes (which don't exist in gaia shared memory) into the metadata buffer.
*/
unsigned char* copy_into_metadata_buffer(void* source, size_t size, int file_fd);
unsigned char* copy_into_metadata_buffer(const void* source, size_t size, int file_fd);

/**
* Perform maintenance actions on in_flight batch after all of its IO entries have been processed.
*/
void perform_post_completion_maintenance();

void add_decisions_to_batch(decision_list_t& decisions);
void add_decisions_to_batch(const decision_list_t& decisions);

/**
* For each commit ts, keep track of the eventfd which the session thread blocks on. Once the txn
* has been made durable, this eventfd is written to so that the session thread can make progress and
* return commit decision to the client.
*/
void map_commit_ts_to_session_decision_efd(gaia_txn_id_t commit_ts, int session_decision_efd);
void map_commit_ts_to_session_decision_eventfd(gaia_txn_id_t commit_ts, int session_decision_eventfd);

private:
// Reserve slots in the in_progress batch to be able to append additional operations to it (before it gets submitted to the kernel)
static constexpr size_t c_submit_batch_sqe_count = 3;
static constexpr size_t c_single_submission_entry_count = 1;
static constexpr size_t c_async_batch_size = 32;
static constexpr size_t c_max_iovec_array_size_bytes = IOV_MAX * sizeof(iovec);
static inline eventfd_t c_default_flush_efd_value = 1;
static inline iovec c_default_iov = {static_cast<void*>(&c_default_flush_efd_value), sizeof(eventfd_t)};
static inline eventfd_t c_default_flush_eventfd_value = 1;
static inline iovec c_default_iov = {static_cast<void*>(&c_default_flush_eventfd_value), sizeof(eventfd_t)};

// eventfd to signal that a batch flush has completed.
// Used to block new writes to disk when a batch is already getting flushed.
static inline int s_flush_efd = -1;
static inline int s_flush_eventfd = -1;

// eventfd to signal that the IO results belonging to a batch are ready to be validated.
int m_validate_flush_efd = -1;
int m_validate_flush_eventfd = -1;

// eventfd to signal that a file is ready to be checkpointed.
int m_signal_checkpoint_efd = -1;
int m_signal_checkpoint_eventfd = -1;

// Keep track of session threads to unblock.
std::unordered_map<gaia_txn_id_t, int> m_ts_to_session_decision_fd_map;
std::unordered_map<gaia_txn_id_t, int> m_ts_to_session_decision_eventfd_map;

// Writes are batched and we maintain two buffers so that writes to a buffer
// can still proceed when the other buffer is getting flushed to disk.
Expand Down
8 changes: 8 additions & 0 deletions production/db/core/inc/db_helpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,11 @@ inline void allocate_object(
gaia_offset_t object_offset = chunk_manager->allocate(size + c_db_object_header_size);
if (object_offset == c_invalid_gaia_offset)
{
if (gaia::db::get_mapped_log()->data()->chunk_count == c_max_chunks_per_txn)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not clear to me that there's any reason for the chunk_count to be persisted in the txn log itself, rather than in client-side session thread TLS or shared session state (which we need anyway for crash recovery). Do we need the chunk count in the txn log as anything but a consistency check when we extract the set of used chunks from redo offsets during a scan on the server?

In general I'd prefer to avoid storing redundant information like this in persistent structures, unless there's a compelling reason to do so for performance or simplicity.

Copy link
Author

Choose a reason for hiding this comment

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

Youre correct. i can infer the set of chunks from the txn log and move this to client tls.

{
throw memory_allocation_error_internal("Maximum number of chunks for this transaction has been reached.");
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is an appropriate exception type since 1) memory_allocation_error is a user-facing exception type, but "chunk" is an internal implementation detail, and 2) we haven't really run out of memory at all (i.e. other allocations, including concurrent allocations, could still succeed). I think instead we should introduce a new user-facing exception type transaction_memory_limit_exceeded, analogous to transaction_object_limit_exceeded:

/**
 * \brief The transaction exceeded its memory limit.
 *
 * A transaction can use at most 2^32 GB of memory.
 */
class transaction_memory_limit_exceeded : public common::gaia_exception
{
};

class transaction_memory_limit_exceeded_internal : public transaction_memory_limit_exceeded
{
public:
    transaction_memory_limit_exceeded_internal();
};

transaction_memory_limit_exceeded_internal::transaction_memory_limit_exceeded_internal()
{
    m_message = "Transaction exceeded its memory limit.";
}

Copy link
Author

Choose a reason for hiding this comment

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

done

}

if (chunk_manager->initialized())
{
// The current chunk is out of memory, so retire it and allocate a new chunk.
Expand Down Expand Up @@ -159,6 +164,9 @@ inline void allocate_object(
// on the server in case we crash.
gaia::db::get_mapped_log()->data()->current_chunk = new_chunk_offset;

auto& chunk = gaia::db::get_mapped_log()->data()->chunks[gaia::db::get_mapped_log()->data()->chunk_count++];
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand this initialization that is immediately overwritten. You aren't using the initial value of chunk anywhere, so what is the point of the initialization expression (besides incrementing chunk_count as a side effect)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I think I see, you're actually initializing a reference to the new chunk offset's location and assigning to the dereferenced location. I think that's quite unclear and you should just take the address and assign to a dereferenced pointer.

Copy link
Contributor

Choose a reason for hiding this comment

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

(Besides which, I'm not sure we need to store the set of chunks at all in the log, but if we do, why are you reconstructing it again from the offsets when you scan the log?)

Copy link
Author

Choose a reason for hiding this comment

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

The set of chunks is merely stored to retain chunk order. I have deleted this logic now.

chunk = static_cast<size_t>(new_chunk_offset);

// Allocate from new chunk.
object_offset = chunk_manager->allocate(size + c_db_object_header_size);
}
Expand Down
12 changes: 10 additions & 2 deletions production/db/core/inc/db_internal_types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,13 @@ constexpr size_t c_max_locators = (1ULL << 32) - 1;
// similarly optimized by substituting locators for gaia_ids.
constexpr size_t c_hash_buckets = 1ULL << 20;

// This is arbitrary, but we need to keep txn logs to a reasonable size.
constexpr size_t c_max_log_records = 1ULL << 20;
// Track maximum number of new chunks (apart from the one that the txn is already using)
// that can be allocated per transaction.
// This sets an upper bound on txn size: 32MB < txn_size < 36MB
constexpr size_t c_max_chunks_per_txn = 8;

// 8 chunks can hold up to 8 * (2^16 - 2^8) = 522240 64B objects,
constexpr size_t c_max_log_records = 522240;

// This is an array of offsets in the data segment corresponding to object
// versions, where each array index is referred to as a "locator."
Expand All @@ -108,6 +113,8 @@ struct txn_log_t
// convenient place for shared state between the client and server.
memory_manager::chunk_offset_t current_chunk;
size_t record_count;
int session_decision_eventfd;
size_t chunk_count;

struct log_record_t
{
Expand Down Expand Up @@ -135,6 +142,7 @@ struct txn_log_t
};

log_record_t log_records[c_max_log_records];
gaia_offset_t chunks[c_max_chunks_per_txn];

friend std::ostream& operator<<(std::ostream& os, const txn_log_t& l)
{
Expand Down
36 changes: 36 additions & 0 deletions production/db/core/inc/db_server.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "gaia_internal/common/generator_iterator.hpp"

#include "db_internal_types.hpp"
#include "log_io.hpp"
#include "mapped_data.hpp"
#include "memory_manager.hpp"
#include "messages_generated.h"
Expand Down Expand Up @@ -114,6 +115,12 @@ class server_t
private:
static inline server_config_t s_server_conf{};

// TODO: Delete this once recovery/checkpointing implementation is in.
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of adding all these pieces to db_server, can you instead create a separate abstraction for handling log writing and just have the server keep track of an instance of it? That should separate the main server code from the log writing component.

Copy link
Contributor

Choose a reason for hiding this comment

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

static inline bool c_use_gaia_log_implementation = false;

// TODO: Make configurable.
static constexpr int64_t c_txn_group_timeout_us = 100;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add some comment about how you arrived at this fixed value? (E.g., SSD write latency, txn commit latency SLO, etc.)


// This is arbitrary but seems like a reasonable starting point (pending benchmarks).
static constexpr size_t c_stream_batch_size{1ULL << 10};

Expand All @@ -128,6 +135,15 @@ class server_t
static inline int s_server_shutdown_eventfd = -1;
static inline int s_listening_socket = -1;

// Signals the log writer thread to persist txn updates.
static inline int s_signal_log_write_eventfd = -1;

// Signals the log writer thread to persist txn decisions.
static inline int s_signal_decision_eventfd = -1;

// Signals the checkpointing thread to merge log file updates into the LSM store.
static inline int s_signal_checkpoint_log_eventfd = -1;

// These thread objects are owned by the client dispatch thread.
static inline std::vector<std::thread> s_session_threads{};

Expand All @@ -137,6 +153,7 @@ class server_t
static inline mapped_data_t<id_index_t> s_shared_id_index{};
static inline index::indexes_t s_global_indexes{};
static inline std::unique_ptr<persistent_store_manager> s_persistent_store{};
static inline std::unique_ptr<persistence::log_handler_t> s_log_handler{};

// These fields have transaction lifetime.
thread_local static inline gaia_txn_id_t s_txn_id = c_invalid_gaia_txn_id;
Expand All @@ -155,6 +172,11 @@ class server_t
thread_local static inline bool s_session_shutdown = false;
thread_local static inline int s_session_shutdown_eventfd = -1;

thread_local static inline int s_session_decision_eventfd = -1;

// Signal to persistence thread that a batch is ready to be validated.
Copy link
Contributor

Choose a reason for hiding this comment

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

This declaration should be moved out of the group labeled "These fields have session lifetime." since it's not thread-local and not scoped to a session.

Copy link
Author

Choose a reason for hiding this comment

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

fixd

static inline int s_validate_persistence_batch_eventfd = -1;

// These thread objects are owned by the session thread that created them.
thread_local static inline std::vector<std::thread> s_session_owned_threads{};

Expand Down Expand Up @@ -242,6 +264,12 @@ class server_t
// The current thread's index in `s_safe_ts_per_thread_entries`.
thread_local static inline size_t s_safe_ts_index{c_invalid_safe_ts_index};

// Keep track of the last txn that has been submitted to the async_disk_writer.
static inline std::atomic<gaia_txn_id_t> s_last_queued_commit_ts_upper_bound = c_invalid_gaia_txn_id;

// Keep a track of undecided txns submitted to the async_disk_writer.
static inline std::set<gaia_txn_id_t> s_seen_and_undecided_txn_set{};
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be ordered? If not I'd prefer std::unordered_set. Also, I generally want to avoid allocation-heavy structures in the server, even though this isn't in the txn commit critical path. Why don't we discuss the requirements for this data structure further? (I think it may be worth adding a lock-free set that uses just std::array with linear search to our common library.)

Copy link
Author

Choose a reason for hiding this comment

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

This structure tracks txn commit timestamps and it enables txn decisions to be logged in commit order - so for example if txn 1 -> txn 2 -> txn 3; then these 3 decisions will be logged in order and recovery will proceed in this order.
Note that the txn data itself can be logged out of commit order.
Regarding the lock free array, how will memory reclamation work? Is it similar to how it happens in the txn array? Will this also be a shared memory array? I like the idea but I will keep out of current PR since I need to understand more details.


private:
// Returns the current value of the given watermark.
static inline gaia_txn_id_t get_watermark(watermark_type_t watermark_type)
Expand Down Expand Up @@ -409,6 +437,14 @@ class server_t

static void client_dispatch_handler(const std::string& socket_name);

static void log_writer_handler();

static void write_to_persistent_log(int64_t txn_group_timeout_us, bool sync_writes = false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is txn_group_timeout_us a signed type?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, is it necessary to make this configurable rather than just using the constant c_txn_group_timeout_us?

Copy link
Author

Choose a reason for hiding this comment

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

Updated everywhere to constant.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the same parameter name as async_disk_writer_t::submit_and_swap_in_progress_batch(): should_wait_for_completion.

Copy link
Author

Choose a reason for hiding this comment

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

Updated

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about the name write_to_persistent_log() either. Write what to a persistent log? The name suggests an object of the "write" action, but there is no method target or function parameter that could serve as such. Maybe something like persist_pending_writes() or write_pending_updates_to_persistent_log() would be clearer?

Copy link
Author

Choose a reason for hiding this comment

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

Updated to persist_pending_writes()


static void recover_persistent_log();

static void flush_all_pending_writes();
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be just flush_pending_writes().

Copy link
Author

Choose a reason for hiding this comment

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

Updated


static void session_handler(int session_socket);

static std::pair<int, int> get_stream_socket_pair();
Expand Down
11 changes: 8 additions & 3 deletions production/db/core/inc/log_file.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ class log_file_t
/**
* Obtain offset to write the next log record at.
*/
size_t get_current_offset();
file_offset_t get_current_offset();

/**
* Get remaining space in persistent log file.
*/
size_t get_remaining_bytes_count(size_t record_size);
size_t get_bytes_remaining_after_append(size_t record_size);

/**
* Allocate space in persistent log file.
Expand All @@ -48,10 +48,15 @@ class log_file_t

int get_file_fd();

/**
* Obtain sequence number for the file.
*/
file_sequence_t get_file_sequence();

private:
size_t m_file_size;
file_sequence_t m_file_seq;
size_t m_current_offset;
file_offset_t m_current_offset;
std::string m_dir_name;
int m_dir_fd;
int m_file_fd;
Expand Down
109 changes: 109 additions & 0 deletions production/db/core/inc/log_io.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/////////////////////////////////////////////
// Copyright (c) Gaia Platform LLC
// All rights reserved.
/////////////////////////////////////////////

#pragma once

#include <fcntl.h>
#include <unistd.h>

#include <cstddef>

#include <atomic>
#include <functional>
#include <memory>
#include <string>
#include <unordered_map>

#include "gaia/common.hpp"

#include "gaia_internal/db/db_object.hpp"

#include "async_disk_writer.hpp"
#include "db_internal_types.hpp"
#include "log_file.hpp"
#include "memory_manager.hpp"

namespace gaia
{
namespace db
{
namespace persistence
{

/*
* Fill the record_header.crc field with CRC_INITIAL_VALUE when
* computing the checksum: crc32c is vulnerable to 0-prefixing,
* so we make sure the initial bytes are non-zero.
*/
static constexpr crc32_t c_crc_initial_value = ((uint32_t)-1);

class log_handler_t
{
public:
explicit log_handler_t(const std::string& directory_path);
~log_handler_t();
void open_for_writes(int validate_flushed_batch_eventfd, int signal_checkpoint_eventfd);

/**
* Allocate space in the log file and return starting offset of allocation.
*/
file_offset_t allocate_log_space(size_t payload_size);

/**
* Create a log record which stores txn information.
*/
void create_txn_record(
gaia_txn_id_t commit_ts,
record_type_t type,
std::vector<gaia_offset_t>& object_offsets,
std::vector<gaia::common::gaia_id_t>& deleted_ids);

/**
* Process the in memory txn_log and submit the processed writes (generated log records) to the async_disk_writer.
*/
void process_txn_log_and_write(int txn_log_fd, gaia_txn_id_t commit_ts);

/**
* Create a log record which stores decisions for one or more txns.
*/
void create_decision_record(const decision_list_t& txn_decisions);

/**
* Submit async_disk_writer's internal I/O request queue to the kernel for processing.
*/
void submit_writes(bool sync);

/**
* Validate the result of I/O calls submitted to the kernel for processing.
*/
void validate_flushed_batch();

/**
* Track the session_decision_eventfd for each commit_ts; txn_commit() will only return once
* session_decision_eventfd is written to by the log_writer thread - signifying that the txn decision
* has been persisted.
*/
void register_commit_ts_for_session_notification(gaia_txn_id_t commit_ts, int session_decision_eventfd);

private:
// TODO: Make log file size configurable.
static constexpr uint64_t c_file_size = 4 * 1024 * 1024;
static constexpr std::string_view c_gaia_wal_dir_name = "/wal_dir";
static constexpr int c_gaia_wal_dir_permissions = 0755;
static inline std::string s_wal_dir_path{};
static inline int s_dir_fd = -1;

// Log file sequence starts from 1.
static inline std::atomic<file_sequence_t::value_type> s_file_num = 1;

// Keep track of the current log file.
std::unique_ptr<log_file_t> m_current_file;

std::unique_ptr<async_disk_writer_t> m_async_disk_writer;
};

} // namespace persistence
} // namespace db
} // namespace gaia
Loading