From 3c3b18e279efde5af961786c200f675df8898f05 Mon Sep 17 00:00:00 2001 From: Liang Jeff Chen Date: Mon, 17 Aug 2026 00:24:30 -0700 Subject: [PATCH 1/7] feat(ckpt): publish checkpoint ts per partition, bounded by durability A data sync round published its checkpoint timestamps only after the whole round finished, and released the flush memory quota in one step at the same point. Under a large flush the shard therefore held quota it no longer needed and blocked admission of new writes, which showed up as multi-second stalls in the write path. Checkpoint ts is now published per partition, but only where that is actually durable. DeferCkptTsUpdate(need_persist_kv, enable_mvcc) decides: - EloqStore, non-MVCC: publish per partition, once BatchWriteRecords for that partition is durable. - RocksDB / RocksDB-Cloud: defer to the end of the round, because durability is established by PersistKV(), not by the batch write. - MVCC on any backend: defer, because a version is only durable once both the base and the archive writes have landed. The deferred path collects the updates it skipped and applies them after PersistKV()/PutArchivesAll() succeed, so both paths end in the same state. Two correctness fixes fall out of publishing early: - A KV partition can receive records from several DataSyncTasks whose node-group terms differ. FindNewestTerms()/IsNewestTerm() discard entries from a stale term before grouping, so a lagging task cannot publish a ts for records that were never written; the stale task then fails CheckLeaderTerm() and its dirty entries are re-flushed by the new term. - FetchRecordCc resumed its requesters inline while still owning them. Requesters are now swapped to a local list and removed from the shard's fetch map before being resumed, so a requester that recursively fetches the same key cannot observe a half-torn request. Flush quota is released progressively, weighted by bytes actually written, through a single SyncPutAllData::OnPartitionCompleted() that both releases the quota and wakes the round's waiter on the last partition -- previously two callbacks with overlapping responsibility. UpdateCceCkptTsCc's fan-in now keeps its unfinished-core count and its waiter flag in one 8-byte atomic. Publishing the flag and testing the count in a single RMW is what makes "suspend only if work remains" exact; with two variables a waiter could arm itself and then not suspend, leaving a resume queued for a coroutine that had already run. Requests parked on the shard's memory wait list move from std::list to an intrusive CcRequestList threaded through CcRequestBase, so parking a request allocates nothing -- that list grows precisely when the shard heap is exhausted. Tests: CheckpointFlush-Test (23 cases) covers the publication contract per backend, term grouping, the fan-in protocol including both coroutine completion races, and progressive quota release; FetchRecordCc-Test (6) covers inline resume and requeue; RealDataStore-Test drives a production datastore to confirm only newest-term data is persisted. The in-memory harness gains flush-failure injection. Co-Authored-By: Claude Opus 5 (1M context) --- docs/07-durability-and-recovery.md | 14 +- docs/09-store-handler.md | 14 +- store_handler/data_store_service_client.cpp | 167 +- store_handler/data_store_service_client.h | 55 +- .../data_store_service_client_closure.cpp | 83 +- .../data_store_service_client_closure.h | 106 +- .../rocksdb_data_store_common.cpp | 2 +- store_handler/rocksdb_handler.cpp | 6 +- store_handler/rocksdb_handler.h | 4 +- tx_service/include/cc/cc_req_base.h | 126 ++ tx_service/include/cc/cc_req_misc.h | 222 +- tx_service/include/cc/cc_request.h | 2 +- tx_service/include/cc/cc_shard.h | 46 +- tx_service/include/data_sync_task.h | 65 +- tx_service/include/store/data_store_handler.h | 4 +- tx_service/src/cc/cc_req_misc.cpp | 113 +- tx_service/src/cc/cc_shard.cpp | 83 +- tx_service/src/cc/local_cc_shards.cpp | 220 +- tx_service/src/data_sync_task.cpp | 96 +- tx_service/tests/CMakeLists.txt | 2 + tx_service/tests/CheckpointFlush-Test.cpp | 1911 +++++++++++++++++ tx_service/tests/FetchRecordCc-Test.cpp | 562 +++++ tx_service/tests/RealDataStore-Test.cpp | 519 +++++ tx_service/tests/harness/mem_data_store.cpp | 10 +- tx_service/tests/harness/mem_data_store.h | 8 +- .../tests/harness/mem_data_store_factory.h | 11 +- 26 files changed, 4157 insertions(+), 294 deletions(-) create mode 100644 tx_service/tests/CheckpointFlush-Test.cpp create mode 100644 tx_service/tests/RealDataStore-Test.cpp diff --git a/docs/07-durability-and-recovery.md b/docs/07-durability-and-recovery.md index 29b7e766d..4b8f1d922 100644 --- a/docs/07-durability-and-recovery.md +++ b/docs/07-durability-and-recovery.md @@ -53,10 +53,16 @@ published as durable until that barrier succeeds. A failed, skipped, or non-truncatable task prevents the affected checkpoint round from advancing the log-truncation boundary. -Once every included task is durable, the checkpointer updates entry state, -publishes the NG checkpoint timestamp, and reports the safe truncation point to -the log service. Clean entries at or below their checkpoint timestamp may then -be evicted and fetched from storage on demand. +Entry checkpoint timestamps advance at the complete backend durability +boundary. Non-MVCC EloqStore flushes publish each durable partition; RocksDB +backends wait for `PersistKV`, and MVCC also waits for archive writes. A merged +flush retains only the newest represented term for each NG, so discarded +lower-term writes cannot publish checkpoint state. Split-child flushes update +entries on their source range's owning shard. + +Clean entries can be evicted after their checkpoint state is published. The NG +checkpoint timestamp and log-truncation boundary advance only after every +included task succeeds, independently of earlier entry-level progress. The same data-sync machinery also supports range splits, index construction, bucket migration, snapshots, and shutdown flushing. Those callers can require diff --git a/docs/09-store-handler.md b/docs/09-store-handler.md index 8f0730245..59cb373a3 100644 --- a/docs/09-store-handler.md +++ b/docs/09-store-handler.md @@ -85,10 +85,16 @@ them through `PutAll`. Some backends stage these batches without making them independently durable. When `NeedPersistKV` is true, `PersistKV` is the required barrier after all batches for the checkpoint have completed. -The checkpointer may advance entry and NG checkpoint timestamps, make clean -entries evictable, and report a WAL truncation point only after both the writes -and any required persistence barrier succeed. This is the central contract -between `DataStoreHandler` and the durability layer. +EloqStore batch completion establishes durability, so non-MVCC flushes may +publish entry checkpoint timestamps per completed partition. RocksDB-backed +batches require `PersistKV`; MVCC also requires archive writes before entries +become clean and evictable. Lower-term tasks in a merged flush are excluded +from both datastore work and checkpoint publication. NG checkpoint timestamps +and WAL truncation still wait for the entire included round to succeed. + +This distinction between entry-level progress and the NG-wide truncation +boundary is the central contract between `DataStoreHandler` and the durability +layer. Cache misses reverse the path: the handler returns a record or streams a bounded range slice, and the completion is installed on the owning shard. MVCC reads diff --git a/store_handler/data_store_service_client.cpp b/store_handler/data_store_service_client.cpp index d9618b461..b32d79485 100644 --- a/store_handler/data_store_service_client.cpp +++ b/store_handler/data_store_service_client.cpp @@ -317,9 +317,14 @@ bool DataStoreServiceClient::PutAll( &flush_task, const std::function *yield_fptr, const std::function *resume_fptr, - const std::function *sync_yield_fptr) + const std::function *sync_yield_fptr, + const std::function *partition_progress_fptr) { - return PutAllImpl(flush_task, yield_fptr, resume_fptr, sync_yield_fptr); + return PutAllImpl(flush_task, + yield_fptr, + resume_fptr, + sync_yield_fptr, + partition_progress_fptr); } bool DataStoreServiceClient::PutAllImpl( @@ -328,7 +333,8 @@ bool DataStoreServiceClient::PutAllImpl( &flush_task, const std::function *yield_fptr, const std::function *resume_fptr, - const std::function *sync_yield_fptr) + const std::function *sync_yield_fptr, + const std::function *partition_progress_fptr) { DLOG(INFO) << "DataStoreServiceClient::PutAll called with " << flush_task.size() << " tables to flush."; @@ -340,9 +346,35 @@ bool DataStoreServiceClient::PutAllImpl( size_t records_count = 0; std::vector callback_data_list; + const txservice::NewestTermByNodeGroup newest_terms = + txservice::FindNewestTerms(flush_task); + + // Whether a partition may publish its cc entries' ckpt ts the moment its + // own writes complete. Only sound when the store guarantees durability on + // BatchWriteRecords return (EloqStore). Stores that defer durability to + // PersistKV (RocksDB-backed) must not mark entries clean that early: a + // later PersistKV failure would leave un-persisted records already marked + // clean and therefore never re-flushed. For those, no entries are + // collected and no request is armed here; FlushDataImpl publishes the + // ckpt ts after PersistKV succeeds. + // The caller only installs a progress callback when the rest of this + // flush becomes durable together with each base partition. MVCC archives + // are written after PutAll, so even EloqStore must defer publication in + // that mode until PutArchivesAll succeeds. + const bool ckpt_ts_on_partition_complete = + !NeedPersistKV() && partition_progress_fptr != nullptr; + // Process each table for (auto &[kv_table_name, entries] : flush_task) { + // FlushDataTask normally creates a table bucket together with its + // first entry, but PutAll is a public handler API and callers may pass + // an empty bucket. It carries no work and, importantly, has no task + // from which a TableName could be derived. + if (entries.empty()) + { + continue; + } auto &table_name = entries.front()->data_sync_task_->table_name_; // Group records by partition @@ -350,16 +382,25 @@ bool DataStoreServiceClient::PutAllImpl( hash_partitions_map; std::unordered_map> range_partitions_map; + // A merged flush buffer can straddle a leader-term transition. Discard + // older tasks before grouping any records, even when their records + // occupy a kv partition with no newer-term record in this buffer: the + // new term's checkpoint owns the current in-memory contents. Terms + // from different node groups are unrelated. size_t flush_task_entry_idx = 0; for (auto &entry : entries) { + const size_t entry_idx = flush_task_entry_idx++; + if (!txservice::IsNewestTerm(*entry, newest_terms)) + { + continue; + } auto &batch = *entry->data_sync_vec_; if (batch.empty()) { continue; } records_count += batch.size(); - if (table_name.IsHashPartitioned()) { for (size_t i = 0; i < batch.size(); ++i) @@ -373,8 +414,7 @@ bool DataStoreServiceClient::PutAllImpl( it->second.reserve(batch.size() / 1024 * 2 * entries.size()); } - it->second.emplace_back( - std::make_pair(flush_task_entry_idx, i)); + it->second.emplace_back(entry_idx, i); } } else @@ -383,11 +423,9 @@ bool DataStoreServiceClient::PutAllImpl( // table int32_t partition_id = KvPartitionIdOf(batch[0].partition_id_, true); - auto [it, inserted] = - range_partitions_map.try_emplace(partition_id); - it->second.emplace_back(flush_task_entry_idx); + auto [it, _] = range_partitions_map.try_emplace(partition_id); + it->second.emplace_back(entry_idx); } - flush_task_entry_idx++; } uint16_t parts_cnt_per_key = 1; @@ -408,6 +446,8 @@ bool DataStoreServiceClient::PutAllImpl( // Prepare batches for this partition PreparePartitionBatches(*partition_state, + sync_putall, + ckpt_ts_on_partition_complete, flush_recs, entries, table_name, @@ -431,6 +471,8 @@ bool DataStoreServiceClient::PutAllImpl( // Prepare batches for this partition PrepareRangePartitionBatches(*partition_state, + sync_putall, + ckpt_ts_on_partition_complete, flush_recs, entries, table_name, @@ -447,13 +489,20 @@ bool DataStoreServiceClient::PutAllImpl( // Set up global coordinator sync_putall->total_partitions_ = sync_putall->partition_states_.size(); + sync_putall->total_bytes_ = 0; + for (const auto *ps : sync_putall->partition_states_) + { + sync_putall->total_bytes_ += ps->serialized_bytes_; + } - // Set coroutine callbacks BEFORE starting async work (see plan risk - // analysis) + // Install coroutine callbacks before starting async writes: a local + // backend may complete synchronously and otherwise miss the wake-up + // handshake entirely. if (yield_fptr != nullptr && resume_fptr != nullptr) { sync_putall->SetCoroCallbacks(yield_fptr, resume_fptr); } + sync_putall->SetProgressCallback(partition_progress_fptr); // Start concurrent processing for each partition constexpr size_t MAX_BATCH_WRITES_WITHOUT_YIELD = 10; @@ -494,7 +543,8 @@ bool DataStoreServiceClient::PutAllImpl( else { // No batches for this partition, mark as completed - sync_putall->OnPartitionCompleted(); + sync_putall->OnPartitionCompleted( + partition_state->serialized_bytes_); } } // Wait for all partitions to complete @@ -3034,10 +3084,19 @@ bool DataStoreServiceClient::PutArchivesAllImpl( uint32_t, std::vector>> partitions_map; + const txservice::NewestTermByNodeGroup newest_terms = + txservice::FindNewestTerms(flush_task); for (auto &[kv_table_name, flush_task_entry] : flush_task) { + // Apply the same batch-wide term fence as PutAll. Otherwise an + // obsolete task rejected for the base table could still append its + // in-memory versions to the archive table. for (auto &entry : flush_task_entry) { + if (!txservice::IsNewestTerm(*entry, newest_terms)) + { + continue; + } auto &archive_vec = *entry->archive_vec_; if (archive_vec.empty()) @@ -3276,18 +3335,28 @@ bool DataStoreServiceClient::CopyBaseToArchiveImpl( std::vector>> archive_flush_task; constexpr uint32_t MAX_FLYING_READ_COUNT = 100; + const txservice::NewestTermByNodeGroup newest_terms = + txservice::FindNewestTerms(flush_task); for (auto &[base_kv_table_name, flush_task_entry] : flush_task) { + if (flush_task_entry.empty()) + { + continue; + } auto &table_name = flush_task_entry.front()->data_sync_task_->table_name_; - auto &table_schema = flush_task_entry.front()->table_schema_; bool is_range_partitioned = !table_name.IsHashPartitioned(); - + // CopyBaseToArchive precedes PutAll, so it must fence old terms here; + // waiting for PutAll's grouping filter would already be too late. auto *catalog_factory = GetCatalogFactory(table_name.Engine()); assert(catalog_factory != nullptr); for (auto &entry : flush_task_entry) { + if (!txservice::IsNewestTerm(*entry, newest_terms)) + { + continue; + } auto &base_vec = *entry->mv_base_vec_; if (base_vec.empty()) { @@ -3450,8 +3519,8 @@ bool DataStoreServiceClient::CopyBaseToArchiveImpl( std::move(archive_vec), nullptr, nullptr, - flush_task_entry.front()->data_sync_task_, - table_schema, + entry->data_sync_task_, + entry->table_schema_, batch_size)); } } @@ -5838,6 +5907,8 @@ bool DataStoreServiceClient::DeleteCatalog( void DataStoreServiceClient::PreparePartitionBatches( EloqDS::PartitionFlushState &partition_state, + EloqDS::SyncPutAllData *sync_putall, + bool publish_ckpt_ts_on_complete, const std::vector> &flush_recs, const std::vector> &entries, const txservice::TableName &table_name, @@ -5941,8 +6012,23 @@ void DataStoreServiceClient::PreparePartitionBatches( // Process records and create batches for (auto idx : flush_recs) { + const auto &flush_entry = entries.at(idx.first); txservice::FlushRecord &ckpt_rec = - entries.at(idx.first)->data_sync_vec_->at(idx.second); + flush_entry->data_sync_vec_->at(idx.second); + + // Remember which cc entry this record came from, so the partition can + // advance its ckpt ts as soon as it is durable. A hash-partitioned + // task's id is the owning cc shard. + if (publish_ckpt_ts_on_complete && ckpt_rec.cce_ != nullptr && + flush_entry->data_sync_task_->need_update_ckpt_ts_) + { + partition_state.AddCkptTsEntry( + flush_entry->data_sync_task_.get(), + static_cast(flush_entry->data_sync_task_->id_), + ckpt_rec.cce_, + ckpt_rec.commit_ts_, + ckpt_rec.post_flush_size_); + } // Start a new batch if size limit reached // or the record_tmp_mem_area is full. Since the record_parts is a @@ -5953,6 +6039,7 @@ void DataStoreServiceClient::PreparePartitionBatches( batch_request.record_tmp_mem_area.size() == batch_request.record_tmp_mem_area.capacity()) { + partition_state.serialized_bytes_ += write_batch_size; partition_state.AddBatch(std::move(batch_request)); batch_request.Reset( @@ -5976,12 +6063,17 @@ void DataStoreServiceClient::PreparePartitionBatches( // Add the last batch if it has data if (batch_request.key_parts.size() > 0) { + partition_state.serialized_bytes_ += write_batch_size; partition_state.AddBatch(std::move(batch_request)); } + + partition_state.ArmCkptTsUpdate(sync_putall); } void DataStoreServiceClient::PrepareRangePartitionBatches( EloqDS::PartitionFlushState &partition_state, + EloqDS::SyncPutAllData *sync_putall, + bool publish_ckpt_ts_on_complete, const std::vector &flush_recs, const std::vector> &entries, const txservice::TableName &table_name, @@ -5992,8 +6084,12 @@ void DataStoreServiceClient::PrepareRangePartitionBatches( size_t write_batch_size = 0; PartitionBatchRequest batch_request; - bool enabled_mvcc = - txservice::Sharder::Instance().GetLocalCcShards()->EnableMvcc(); + auto *local_shards = txservice::Sharder::Instance().GetLocalCcShards(); + // The public handler API is also used by datastore-only tests and tools + // that have no LocalCcShards. Those callers have no MVCC execution layer, + // so the range encoding follows the non-MVCC path. + const bool enabled_mvcc = + local_shards != nullptr && local_shards->EnableMvcc(); auto PrepareRecordData = [&](txservice::FlushRecord &ckpt_rec, size_t &batch_size, @@ -6049,8 +6145,33 @@ void DataStoreServiceClient::PrepareRangePartitionBatches( // Process records and create batches for (auto idx : flush_recs) { - for (auto &ckpt_rec : *entries.at(idx)->data_sync_vec_) + const auto &flush_entry = entries.at(idx); + const bool collect_ckpt_ts = + publish_ckpt_ts_on_complete && + flush_entry->data_sync_task_->need_update_ckpt_ts_; + size_t core_idx = 0; + if (collect_ckpt_ts) + { + assert(local_shards != nullptr); + // A range task's id is the range id; its owning cc shard is + // derived from the low bits, matching how the scan sharded it. + core_idx = static_cast( + (flush_entry->data_sync_task_->id_ & 0x3FF) % + local_shards->Count()); + } + + for (auto &ckpt_rec : *flush_entry->data_sync_vec_) { + if (collect_ckpt_ts && ckpt_rec.cce_ != nullptr) + { + partition_state.AddCkptTsEntry( + flush_entry->data_sync_task_.get(), + core_idx, + ckpt_rec.cce_, + ckpt_rec.commit_ts_, + ckpt_rec.post_flush_size_); + } + // Start a new batch if size limit reached // or the record_tmp_mem_area is full. Since the record_parts is a // vector of string_view that references the record_tmp_mem_area, we @@ -6060,6 +6181,7 @@ void DataStoreServiceClient::PrepareRangePartitionBatches( batch_request.record_tmp_mem_area.size() == batch_request.record_tmp_mem_area.capacity()) { + partition_state.serialized_bytes_ += write_batch_size; partition_state.AddBatch(std::move(batch_request)); batch_request.Reset( @@ -6079,8 +6201,11 @@ void DataStoreServiceClient::PrepareRangePartitionBatches( // Add the last batch if it has data if (batch_request.key_parts.size() > 0) { + partition_state.serialized_bytes_ += write_batch_size; partition_state.AddBatch(std::move(batch_request)); } + + partition_state.ArmCkptTsUpdate(sync_putall); } } // namespace EloqDS diff --git a/store_handler/data_store_service_client.h b/store_handler/data_store_service_client.h index 62440049d..73c9966c6 100644 --- a/store_handler/data_store_service_client.h +++ b/store_handler/data_store_service_client.h @@ -49,6 +49,7 @@ struct PartitionFlushState; struct PartitionBatchRequest; struct PartitionCallbackData; struct SyncConcurrentRequest; +struct SyncPutAllData; class DataStoreServiceClient; class BatchWriteRecordsClosure; class ReadClosure; @@ -229,18 +230,28 @@ class DataStoreServiceClient : public txservice::store::DataStoreHandler * @param node_group * @return whether all entries are written to data store successfully */ - bool PutAll( - std::unordered_map< - std::string_view, - std::vector>> - &flush_task, - const std::function *yield_fptr = nullptr, - const std::function *resume_fptr = nullptr, - const std::function *sync_yield_fptr = nullptr) override; + bool PutAll(std::unordered_map< + std::string_view, + std::vector>> + &flush_task, + const std::function *yield_fptr = nullptr, + const std::function *resume_fptr = nullptr, + const std::function *sync_yield_fptr = nullptr, + const std::function + *partition_progress_fptr = nullptr) override; bool NeedPersistKV() override { +#ifdef DATA_STORE_TYPE_ELOQDSS_ELOQSTORE + // EloqStore guarantees durability by the time BatchWriteRecords + // returns; its FlushData handler is a no-op, so there is nothing to + // persist after PutAll. + return false; +#else + // The RocksDB-backed stores accept writes into memory (WAL skipped) + // and only make them durable in PersistKV. return true; +#endif } uint64_t ApproxStoreKeyCount() override; @@ -631,7 +642,9 @@ class DataStoreServiceClient : public txservice::store::DataStoreHandler &flush_task, const std::function *yield_fptr = nullptr, const std::function *resume_fptr = nullptr, - const std::function *sync_yield_fptr = nullptr); + const std::function *sync_yield_fptr = nullptr, + const std::function + *partition_progress_fptr = nullptr); bool CopyBaseToArchiveImpl( std::unordered_map< @@ -681,10 +694,25 @@ class DataStoreServiceClient : public txservice::store::DataStoreHandler void BatchWriteRecordsInternal(BatchWriteRecordsClosure *closure); /** - * Helper methods for concurrent PutAll implementation + * @brief Fully sets up one hash partition of a concurrent PutAll: builds + * its write batches, accumulates its serialized byte weight and ckpt-ts + * entries, and arms its ckpt-ts update request when there is anything to + * publish. + * + * @param partition_state The partition to set up. + * @param sync_putall The PutAll coordinator the armed request's completion + * hook reports to. + * @param publish_ckpt_ts_on_complete Whether the partition may publish its + * cc entries' ckpt ts as soon as its own writes complete. False for stores + * whose writes only become durable in PersistKV; no entries are collected + * and no request is armed, and FlushDataImpl publishes after PersistKV. + * @param flush_recs (entry index, record index) pairs selecting this + * partition's records within @p entries. */ void PreparePartitionBatches( PartitionFlushState &partition_state, + SyncPutAllData *sync_putall, + bool publish_ckpt_ts_on_complete, const std::vector> &flush_recs, const std::vector> &entries, const txservice::TableName &table_name, @@ -692,8 +720,15 @@ class DataStoreServiceClient : public txservice::store::DataStoreHandler uint16_t parts_cnt_per_record, uint64_t now); + /** + * @brief Range-partition counterpart of PreparePartitionBatches; @p + * flush_recs selects whole entries, since a range table's flush entry + * belongs to a single partition. + */ void PrepareRangePartitionBatches( PartitionFlushState &partition_state, + SyncPutAllData *sync_putall, + bool publish_ckpt_ts_on_complete, const std::vector &flush_recs, const std::vector> &entries, const txservice::TableName &table_name, diff --git a/store_handler/data_store_service_client_closure.cpp b/store_handler/data_store_service_client_closure.cpp index 738f1798d..66fdb0794 100644 --- a/store_handler/data_store_service_client_closure.cpp +++ b/store_handler/data_store_service_client_closure.cpp @@ -605,7 +605,8 @@ void PartitionBatchCallback(void *data, { partition_state->MarkFailed(result); // Notify the global coordinator that this partition failed - global_coordinator->OnPartitionCompleted(); + global_coordinator->OnPartitionCompleted( + partition_state->serialized_bytes_); return; } @@ -633,8 +634,43 @@ void PartitionBatchCallback(void *data, } else { - // Notify the global coordinator that this partition completed - global_coordinator->OnPartitionCompleted(); + // Every batch of this partition is durable. Publish the ckpt ts of the + // cc entries it carried before reporting completion, so those entries + // become evictable now instead of when the slowest sibling partition in + // the same flush task lands. The request was constructed and its + // completion hook armed when the partition was set up; this only hands + // it to the shards, chained rather than waited on because this runs on + // a storage completion thread, which must not block on cc shards. The + // hook reports the partition complete once every shard has applied. + if (partition_state->ckpt_ts_update_.has_value()) + { + auto *local_shards = + txservice::Sharder::Instance().GetLocalCcShards(); + + auto *update = &partition_state->ckpt_ts_update_.value(); + auto it = partition_state->ckpt_ts_entries_.begin(); + const auto end = partition_state->ckpt_ts_entries_.end(); + while (it != end) + { + const uint16_t core_id = static_cast(it->first); + // Advance before publishing: the final shard may complete the + // request immediately and wake PutAll, which can recycle + // partition_state while EnqueueToCcShard returns. + const bool is_last = ++it == end; + local_shards->EnqueueToCcShard(core_id, update); + if (is_last) + { + break; + } + } + } + else + { + // No ckpt-ts entries were collected, so no request was armed. + // Report completion directly. + global_coordinator->OnPartitionCompleted( + partition_state->serialized_bytes_); + } } } @@ -1813,4 +1849,45 @@ bool PartitionFlushState::GetNextBatch(PartitionBatchRequest &batch) pending_batches.pop(); return true; } + +void PartitionFlushState::AddCkptTsEntry(const txservice::DataSyncTask *task, + size_t core_idx, + txservice::LruEntry *cce, + uint64_t commit_ts, + size_t post_flush_size) +{ + assert(task != nullptr); + if (ckpt_ts_task_ == nullptr) + { + ckpt_ts_task_ = task; + } + else + { + // PutAllImpl removes lower-term records before batch preparation, so a + // partition must never mix publication metadata from different terms. + assert(ckpt_ts_task_->node_group_id_ == task->node_group_id_); + assert(ckpt_ts_task_->node_group_term_ == task->node_group_term_); + assert(ckpt_ts_task_->table_name_ == task->table_name_); + } + ckpt_ts_entries_[core_idx].emplace_back(cce, commit_ts, post_flush_size); +} + +void PartitionFlushState::ArmCkptTsUpdate(SyncPutAllData *sync_putall) +{ + assert(sync_putall != nullptr); + if (ckpt_ts_entries_.empty()) + { + assert(ckpt_ts_task_ == nullptr); + return; + } + assert(ckpt_ts_task_ != nullptr); + ckpt_ts_update_.emplace(ckpt_ts_task_->node_group_id_, + ckpt_ts_task_->node_group_term_, + ckpt_ts_task_->table_name_, + ckpt_ts_entries_); + const uint64_t done_bytes = serialized_bytes_; + ckpt_ts_update_->SetOnFinished( + [sync_putall, done_bytes] + { sync_putall->OnPartitionCompleted(done_bytes); }); +} } // namespace EloqDS diff --git a/store_handler/data_store_service_client_closure.h b/store_handler/data_store_service_client_closure.h index 642c4af84..4a77e9acb 100644 --- a/store_handler/data_store_service_client_closure.h +++ b/store_handler/data_store_service_client_closure.h @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -41,7 +42,8 @@ namespace EloqDS { class DataStoreServiceClient; -} +struct SyncPutAllData; +} // namespace EloqDS namespace EloqDS { @@ -175,7 +177,7 @@ struct SyncCallbackData : public Poolable }; /** - * @brief Per-partition state management for concurrent flushing + * @brief Per-partition state management for concurrent flushing. */ struct PartitionFlushState : public Poolable { @@ -186,6 +188,24 @@ struct PartitionFlushState : public Poolable remote::CommonResult result; mutable bthread::Mutex mux; + // Serialized bytes this partition carries across all of its batches, + // accumulated while the batches are prepared. Used as the weight of this + // partition when the flush releases its memory quota progressively: byte + // weights track the real (uneven) split of the flush task far better than + // partition counts. + uint64_t serialized_bytes_{0}; + + // PutAll discards lower-term tasks per node group before partition + // grouping. All entries retained in one kv partition therefore share one + // node-group term and can be published by one request after durability. + const txservice::DataSyncTask *ckpt_ts_task_{nullptr}; + absl::flat_hash_map> + ckpt_ts_entries_; + // Declared after ckpt_ts_entries_ so it is destroyed first; the request + // retains a reference to the entries for its entire lifetime. + std::optional ckpt_ts_update_; + PartitionFlushState() : partition_id(0) { result.Clear(); @@ -201,6 +221,10 @@ struct PartitionFlushState : public Poolable } failed = false; result.Clear(); + serialized_bytes_ = 0; + ckpt_ts_update_.reset(); + ckpt_ts_entries_.clear(); + ckpt_ts_task_ = nullptr; } void Clear() override @@ -210,7 +234,26 @@ struct PartitionFlushState : public Poolable { pending_batches.pop(); } - } + serialized_bytes_ = 0; + ckpt_ts_update_.reset(); + ckpt_ts_entries_.clear(); + ckpt_ts_task_ = nullptr; + } + + /** + * @brief Records a cc entry made durable by this partition. + * + * @param task The newest-term data-sync task selected for the partition. + * @param core_idx The cc shard that owns the entry. + */ + void AddCkptTsEntry(const txservice::DataSyncTask *task, + size_t core_idx, + txservice::LruEntry *cce, + uint64_t commit_ts, + size_t post_flush_size); + + /** Arms the partition's single ckpt-ts update after setup is complete. */ + void ArmCkptTsUpdate(SyncPutAllData *sync_putall); bool IsFailed() const { std::unique_lock lk(mux); @@ -264,18 +307,24 @@ struct SyncPutAllData : public Poolable partition_states_.clear(); completed_partitions_ = 0; total_partitions_ = 0; + completed_bytes_ = 0; + total_bytes_ = 0; waiting_.store(false); yield_fn_ = nullptr; resume_fn_ = nullptr; + progress_fn_ = nullptr; } virtual void Clear() override { completed_partitions_ = 0; total_partitions_ = 0; + completed_bytes_ = 0; + total_bytes_ = 0; waiting_.store(false); yield_fn_ = nullptr; resume_fn_ = nullptr; + progress_fn_ = nullptr; for (auto *partition_state : partition_states_) { partition_state->Clear(); @@ -291,15 +340,57 @@ struct SyncPutAllData : public Poolable resume_fn_ = resume_fn; } + /** + * @brief Installs the flush-progress consumer, invoked directly from + * OnPartitionCompleted() on whichever thread completes a partition, with + * the cumulative serialized bytes of the finished partitions and the + * flush's total. Byte weights, not partition counts: partitions are + * unevenly sized, and the caller releases flush memory quota in + * proportion. + * + * The callback must be safe to run from any completion context (a + * storage callback thread, the flush coroutine, or a cc shard via the + * ckpt-ts continuation): it may take only short, leaf-level locks. Calls + * are serialized by mux_. + */ + void SetProgressCallback( + const std::function *progress_fn) + { + progress_fn_ = progress_fn; + } + void Wait(); void Wait(const std::function *yield_fn, const std::function *resume_fn); - void OnPartitionCompleted() + /** + * @brief The single completion call for one partition (success or + * failure): folds its byte weight into the flush's progress, hands the + * corresponding memory quota back through the progress callback, and + * wakes the PutAll waiter when the last partition lands. + * + * The quota is released here, on the completing thread, rather than by + * waking the waiter to do it: mux_ serializes concurrent completions + * (which also protects the release watermark inside the callback), and + * the release itself only takes the data-sync memory controller's + * short-lived lock, whose sleepers are pthreads -- safe from any + * completion context. The waiter therefore sleeps exactly once. The + * final progress call precedes the final wake, so the callback's state, + * which lives on the waiter's frame, is still alive whenever the + * callback runs. + * + * @param done_bytes The serialized bytes the partition carried. + */ + void OnPartitionCompleted(uint64_t done_bytes) { std::unique_lock lk(mux_); completed_partitions_++; + completed_bytes_ += done_bytes; + if (progress_fn_ != nullptr) + { + (*progress_fn_)(completed_bytes_, total_bytes_); + } if (completed_partitions_ >= total_partitions_) { if (resume_fn_ && waiting_.load(std::memory_order_acquire)) @@ -326,10 +417,17 @@ struct SyncPutAllData : public Poolable std::vector partition_states_; int32_t completed_partitions_{0}; int32_t total_partitions_{0}; + // Serialized-byte progress mirror of the two counters above, passed to + // progress_fn_ as the weights for proportional quota release. Guarded by + // mux_ like the partition counters. total_bytes_ is written once before + // the sends start and is immutable while anyone waits. + uint64_t completed_bytes_{0}; + uint64_t total_bytes_{0}; // Coroutine yield/resume support const std::function *yield_fn_{nullptr}; const std::function *resume_fn_{nullptr}; + const std::function *progress_fn_{nullptr}; std::atomic waiting_{false}; }; diff --git a/store_handler/eloq_data_store_service/rocksdb_data_store_common.cpp b/store_handler/eloq_data_store_service/rocksdb_data_store_common.cpp index 8d01030d1..ba5fa0cd2 100644 --- a/store_handler/eloq_data_store_service/rocksdb_data_store_common.cpp +++ b/store_handler/eloq_data_store_service/rocksdb_data_store_common.cpp @@ -742,6 +742,7 @@ void RocksDBDataStoreCommon::BatchWriteRecords( auto write_status = db->Write(write_options, &write_batch); + result.set_error_code(::EloqDS::remote::DataStoreError::NO_ERROR); if (!write_status.ok()) { LOG(ERROR) << "BatchWriteRecords failed, table:" @@ -769,7 +770,6 @@ void RocksDBDataStoreCommon::BatchWriteRecords( } } - result.set_error_code(::EloqDS::remote::DataStoreError::NO_ERROR); batch_write_req->SetFinish(result); }); diff --git a/store_handler/rocksdb_handler.cpp b/store_handler/rocksdb_handler.cpp index bac3f1867..f07045b13 100644 --- a/store_handler/rocksdb_handler.cpp +++ b/store_handler/rocksdb_handler.cpp @@ -516,9 +516,13 @@ bool RocksDBHandler::PutAll( &batch, const std::function *yield_fptr, const std::function *resume_fptr, - const std::function *sync_yield_fptr) + const std::function *sync_yield_fptr, + const std::function *partition_progress_fptr) { (void) sync_yield_fptr; + // This backend writes the batch as a unit, so there is no partition + // progress to report. + (void) partition_progress_fptr; std::thread::id this_id = std::this_thread::get_id(); if (batch.empty()) { diff --git a/store_handler/rocksdb_handler.h b/store_handler/rocksdb_handler.h index 72c9a50c2..e697e88d4 100644 --- a/store_handler/rocksdb_handler.h +++ b/store_handler/rocksdb_handler.h @@ -278,7 +278,9 @@ class RocksDBHandler : public txservice::store::DataStoreHandler std::vector>> &batch, const std::function *yield_fptr = nullptr, const std::function *resume_fptr = nullptr, - const std::function *sync_yield_fptr = nullptr) override; + const std::function *sync_yield_fptr = nullptr, + const std::function *partition_progress_fptr = + nullptr) override; /** * @brief indicate end of flush entries in a single ckpt for \@param diff --git a/tx_service/include/cc/cc_req_base.h b/tx_service/include/cc/cc_req_base.h index c766584c1..bfdab118c 100644 --- a/tx_service/include/cc/cc_req_base.h +++ b/tx_service/include/cc/cc_req_base.h @@ -24,6 +24,8 @@ #pragma once #include +#include +#include #include "cc_protocol.h" #include "error_messages.h" @@ -104,5 +106,129 @@ struct CcRequestBase TxNumber tx_number_{0}; CcProtocol proto_{CcProtocol::OCC}; IsolationLevel isolation_level_{IsolationLevel::ReadCommitted}; + +private: + // Intrusive links, managed exclusively by CcRequestList. They let a shard + // park a request -- e.g. on the memory wait list -- without allocating a + // list node, which matters because that list grows precisely when the + // shard heap is exhausted. The shard scheduler ensures that a request is + // either executing, queued, or parked on one wait list; it cannot be + // inserted into another wait list until the first one re-enqueues it. + CcRequestBase *list_prev_{nullptr}; + CcRequestBase *list_next_{nullptr}; + + friend class CcRequestList; +}; + +/** + * @brief Intrusive doubly-linked list of cc requests, threaded through + * CcRequestBase's own link fields. + * + * Parking and removal are O(1) and allocation-free. PushBack()/Remove() assert + * list-local membership, including the one-element case where both intrusive + * links are null. Not thread-safe: a list must only be mutated from its owning + * shard. + * + * A request must be removed from the list before it is aborted or freed: + * Free() returns it to a pool where another producer may reuse and re-link + * it while the stale links still point into this list. + */ +class CcRequestList +{ +public: + bool Empty() const + { + return size_ == 0; + } + + size_t Size() const + { + return size_; + } + + CcRequestBase *Front() const + { + return head_; + } + + bool Contains(const CcRequestBase *req) const + { + if (size_ == 0) + { + return false; + } + if (size_ == 1) + { + return head_ == req; + } + return req->list_prev_ != nullptr || req->list_next_ != nullptr; + } + + static CcRequestBase *NextOf(const CcRequestBase *req) + { + return req->list_next_; + } + + void PushBack(CcRequestBase *req) + { + assert(!Contains(req)); + assert(req->list_prev_ == nullptr && req->list_next_ == nullptr); + + req->list_prev_ = tail_; + if (tail_ != nullptr) + { + tail_->list_next_ = req; + } + else + { + head_ = req; + } + tail_ = req; + ++size_; + } + + CcRequestBase *PopFront() + { + CcRequestBase *req = head_; + if (req != nullptr) + { + Remove(req); + } + return req; + } + + void Remove(CcRequestBase *req) + { + assert(Contains(req)); + if (req->list_prev_ != nullptr) + { + req->list_prev_->list_next_ = req->list_next_; + } + else + { + assert(head_ == req); + head_ = req->list_next_; + } + + if (req->list_next_ != nullptr) + { + req->list_next_->list_prev_ = req->list_prev_; + } + else + { + assert(tail_ == req); + tail_ = req->list_prev_; + } + + req->list_prev_ = nullptr; + req->list_next_ = nullptr; + assert(size_ > 0); + --size_; + } + +private: + CcRequestBase *head_{nullptr}; + CcRequestBase *tail_{nullptr}; + size_t size_{0}; }; } // namespace txservice diff --git a/tx_service/include/cc/cc_req_misc.h b/tx_service/include/cc/cc_req_misc.h index 570ad5c35..052b36679 100644 --- a/tx_service/include/cc/cc_req_misc.h +++ b/tx_service/include/cc/cc_req_misc.h @@ -1171,16 +1171,16 @@ struct UpdateCceCkptTsCc : public CcRequestBase NodeGroupId node_group_id, int64_t term, const TableName &table_name, - absl::flat_hash_map> &cce_entries, - bool update_ckpt_ts) + absl::flat_hash_map> &cce_entries) : cce_entries_(cce_entries), node_group_id_(node_group_id), term_(term), - table_name_(table_name), - update_ckpt_ts_(update_ckpt_ts) + table_name_(table_name) { - unfinished_core_cnt_ = cce_entries_.size(); - assert(unfinished_core_cnt_ > 0); + assert(cce_entries_.size() > 0 && cce_entries_.size() <= UINT32_MAX); + state_.store( + CompletionState{static_cast(cce_entries_.size()), 0}, + std::memory_order_relaxed); for (const auto &entry : cce_entries_) { @@ -1196,56 +1196,162 @@ struct UpdateCceCkptTsCc : public CcRequestBase void SetCoroCallbacks(const std::function *yield_fn, const std::function *resume_fn) { + // The coroutine mode (SetCoroCallbacks + Wait) and the continuation + // mode (SetOnFinished) are mutually exclusive; see SetFinished(). + assert(on_finished_ == nullptr); yield_fn_ = yield_fn; resume_fn_ = resume_fn; } + /** + * @brief Continues execution on the last core to apply its slice, instead + * of waking a thread parked in Wait(). + * + * For callers that cannot block: the flush path installs a continuation + * that reports its partition complete, which keeps "PutAll has returned" + * meaning "every ckpt ts is published" even though nothing waits for it. + * + * @p on_finished runs on a cc shard and may destroy this request's owner, + * so it must be the last thing that touches the request. Consumed on + * invocation. + */ + void SetOnFinished(std::function on_finished) + { + // Mutually exclusive with the coroutine mode; see SetFinished(). + assert(yield_fn_ == nullptr && resume_fn_ == nullptr); + on_finished_ = std::move(on_finished); + } + + /** + * @brief Marks the calling core's slice applied. The core that brings the + * count to zero performs the completion action: the on-finished + * continuation, the coroutine resume, or the condition-variable notify. + * + * The three actions are mutually exclusive completion modes, not stages: + * exactly one fires, selected by what the creator installed. The + * continuation mode (SetOnFinished; the per-partition flush) returns + * without touching the wake machinery -- its consumer reports the + * partition complete and wakes the PutAll waiter itself. The resume and + * notify modes serve a caller blocked in this request's own Wait() (the + * deferred publication path). A request never has more than one waiter. + * + * Mutex-free except in condition-variable mode. In coroutine mode, count + * and waiter flag share one atomic word, so the decrement that reaches zero + * atomically collects whether a waiter has committed to suspending. The + * resume callback is captured before that decrement: once zero is visible, + * the terminal shard either uses only that local pointer or returns without + * touching the request, allowing a waiter that never suspended to destroy + * it safely. acquire/release suffices because all fan-in operations are + * RMWs on one release sequence. + * + * In condition-variable mode the decrement itself is protected by mux_. A + * waiter therefore cannot observe zero, return, and destroy the request + * before the terminal shard has finished notifying through cv_. + */ void SetFinished() { - std::unique_lock lk(mux_); - unfinished_core_cnt_--; - if (unfinished_core_cnt_ == 0) + // Both modes are immutable once requests are published to the shards. + // Capture them before a coroutine-mode terminal decrement publishes + // zero, after which an unarmed waiter may return and destroy `this`. + const bool continuation_mode = static_cast(on_finished_); + const std::function *resume_fn = resume_fn_; + + if (continuation_mode || resume_fn != nullptr) { - if (resume_fn_ != nullptr && - waiting_.load(std::memory_order_acquire)) + CompletionState prev = state_.load(std::memory_order_relaxed); + CompletionState next; + do { - waiting_.store(false, std::memory_order_release); - auto *fn = resume_fn_; - lk.unlock(); - (*fn)(); + assert(prev.unfinished_core_cnt_ >= 1); + next = prev; + --next.unfinished_core_cnt_; + } while (!state_.compare_exchange_weak(prev, + next, + std::memory_order_acq_rel, + std::memory_order_relaxed)); + if (prev.unfinished_core_cnt_ != 1) + { + return; } - else if (resume_fn_ == nullptr) + + if (continuation_mode) { - cv_.notify_one(); + // May destroy this request's owner; nothing touches members + // afterwards. + auto fn = std::move(on_finished_); + on_finished_ = nullptr; + fn(); + } + else if (prev.waiter_suspended_ != 0) + { + // The waiter committed to yielding before publishing the flag. + // resume_fn may queue that wake before yield_fn runs. + (*resume_fn)(); } } - } - - void Wait() - { - std::unique_lock lk(mux_); - while (unfinished_core_cnt_ > 0) + else { - cv_.wait_for(lk, 10000L); // timeout_us, preserve original value + // Publish zero while holding the same mutex used by Wait(). Wait + // cannot return and destroy the request until notification is done + // and this critical section has released the mutex. + std::lock_guard lk(mux_); + CompletionState prev = state_.load(std::memory_order_relaxed); + assert(prev.waiter_suspended_ == 0); + assert(prev.unfinished_core_cnt_ >= 1); + --prev.unfinished_core_cnt_; + state_.store(prev, std::memory_order_release); + if (prev.unfinished_core_cnt_ == 0) + { + cv_.notify_one(); + } } } - void Wait(const std::function *yield_fn, - const std::function *resume_fn) + /** + * @brief Blocks the caller until every core has applied its slice. + * + * With coroutine callbacks installed (SetCoroCallbacks), suspends through + * them and never touches the mutex; otherwise waits on the condition + * variable under mux_. + */ + void Wait() { - if (yield_fn == nullptr || resume_fn == nullptr) + assert((yield_fn_ == nullptr) == (resume_fn_ == nullptr)); + // The continuation mode (SetOnFinished) completes asynchronously and + // never waits. + assert(on_finished_ == nullptr); + if (yield_fn_ != nullptr) { - Wait(); - return; + CompletionState cur = state_.load(std::memory_order_acquire); + while (cur.unfinished_core_cnt_ != 0) + { + assert(cur.waiter_suspended_ == 0); + // Publish the waiter and the count > 0 condition it depends + // on in one RMW: the flag can only be set while cores remain, + // so the terminal decrement either sees it (and resumes the + // suspension entered unconditionally below) or the CAS fails + // and the reloaded count exits the loop without suspending. + CompletionState suspended = cur; + suspended.waiter_suspended_ = 1; + if (state_.compare_exchange_weak(cur, + suspended, + std::memory_order_acq_rel, + std::memory_order_acquire)) + { + (*yield_fn_)(); + cur = state_.load(std::memory_order_acquire); + } + } } - std::unique_lock lk(mux_); - while (unfinished_core_cnt_ > 0) + else { - waiting_.store(true, std::memory_order_release); - lk.unlock(); - (*yield_fn)(); - lk.lock(); - waiting_.store(false, std::memory_order_release); + std::unique_lock lk(mux_); + while (state_.load(std::memory_order_acquire).unfinished_core_cnt_ > + 0) + { + // timeout_us, preserve original value + cv_.wait_for(lk, 10000L); + } } } @@ -1257,8 +1363,7 @@ struct UpdateCceCkptTsCc : public CcRequestBase bool IsFinished() const { - std::lock_guard lk(mux_); - return unfinished_core_cnt_ == 0; + return state_.load(std::memory_order_acquire).unfinished_core_cnt_ == 0; } private: @@ -1266,18 +1371,51 @@ struct UpdateCceCkptTsCc : public CcRequestBase // key: core_idx, value: entry_index absl::flat_hash_map indices_; - size_t unfinished_core_cnt_; + /** + * @brief The fan-in count and the waiter's suspension flag, bundled in + * one 8-byte lock-free atomic so their consistency is maintained by + * single RMWs; see SetFinished() and Wait(). + */ + struct CompletionState + { + // Cores that have not yet applied their slice. + uint32_t unfinished_core_cnt_{0}; + // 1 while the coroutine waiter is suspended. uint32_t rather than + // bool keeps the struct padding-free, so compare_exchange only ever + // compares meaningful bytes. + uint32_t waiter_suspended_{0}; + }; + static_assert(sizeof(CompletionState) == 8); + + std::atomic state_{CompletionState{}}; + static_assert(std::atomic::is_always_lock_free); NodeGroupId node_group_id_; int64_t term_; TableName table_name_; - bool update_ckpt_ts_; - mutable bthread::Mutex mux_; + // Guards the count transition and notification in condition-variable + // completion mode; the coroutine and continuation modes never touch it. + bthread::Mutex mux_; bthread::ConditionVariable cv_; + // What to run once every core has applied its slice, instead of waking a + // thread parked in Wait(). The flush path installs a continuation that + // reports the partition complete to its PutAll coordinator, so a partition + // is only counted as done after the cc entries it wrote are marked clean -- + // preserving the guarantee that PutAll returns with every ckpt ts already + // published, without anyone blocking to get it. It exists because both ends + // sit on threads that must not park: SetFinished() runs on a cc shard, and + // the flush path is driven from a data store completion callback. + // + // Runs on whichever shard finishes last. It may destroy this request's + // owner (the request lives inside the pooled partition state that the + // continuation can free), so SetFinished() moves it out, drops the lock, + // and touches no member afterwards. Consumed on invocation, so a pooled + // request cannot fire a stale continuation from an earlier flush. + std::function on_finished_{nullptr}; + // Coroutine yield/resume support const std::function *yield_fn_{nullptr}; const std::function *resume_fn_{nullptr}; - std::atomic waiting_{false}; }; struct WaitNoNakedBucketRefCc : public CcRequestBase diff --git a/tx_service/include/cc/cc_request.h b/tx_service/include/cc/cc_request.h index 29c642ba1..ec9cc363f 100644 --- a/tx_service/include/cc/cc_request.h +++ b/tx_service/include/cc/cc_request.h @@ -6803,7 +6803,7 @@ struct ApplyCc : public TemplatedCcRequest remote_input_.cmd_ = nullptr; remote_input_.is_owner_ = false; } - in_use_.store(false, std::memory_order_release); + CcRequestBase::Free(); } void Reset(const TableName *table_name, diff --git a/tx_service/include/cc/cc_shard.h b/tx_service/include/cc/cc_shard.h index 2afb71544..6de474ea1 100644 --- a/tx_service/include/cc/cc_shard.h +++ b/tx_service/include/cc/cc_shard.h @@ -34,7 +34,6 @@ #include #include #include -#include #include #include #include @@ -417,6 +416,24 @@ class CcShard void WakeUpShardCleanCc(); + /** + * @brief Consumes the wake-up that arrived while the eviction pass was + * already running. + * + * A wake-up cannot be delivered while ShardCleanCc is in use, and the pass + * in flight may have already scanned past the entries the wake-up is about + * -- the memory freed by a checkpoint flush that lands mid-pass is a case + * in point. Since that pass stops itself once it frees nothing, the + * wake-up would be lost and the shard would hold its parked requests until + * some later flush. Recording it lets the pass re-run instead. + * + * @return Whether a wake-up was dropped since the last call. + */ + bool TakeShardCleanCcWakeUp() + { + return std::exchange(shard_clean_cc_wake_pending_, false); + } + /** * @brief Puts a cc request into the shard's request queue to be processed. * @@ -988,7 +1005,15 @@ class CcShard store::DataStoreHandler::DataStoreOpStatus FetchBucketData( FetchBucketDataCc *fetch_bucket_data_cc); - void RemoveFetchRecordRequest(LruEntry *cce); + /** + * Removes the active fetch index and transfers ownership to the caller. + * Keep the returned owner alive while resuming requesters inline so a + * nested fetch cannot reuse the completing request. Releasing the owner + * recycles a pooled request or destroys a temporary one; Execute() must + * then return false to prevent the dispatcher from freeing it again. + * Callers abandoning a fetch may discard the returned owner immediately. + */ + FetchRecordCc::uptr RemoveFetchRecordRequest(LruEntry *cce); CcMap *CreateOrUpdatePkCcMap(const TableName &table_name, const TableSchema *table_schema, @@ -1362,7 +1387,7 @@ class CcShard // Keep a bounded set of reusable shells after a burst of cold reads. // Saturation allocates temporary requests rather than limiting reads. CcRequestPool fetch_record_cc_pool_{128}; - // Addresses stay stable across async callbacks. Erasing an active owner + // Addresses stay stable across async callbacks. Releasing an owner // recycles a pooled request or deletes a temporary one. Declare the map // after the pool so these owners are released before the pool is destroyed. absl::flat_hash_map fetch_record_reqs_; @@ -1394,8 +1419,10 @@ class CcShard std::vector low_priority_thd_token_; std::vector> lazy_free_queue_; std::atomic lazy_free_queue_size_{0}; - // Cc requests waiting for the free memory. - std::list cc_wait_list_for_memory_; + // Cc requests parked until memory is freed. Intrusive: parking allocates + // nothing, which matters because this list grows exactly when the shard + // heap is exhausted. + CcRequestList cc_wait_list_for_memory_; // all the transactions started on this ccshard. Some txs are Ongoing while // others are Available, new transaction request has to traverse the array @@ -1427,12 +1454,17 @@ class CcShard std::unique_ptr retry_fwd_msg_cc_; // Shard clean cc std::unique_ptr shard_clean_cc_; + // Set when a wake-up arrives while shard_clean_cc_ is already in use, so + // that the pass in flight re-runs rather than dropping the wake-up. See + // TakeShardCleanCcWakeUp(). + bool shard_clean_cc_wake_pending_{false}; // Standby forward msg related members used on follower node CcRequestPool key_obj_standby_msg_cc_pool_; absl::flat_hash_map standby_sequence_grps_; - // requests to execute after schema being modified - std::vector waiting_list_for_schema_; + // Cc requests parked until the table schema they saw is updated; intrusive + // like cc_wait_list_for_memory_, so parking allocates nothing. + CcRequestList waiting_list_for_schema_; // The total number of commands buffered on this shard. If standby node has // too many commands buffered, it probably has fallen behind. Resubscribe diff --git a/tx_service/include/data_sync_task.h b/tx_service/include/data_sync_task.h index a6d49f387..65ebb2c12 100644 --- a/tx_service/include/data_sync_task.h +++ b/tx_service/include/data_sync_task.h @@ -28,10 +28,13 @@ #include #include #include +#include #include #include +#include #include #include +#include #include "absl/container/flat_hash_map.h" #include "catalog_factory.h" @@ -45,6 +48,13 @@ namespace txservice { extern bool txservice_skip_wal; +/** Returns whether ckpt-ts publication must wait beyond base-partition writes. + */ +inline bool DeferCkptTsUpdate(bool need_persist_kv, bool enable_mvcc) +{ + return need_persist_kv || enable_mvcc; +} + struct DataSyncTask; struct DataSyncStatus @@ -291,6 +301,11 @@ struct DataSyncTask } const TableName table_name_; + // What this identifies depends on the table's partitioning scheme. Hash + // partition: the cc shard / core index whose scan produces this task's + // records (a round creates one task per core). Range partition: the range + // id; the owning core is derived as (id_ & 0x3FF) % core_cnt, and one + // task covers one range. int32_t id_; uint64_t range_version_; uint32_t node_group_id_; @@ -339,8 +354,6 @@ struct DataSyncTask bool during_split_range_{false}; bool export_base_table_items_{false}; uint64_t tx_number_{0}; - // Core that owns source CCEs collected while flushing a split range. - uint16_t cce_owner_core_{0}; bthread::Mutex update_cce_mux_; std::string kv_table_name_; @@ -383,6 +396,50 @@ struct FlushTaskEntry size_t size_{0}; }; +using FlushTaskEntryMap = + std::unordered_map>>; +using NewestTermByNodeGroup = std::unordered_map; + +/** + * @brief Finds the highest task term represented for each node group. + * @param flush_task A merged flush batch, potentially spanning tables and + * node-group terms. + * @return The highest term in the whole batch for every represented node + * group. + */ +NewestTermByNodeGroup FindNewestTerms(const FlushTaskEntryMap &flush_task); + +/** + * @brief Returns whether an entry belongs to its node group's newest term. + * @param entry The entry to test. + * @param newest_terms Batch-wide newest terms returned by FindNewestTerms(). + */ +bool IsNewestTerm(const FlushTaskEntry &entry, + const NewestTermByNodeGroup &newest_terms); + +/** Deferred publication metadata for one table and node group. */ +struct CkptTsUpdateGroup +{ + NodeGroupId node_group_id_; + int64_t node_group_term_; + absl::flat_hash_map> + cce_entries_; +}; + +/** + * @brief Collects deferred ckpt-ts updates for one table bucket. + * @param table_entries All entries grouped under one physical kv table. + * @param newest_terms Batch-wide newest terms returned by FindNewestTerms(). + * @param cc_shard_count Number of local cc shards, used to map range ids. + * @return One update group per represented node group. Lower-term entries are + * omitted because their datastore writes are discarded from the same batch. + */ +std::vector CollectCkptTsUpdateGroups( + const std::vector> &table_entries, + const NewestTermByNodeGroup &newest_terms, + size_t cc_shard_count); + struct FlushDataTask { public: @@ -494,9 +551,7 @@ struct FlushDataTask return nullptr; } - std::unordered_map>> - flush_task_entries_; + FlushTaskEntryMap flush_task_entries_; size_t pending_flush_size_{0}; size_t max_pending_flush_size_{0}; bthread::Mutex flush_task_entries_mux_; diff --git a/tx_service/include/store/data_store_handler.h b/tx_service/include/store/data_store_handler.h index ded04a309..9632d6ca1 100644 --- a/tx_service/include/store/data_store_handler.h +++ b/tx_service/include/store/data_store_handler.h @@ -92,7 +92,9 @@ class DataStoreHandler &flush_task, const std::function *yield_fptr = nullptr, const std::function *resume_fptr = nullptr, - const std::function *sync_yield_fptr = nullptr) = 0; + const std::function *sync_yield_fptr = nullptr, + const std::function *partition_progress_fptr = + nullptr) = 0; /** * @brief indicate end of flush entries in a single ckpt for \@param batch diff --git a/tx_service/src/cc/cc_req_misc.cpp b/tx_service/src/cc/cc_req_misc.cpp index 03bc36bf6..19a0c298e 100644 --- a/tx_service/src/cc/cc_req_misc.cpp +++ b/tx_service/src/cc/cc_req_misc.cpp @@ -940,6 +940,7 @@ bool FetchRecordCc::Execute(CcShard &ccs) #ifdef DATA_STORE_TYPE_ELOQDSS_ELOQSTORE bool should_reopen = false; #endif + bool resume_requesters_inline = false; if (lock_->GetCcEntry() != nullptr) { @@ -975,13 +976,7 @@ bool FetchRecordCc::Execute(CcShard &ccs) } if (error_code_ == 0) { - for (CcRequestBase *req : requesters_) - { - if (req) - { - ccs.Enqueue(ccs.core_id_, req); - } - } + resume_requesters_inline = true; } else { @@ -1011,6 +1006,18 @@ bool FetchRecordCc::Execute(CcShard &ccs) #ifdef DATA_STORE_TYPE_ELOQDSS_ELOQSTORE if (should_reopen) { + // This fetch remains the active single-flight request while it is + // re-armed below. Keep a queue boundary so resumed requesters cannot + // coalesce a nested fetch into this request before the new operation + // has been installed. + for (CcRequestBase *req : requesters_) + { + if (req != nullptr) + { + ccs.Enqueue(ccs.core_id_, req); + } + } + // Re-arm this request in place rather than erasing it and issuing a // new one: FetchRecord coalesces by cce, so a fresh call would be // merged into this still-registered request and never dispatched, @@ -1056,7 +1063,29 @@ bool FetchRecordCc::Execute(CcShard &ccs) } #endif - ccs.RemoveFetchRecordRequest(cce_); + std::vector ready_requesters; + if (resume_requesters_inline) + { + // Detach the list before making the key available for another fetch. + // A resumed snapshot reader may request another version of this same + // key, which must register a new FetchRecordCc rather than mutate the + // list being iterated here. + ready_requesters.swap(requesters_); + } + + // Remove the completed single-flight operation before resuming waiters, + // but retain ownership until they finish. A nested fetch must neither + // coalesce into this request nor recycle its pooled shell. The owner also + // keeps temporary overflow requests alive throughout inline completion. + auto finishing_fetch = ccs.RemoveFetchRecordRequest(cce_); + for (CcRequestBase *req : ready_requesters) + { + if (req != nullptr && req->Execute(ccs)) + { + req->Free(); + } + } + // finishing_fetch releases this request; the dispatcher must not touch it. return false; } @@ -1363,37 +1392,15 @@ bool UpdateCceCkptTsCc::Execute(CcShard &ccs) size_t last_index = std::min(index + SCAN_BATCH_SIZE, records.size()); + CcMap *ccm = ccs.GetCcm(table_name_, node_group_id_); + assert(ccm != nullptr); + bool range_partitioned = !table_name_.IsHashPartitioned(); bool versioned_payload = table_name_.Engine() != TableEngine::EloqKv; - CcMap *ccm = nullptr; - if (update_ckpt_ts_) - { - ccm = ccs.GetCcm(table_name_, node_group_id_); - assert(ccm != nullptr); - } for (; index < last_index; ++index) { const CkptTsEntry &ref = records[index]; - if (!update_ckpt_ts_) - { - assert(range_partitioned); - - // The split copy is durable under its destination range, but the - // source cce remains dirty. Only release its in-flight state. - if (versioned_payload) - { - static_cast *>(ref.cce_) - ->ClearBeingCkpt(); - } - else - { - static_cast *>(ref.cce_) - ->ClearBeingCkpt(); - } - continue; - } - if (range_partitioned) { if (versioned_payload) @@ -1451,6 +1458,13 @@ bool UpdateCceCkptTsCc::Execute(CcShard &ccs) if (index == records.size()) { + // This shard's entries are now clean, so entries the last eviction + // pass skipped as dirty are reclaimable. Restart its scan cursor and + // wake the pass if requests are parked waiting for memory -- doing it + // here, on the shard that was just updated, keeps the notification in + // step with the flush rather than deferring it to the end of the whole + // flush task. + ccs.OnDirtyDataFlushed(); SetFinished(); } else @@ -1678,9 +1692,12 @@ bool ShardCleanCc::Execute(CcShard &ccs) } else { - // Reach to the tail ccpage, but the allocated memory is - // still larger than the heap threshold, just abort the - // waiting ccrequests. + // Reached the tail ccpage with the allocated memory still + // above the heap threshold. Abort only the parked requests + // that opted in via AbortIfOom() -- txs holding range read + // locks, which would deadlock reclaim by blocking the very + // data sync that frees memory. Ordinary requests stay parked + // until eviction or a flush makes room. ccs.AbortRequestsAfterMemoryFree(); // Notify the checkpointer thread to do checkpoint if there @@ -1693,6 +1710,20 @@ bool ShardCleanCc::Execute(CcShard &ccs) } free_count_ = 0; + + // This pass decided there was nothing to free from state it + // gathered before any wake-up that arrived while it ran -- a + // checkpoint flush turning dirty entries evictable, say. Such + // a wake-up could not be delivered because this request was in + // use, so re-run rather than stopping with requests parked and + // nothing left to schedule another pass. + if (ccs.TakeShardCleanCcWakeUp() && + ccs.WaitListSizeForMemory() > 0) + { + ccs.Enqueue(this); + return false; + } + // Return true will set the request as free, which means the // request is not in working state. return true; @@ -1709,6 +1740,12 @@ bool ShardCleanCc::Execute(CcShard &ccs) // Reset the value if the ccrequest is finished. free_count_ = (wait_list_empty) ? 0 : free_count_; + if (wait_list_empty) + { + // Nothing is parked any more, so a wake-up that arrived during + // this pass has already been served. + ccs.TakeShardCleanCcWakeUp(); + } return wait_list_empty; } } @@ -1722,6 +1759,10 @@ bool ShardCleanCc::Execute(CcShard &ccs) { ccs.Enqueue(this); } + else + { + ccs.TakeShardCleanCcWakeUp(); + } return wait_list_empty; } } diff --git a/tx_service/src/cc/cc_shard.cpp b/tx_service/src/cc/cc_shard.cpp index 336eb79a4..9a834889f 100644 --- a/tx_service/src/cc/cc_shard.cpp +++ b/tx_service/src/cc/cc_shard.cpp @@ -567,57 +567,52 @@ void CcShard::Enqueue(uint32_t thd_id, uint32_t shard_code, CcRequestBase *req) void CcShard::EnqueueWaitListIfMemoryFull(CcRequestBase *req) { - cc_wait_list_for_memory_.emplace_back(req); + cc_wait_list_for_memory_.PushBack(req); } bool CcShard::DequeueWaitListAfterMemoryFree(bool deque_all) { - if (cc_wait_list_for_memory_.size() == 0) - { - return true; - } - + // Releases a batch, not the whole list: the caller (ShardCleanCc) + // re-enqueues itself behind the released requests when this returns + // false, so between batches those requests execute and consume the freed + // memory, and the next pass re-checks the heap before releasing more. + // Releasing everything at once would send the tail of the list into a + // FindEmplace that fails and re-parks. The batch size only sets the + // sample period of that feedback loop; 20 vs. dequeue-all measured + // identically on the tail. uint32_t dequeue_cnt = 0; - auto it = cc_wait_list_for_memory_.begin(); - for (; it != cc_wait_list_for_memory_.end() && - (deque_all || dequeue_cnt < 20);) + while (!cc_wait_list_for_memory_.Empty() && (deque_all || dequeue_cnt < 20)) { - this->Enqueue(LocalCoreId(), (*it)); - ++it; + // Unlink before re-enqueueing: once on the cc queue, the request is + // free to execute and park itself again. + CcRequestBase *req = cc_wait_list_for_memory_.PopFront(); + this->Enqueue(LocalCoreId(), req); ++dequeue_cnt; } - bool is_empty = it == cc_wait_list_for_memory_.end(); - cc_wait_list_for_memory_.erase(cc_wait_list_for_memory_.begin(), it); - - return is_empty; + return cc_wait_list_for_memory_.Empty(); } void CcShard::AbortRequestsAfterMemoryFree() { - if (cc_wait_list_for_memory_.size() == 0) - { - return; - } - - for (auto req_it = cc_wait_list_for_memory_.begin(); - req_it != cc_wait_list_for_memory_.end();) + for (CcRequestBase *req = cc_wait_list_for_memory_.Front(); req != nullptr;) { - if ((*req_it)->AbortIfOom()) + CcRequestBase *next = CcRequestList::NextOf(req); + if (req->AbortIfOom()) { - (*req_it)->AbortCcRequest(CcErrorCode::OUT_OF_MEMORY); - req_it = cc_wait_list_for_memory_.erase(req_it); - } - else - { - ++req_it; + // Unlink before aborting: AbortCcRequest may Free() the request + // back to its pool, where a producer can reuse and re-link it + // while stale links would still point into this list. + cc_wait_list_for_memory_.Remove(req); + req->AbortCcRequest(CcErrorCode::OUT_OF_MEMORY); } + req = next; } } size_t CcShard::WaitListSizeForMemory() { - return cc_wait_list_for_memory_.size(); + return cc_wait_list_for_memory_.Size(); } void CcShard::WakeUpShardCleanCc() @@ -627,6 +622,13 @@ void CcShard::WakeUpShardCleanCc() shard_clean_cc_->Use(); Enqueue(shard_clean_cc_.get()); } + else + { + // The pass in flight may already have scanned past whatever this + // wake-up is about. Remember it so the pass re-runs instead of + // stopping with requests still parked. + shard_clean_cc_wake_pending_ = true; + } } void CcShard::Enqueue(CcRequestBase *req) @@ -2426,14 +2428,13 @@ store::DataStoreHandler::DataStoreOpStatus CcShard::FetchBucketData( return err_code; } -void CcShard::RemoveFetchRecordRequest(LruEntry *cce) +FetchRecordCc::uptr CcShard::RemoveFetchRecordRequest(LruEntry *cce) { auto fetch_it = fetch_record_reqs_.find(cce); assert(fetch_it != fetch_record_reqs_.end()); - // Both fetch completion and acquisition run on this shard. A pooled - // request cannot be reused until Execute unwinds; a temporary request is - // destroyed here, so the caller must not access it after this erasure. + auto request = std::move(fetch_it->second); fetch_record_reqs_.erase(fetch_it); + return request; } CcMap *CcShard::CreateOrUpdatePkCcMap(const TableName &table_name, @@ -3944,19 +3945,17 @@ void CcShard::SubsribeToPrimaryNode(uint32_t seq_grp, uint64_t seq_id) void CcShard::EnqueueWaitListIfSchemaMismatch(CcRequestBase *req) { - waiting_list_for_schema_.push_back(req); + waiting_list_for_schema_.PushBack(req); } void CcShard::DequeueWaitListAfterSchemaUpdated() { - if (waiting_list_for_schema_.size() > 0) + while (!waiting_list_for_schema_.Empty()) { - for (auto req : waiting_list_for_schema_) - { - this->Enqueue(req); - } - - waiting_list_for_schema_.clear(); + // Unlink before re-enqueueing: once on the cc queue, the request is + // free to execute and park itself again. + CcRequestBase *req = waiting_list_for_schema_.PopFront(); + this->Enqueue(req); } } diff --git a/tx_service/src/cc/local_cc_shards.cpp b/tx_service/src/cc/local_cc_shards.cpp index 6477ac631..45d9ecd15 100644 --- a/tx_service/src/cc/local_cc_shards.cpp +++ b/tx_service/src/cc/local_cc_shards.cpp @@ -6116,10 +6116,73 @@ void LocalCcShards::FlushDataImpl(FlushDataTask *cur_work, } } + // Hand this task's flush memory quota back as its partitions land rather + // than all at once when the task ends. A task holds its whole share for the + // duration of PutAll, so with several tasks in flight the quota is what + // stalls DataSyncScan in AllocateFlushMemQuota. + // + // The release is proportional by serialized bytes -- quota * done_bytes / + // total_bytes. The store handler reports each finished partition's + // serialized weight, which tracks the real (uneven) split of the flush + // task; the quota itself was charged in in-memory bytes as one lump, so a + // proportion is still needed to convert between the two units. The + // progress is a cumulative watermark rather than a fixed amount per + // report: one wake-up may coalesce several partition completions, so + // `done_bytes` can jump arbitrarily between reports. Each report releases + // the difference between the new watermark and what has already been + // released; a report whose (truncated) watermark has not advanced is a + // no-op, which also makes duplicate reports harmless. The remainder -- + // integer truncation, or the whole share when PutAll fails or is skipped + // -- is released by the unconditional DeallocateFlushMemQuota below, so + // the amounts always sum to exactly what was taken. + // The state is bundled behind a single captured reference so the lambda + // fits std::function's small-buffer optimization (16 bytes on libstdc++) + // and constructing partition_progress_func does not heap-allocate. + struct FlushQuotaProgress + { + DataSyncMemoryController &mem_controller_; + const uint64_t task_flush_quota_; + uint64_t released_; + } quota_progress{ + data_sync_mem_controller_, cur_work->pending_flush_size_, 0}; + const std::function partition_progress_func = + ["a_progress](uint64_t done_bytes, uint64_t total_bytes) + { + if (total_bytes == 0 || done_bytes == 0) + { + return; + } + // 128-bit intermediate: quota and byte totals are both full-width + // uint64 counters, so the product can exceed 64 bits. + const uint64_t target = static_cast( + static_cast(quota_progress.task_flush_quota_) * + done_bytes / total_bytes); + if (target > quota_progress.released_) + { + quota_progress.mem_controller_.DeallocateFlushMemQuota( + target - quota_progress.released_); + quota_progress.released_ = target; + } + }; + + // Progressive quota release pairs with per-partition ckpt-ts publication: + // it only makes sense when partial-batch progress is actionable. For + // stores that defer durability to PersistKV, or MVCC flushes whose archive + // writes follow PutAll, nothing downstream can act on base-partition + // progress. Such a flush holds its quota to the end (released by the + // unconditional deallocation below) and wakes the PutAll waiter once. + const bool deferred_ckpt_ts_update = + DeferCkptTsUpdate(store_hd_->NeedPersistKV(), EnableMvcc()); + const std::function *partition_progress_fptr = + deferred_ckpt_ts_update ? nullptr : &partition_progress_func; + if (succ) { - succ = store_hd_->PutAll( - flush_task_entries, &yield_fn, &resume_fn, &sync_yield_func); + succ = store_hd_->PutAll(flush_task_entries, + &yield_fn, + &resume_fn, + &sync_yield_func, + partition_progress_fptr); if (!succ) { checkpoint_failure_reason = @@ -6176,6 +6239,43 @@ void LocalCcShards::FlushDataImpl(FlushDataTask *cur_work, } } + if (succ && deferred_ckpt_ts_update) + { + // RocksDB-backed stores arrive here only after PersistKV makes their + // WAL-disabled writes durable. MVCC EloqStore arrives here after both + // PutAll and PutArchivesAll: publishing after base writes alone could + // make an entry clean and evictable even if its archive write failed, + // causing that history never to be retried. Use the datastore's same + // batch-wide term fence below: an older-term write was discarded and + // its cc entry must therefore remain dirty. + const NewestTermByNodeGroup newest_terms = + FindNewestTerms(flush_task_entries); + for (auto &[_, entries] : flush_task_entries) + { + assert(!entries.empty()); + const TableName &table_name = + entries.front()->data_sync_task_->table_name_; + std::vector update_groups = + CollectCkptTsUpdateGroups(entries, newest_terms, Count()); + for (CkptTsUpdateGroup &group : update_groups) + { + assert(!group.cce_entries_.empty()); + UpdateCceCkptTsCc update_cce_req(group.node_group_id_, + group.node_group_term_, + table_name, + group.cce_entries_); + update_cce_req.SetCoroCallbacks(&yield_fn, &resume_fn); + for (const auto &[core_idx, cce_entries] : group.cce_entries_) + { + (void) cce_entries; + EnqueueToCcShard(static_cast(core_idx), + &update_cce_req); + } + update_cce_req.Wait(); + } + } + } + // Record that data was written in DataSyncStatus if flush succeeded. if (succ) { @@ -6201,118 +6301,26 @@ void LocalCcShards::FlushDataImpl(FlushDataTask *cur_work, } } - std::unordered_set updated_ckpt_ts_core_ids; - // Finalize in-memory state for successfully flushed CCEs. - if (succ) - { - size_t iterations_since_yield = 0; - constexpr size_t MAX_ITERATIONS_WITHOUT_YIELD = 10; - - for (auto &[kv_table_name, entries] : flush_task_entries) - { - for (auto &entry : entries) - { - absl::flat_hash_map> - cce_entries_map; - - auto &table_name = - entries.front()->data_sync_task_->table_name_; - bool update_ckpt_ts = - entry->data_sync_task_->need_update_ckpt_ts_; - - for (auto &rec : *(entry->data_sync_vec_)) - { - auto cce = rec.cce_; - if (cce != nullptr) - { - size_t key_core_idx = 0; - if (!update_ckpt_ts) - { - // Split scans collected these CCEs from the source - // range; id_ identifies the destination range. - key_core_idx = - entry->data_sync_task_->cce_owner_core_; - } - else if (!table_name.IsHashPartitioned()) - { - int32_t range_id = entry->data_sync_task_->id_; - key_core_idx = static_cast( - (range_id & 0x3FF) % Count()); - } - else - { - key_core_idx = entry->data_sync_task_->id_; - } - auto insert_it = cce_entries_map.try_emplace( - key_core_idx, - std::vector()); - insert_it.first->second.emplace_back( - cce, rec.commit_ts_, rec.post_flush_size_); - } - } - - if (cce_entries_map.size() > 0) - { - UpdateCceCkptTsCc update_cce_req( - entry->data_sync_task_->node_group_id_, - entry->data_sync_task_->node_group_term_, - table_name, - cce_entries_map, - update_ckpt_ts); - update_cce_req.SetCoroCallbacks(&yield_fn, &resume_fn); - for (auto &[core_idx, cce_entries] : cce_entries_map) - { - if (update_ckpt_ts) - { - updated_ckpt_ts_core_ids.insert(core_idx); - } - EnqueueToCcShard(core_idx, &update_cce_req); - } - - bool is_finished = update_cce_req.IsFinished(); - if (is_finished) - { - iterations_since_yield++; - if (iterations_since_yield >= - MAX_ITERATIONS_WITHOUT_YIELD) - { - sync_yield_func(); - iterations_since_yield = 0; - } - } - - update_cce_req.Wait(&yield_fn, &resume_fn); - } - } - } - } - - // Notify cc shards that dirty data has been flushed. This will re-enqueue - // kickout data cc reqs if there are any. - WaitableCc reset_cc( - [&](CcShard &ccs) - { - ccs.OnDirtyDataFlushed(); - return true; - }, - updated_ckpt_ts_core_ids.size()); - reset_cc.SetCoroCallbacks(&yield_fn, &resume_fn); - for (uint16_t core_idx : updated_ckpt_ts_core_ids) - { - EnqueueToCcShard(core_idx, &reset_cc); - } - reset_cc.Wait(&yield_fn, &resume_fn); + // Non-MVCC EloqStore publishes ckpt ts per partition from + // PartitionBatchCallback. Stores requiring PersistKV, and all MVCC + // flushes, publish in the deferred block above after the full durability + // boundary. UpdateCceCkptTsCc restarts each affected shard's eviction scan + // in either path. auto ckpt_err = succ ? DataSyncTask::CkptErrorCode::NO_ERROR : DataSyncTask::CkptErrorCode::FLUSH_ERROR; // notify waiting data sync scan thread + // Whatever the per-partition reports did not cover -- a failed or skipped + // PutAll, or integer division remainder. uint64_t old_usage = data_sync_mem_controller_.DeallocateFlushMemQuota( - cur_work->pending_flush_size_); + quota_progress.task_flush_quota_ - quota_progress.released_); DLOG(INFO) << "DelocateFlushDataMemQuota old_usage: " << old_usage - << " new_usage: " << old_usage - cur_work->pending_flush_size_ + << " new_usage: " + << old_usage - (quota_progress.task_flush_quota_ - + quota_progress.released_) + << " released_during_flush: " << quota_progress.released_ << " quota: " << data_sync_mem_controller_.FlushMemoryQuota(); PostProcessFlushTaskEntries( diff --git a/tx_service/src/data_sync_task.cpp b/tx_service/src/data_sync_task.cpp index d74c2386d..72e1d3009 100644 --- a/tx_service/src/data_sync_task.cpp +++ b/tx_service/src/data_sync_task.cpp @@ -23,7 +23,10 @@ #include +#include +#include #include +#include #include "cc_req_misc.h" #include "cc_shard.h" @@ -60,6 +63,98 @@ void ReportCheckpointResult( } } // namespace +NewestTermByNodeGroup FindNewestTerms(const FlushTaskEntryMap &flush_task) +{ + NewestTermByNodeGroup newest_terms; + for (const auto &[_, entries] : flush_task) + { + for (const auto &entry : entries) + { + const DataSyncTask *task = entry->data_sync_task_.get(); + assert(task != nullptr); + auto [term_it, inserted] = newest_terms.try_emplace( + task->node_group_id_, task->node_group_term_); + if (!inserted) + { + term_it->second = + std::max(term_it->second, task->node_group_term_); + } + } + } + return newest_terms; +} + +bool IsNewestTerm(const FlushTaskEntry &entry, + const NewestTermByNodeGroup &newest_terms) +{ + const DataSyncTask *task = entry.data_sync_task_.get(); + assert(task != nullptr); + auto term_it = newest_terms.find(task->node_group_id_); + assert(term_it != newest_terms.end()); + return task->node_group_term_ == term_it->second; +} + +std::vector CollectCkptTsUpdateGroups( + const std::vector> &table_entries, + const NewestTermByNodeGroup &newest_terms, + size_t cc_shard_count) +{ + std::vector update_groups; + if (table_entries.empty()) + { + return update_groups; + } + + assert(cc_shard_count > 0); + const DataSyncTask *first_task = + table_entries.front()->data_sync_task_.get(); + assert(first_task != nullptr); + const bool hash_partitioned = first_task->table_name_.IsHashPartitioned(); + std::unordered_map group_indices; + + for (const auto &entry : table_entries) + { + const DataSyncTask *task = entry->data_sync_task_.get(); + assert(task != nullptr); + assert(task->table_name_ == first_task->table_name_); + if (!IsNewestTerm(*entry, newest_terms) || + !task->need_update_ckpt_ts_ || entry->data_sync_vec_ == nullptr) + { + continue; + } + + const size_t core_idx = + hash_partitioned + ? static_cast(task->id_) + : static_cast((task->id_ & 0x3FF) % cc_shard_count); + assert(core_idx < cc_shard_count); + + for (const FlushRecord &record : *entry->data_sync_vec_) + { + if (record.cce_ == nullptr) + { + continue; + } + + auto [group_it, inserted] = group_indices.try_emplace( + task->node_group_id_, update_groups.size()); + if (inserted) + { + update_groups.emplace_back(CkptTsUpdateGroup{ + task->node_group_id_, task->node_group_term_, {}}); + } + + CkptTsUpdateGroup &group = update_groups[group_it->second]; + assert(group.node_group_id_ == task->node_group_id_); + assert(group.node_group_term_ == task->node_group_term_); + group.cce_entries_[core_idx].emplace_back( + record.cce_, record.commit_ts_, record.post_flush_size_); + } + } + + return update_groups; +} + DataSyncStatus::DataSyncStatus(NodeGroupId node_group_id, int64_t node_group_term, bool need_truncate_log, @@ -176,7 +271,6 @@ DataSyncTask::DataSyncTask(const TableName &table_name, int32_t old_range_id = range_entry_->GetRangeInfo()->PartitionId(); uint16_t old_range_owner_shard = static_cast((old_range_id & 0x3FF) % local_shard_count); - cce_owner_core_ = old_range_owner_shard; uint16_t new_range_owner_shard = static_cast((id_ & 0x3FF) % local_shard_count); need_update_ckpt_ts_ = diff --git a/tx_service/tests/CMakeLists.txt b/tx_service/tests/CMakeLists.txt index fa65ca1e8..dc6de2620 100644 --- a/tx_service/tests/CMakeLists.txt +++ b/tx_service/tests/CMakeLists.txt @@ -66,7 +66,9 @@ set(CATCH_MAIN_TESTS CcRequestWait-Test CcRequestPool-Test CheckpointMetricsState-Test + CheckpointFlush-Test FetchRecordCc-Test + RealDataStore-Test NonBlockingLock-Test AcquireAllError-Test StandbyForward-Test diff --git a/tx_service/tests/CheckpointFlush-Test.cpp b/tx_service/tests/CheckpointFlush-Test.cpp new file mode 100644 index 000000000..2ecc4585a --- /dev/null +++ b/tx_service/tests/CheckpointFlush-Test.cpp @@ -0,0 +1,1911 @@ +/** + * Copyright (C) 2025 EloqData Inc. + * + * This program is free software: you can redistribute it and/or modify it + * under either GNU Affero General Public License v3 or GNU General Public + * License v2. + */ + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "cc/cc_req_base.h" +#include "cc/cc_req_misc.h" +#include "cc/cc_request.h" +#define private public +#include "cc/local_cc_shards.h" +#undef private +#include "data_store_service_client_closure.h" +#include "data_sync_task.h" +#include "eloq_basic_catalog_factory.h" +#include "eloq_data_store_service/data_store_service.h" +#include "eloq_string_key_record.h" +#include "harness/mem_data_store_factory.h" +#include "harness/port_util.h" +#include "harness/test_node.h" +#include "include/mock/mock_catalog_factory.h" + +using namespace std::chrono_literals; +using namespace txservice; + +namespace +{ +class ListRequest : public CcRequestBase +{ +public: + bool Execute(CcShard &) override + { + return false; + } +}; + +class ShardRequest : public CcRequestBase +{ +public: + explicit ShardRequest(bool abort_if_oom = false) + : abort_if_oom_(abort_if_oom) + { + } + + bool Execute(CcShard &) override + { + execute_count_.fetch_add(1, std::memory_order_release); + return true; + } + + bool AbortIfOom() const override + { + return abort_if_oom_; + } + + void AbortCcRequest(CcErrorCode error) override + { + abort_error_.store(error, std::memory_order_release); + Free(); + } + + int ExecuteCount() const + { + return execute_count_.load(std::memory_order_acquire); + } + + CcErrorCode AbortError() const + { + return abort_error_.load(std::memory_order_acquire); + } + +private: + const bool abort_if_oom_; + std::atomic execute_count_{0}; + std::atomic abort_error_{CcErrorCode::NO_ERROR}; +}; + +class ShardCleanerFixture +{ +public: + explicit ShardCleanerFixture(uint32_t node_memory_limit_mb) + : tx_cnf_{{"node_memory_limit_mb", node_memory_limit_mb}, + {"enable_key_cache", 0}, + {"reltime_sampling", 0}, + {"range_split_worker_num", 1}, + {"range_slice_memory_limit_percent", 20}, + {"core_num", 1}, + {"realtime_sampling", 0}, + {"checkpointer_interval", 10}, + {"checkpointer_delay_seconds", 0}, + {"checkpointer_min_ckpt_request_interval", 5}, + {"enable_shard_heap_defragment", 0}, + {"node_log_limit_mb", 1000}, + {"collect_active_tx_ts_interval_seconds", 2}, + {"rep_group_cnt", 1}}, + catalog_factories_{ + &catalog_factory_, &catalog_factory_, &catalog_factory_}, + local_shards_(/*node_id=*/0, + /*ng_id=*/0, + tx_cnf_, + catalog_factories_, + /*system_handler=*/nullptr, + &ng_configs_, + /*cluster_config_version=*/2, + /*store_hd=*/nullptr, + /*tx_service=*/nullptr, + /*enable_mvcc=*/false) + { + local_shards_.BindThreadToFastMetaDataShard(0); + shard_ = local_shards_.GetCcShard(0); + assert(shard_ != nullptr); + shard_->Init(); + } + + CcShard &Shard() + { + return *shard_; + } + +private: + std::unordered_map> ng_configs_{ + {0, {NodeConfig(0, "127.0.0.1", 8600)}}}; + std::map tx_cnf_; + MockCatalogFactory catalog_factory_; + CatalogFactory *catalog_factories_[NUM_EXTERNAL_ENGINES]; + LocalCcShards local_shards_; + CcShard *shard_{nullptr}; +}; + +class Watchdog +{ +public: + explicit Watchdog(std::chrono::milliseconds budget) + { + thread_ = std::thread( + [this, budget] + { + const auto deadline = std::chrono::steady_clock::now() + budget; + while (!done_.load(std::memory_order_acquire)) + { + if (std::chrono::steady_clock::now() >= deadline) + { + std::abort(); + } + std::this_thread::sleep_for(1ms); + } + }); + } + + ~Watchdog() + { + done_.store(true, std::memory_order_release); + thread_.join(); + } + +private: + std::atomic done_{false}; + std::thread thread_; +}; + +DataSyncTask MakeTask(const TableName &table_name, int64_t term) +{ + return DataSyncTask(table_name, + /*id=*/0, + /*range_version=*/0, + /*ng_id=*/1, + term, + /*data_sync_ts=*/100, + /*status=*/nullptr, + /*is_dirty=*/false, + /*need_adjust_ts=*/false, + /*hres=*/nullptr); +} + +std::shared_ptr MakeTaskPtr(const TableName &table_name, + int64_t term, + NodeGroupId node_group_id = 1, + int32_t id = 0) +{ + return std::make_shared(table_name, + id, + /*range_version=*/0, + node_group_id, + term, + /*data_sync_ts=*/100, + /*status=*/nullptr, + /*is_dirty=*/false, + /*need_adjust_ts=*/false, + /*hres=*/nullptr); +} + +std::unique_ptr MakeFlushEntry( + std::shared_ptr task, + std::unique_ptr> records) +{ + return std::make_unique( + std::move(records), + std::make_unique>(), + std::make_unique>>(), + /*data_sync_txm=*/nullptr, + std::move(task), + /*table_schema=*/nullptr, + /*size=*/0); +} + +std::unique_ptr MakeArchiveEntry( + std::shared_ptr task, + std::unique_ptr> records) +{ + return std::make_unique( + std::make_unique>(), + std::move(records), + std::make_unique>>(), + /*data_sync_txm=*/nullptr, + std::move(task), + /*table_schema=*/nullptr, + /*size=*/0); +} + +std::unique_ptr MakeMvBaseEntry( + std::shared_ptr task, + std::unique_ptr>> base_records) +{ + return std::make_unique( + std::make_unique>(), + std::make_unique>(), + std::move(base_records), + /*data_sync_txm=*/nullptr, + std::move(task), + /*table_schema=*/nullptr, + /*size=*/0); +} + +EloqDS::remote::DataStoreError ReadKey(EloqDS::DataStoreService &service, + std::string_view table_name, + int32_t partition_id, + std::string_view key, + uint64_t &record_ts) +{ + std::string record; + uint64_t ttl = 0; + EloqDS::remote::CommonResult result; + service.Read(table_name, + partition_id, + /*shard_id=*/0, + key, + /*reopen=*/false, + &record, + &record_ts, + &ttl, + &result, + /*done=*/nullptr); + return static_cast(result.error_code()); +} + +FlushRecord MakeObjectRecord(int key, + std::string value, + RecordStatus status, + uint64_t commit_ts, + uint64_t ttl, + int32_t partition_id) +{ + std::shared_ptr payload; + if (status == RecordStatus::Normal) + { + auto blob = std::make_shared(); + blob->value_ = std::move(value); + blob->ttl_ = ttl; + payload = std::move(blob); + } + return FlushRecord( + TxKey(std::make_unique(std::to_string(key))), + std::move(payload), + status, + commit_ts, + /*cce=*/nullptr, + /*post_flush_size=*/0, + partition_id); +} + +FlushRecord MakeSerializedRecord(int key, + std::string value, + RecordStatus status, + uint64_t commit_ts, + int32_t partition_id) +{ + std::shared_ptr payload; + if (status == RecordStatus::Normal) + { + auto record = std::make_shared(); + record->SetEncodedBlob( + reinterpret_cast(value.data()), + value.size()); + payload = std::move(record); + } + return FlushRecord( + TxKey(std::make_unique(std::to_string(key))), + std::move(payload), + status, + commit_ts, + /*cce=*/nullptr, + /*post_flush_size=*/0, + partition_id); +} + +class PutAllFixture +{ +public: + explicit PutAllFixture(bool fail_flush_data = false) + { + GFLAGS_NAMESPACE::SetCommandLineOption("bthread_concurrency", "4"); +#ifdef ELOQ_MODULE_ENABLED + // A later dispatch test starts TxService on the same process-global + // brpc worker pool. Configure those workers before this fixture starts + // the first brpc server, matching TestNode's bring-up requirement. + GFLAGS_NAMESPACE::SetCommandLineOption("brpc_worker_as_ext_processor", + "true"); + GFLAGS_NAMESPACE::SetCommandLineOption("worker_polling_time_us", + "100000"); +#endif + dir_ = std::filesystem::temp_directory_path() / + ("checkpoint_putall_" + std::to_string(::getpid()) + "_" + + std::to_string(reinterpret_cast(this))); + std::filesystem::create_directories(dir_); + + constexpr int kMaxBindRetries = 16; + for (int attempt = 0; attempt < kMaxBindRetries && !service_; ++attempt) + { + auto [fd, port] = txservice::test::BindEphemeralPort(); + ::close(fd); + cluster_manager_.Initialize("127.0.0.1", port); + auto candidate = std::make_unique( + cluster_manager_, + (dir_ / "dss_config.ini").string(), + (dir_ / "DSMigrateLog").string(), + std::make_unique(fail_flush_data)); + if (candidate->StartService(/*create_db_if_missing=*/true)) + { + service_ = std::move(candidate); + } + } + if (!service_) + { + throw std::runtime_error("failed to start PutAll test service"); + } + + txservice::CatalogFactory *catalog_factories[3]{ + &range_catalog_factory_, &hash_catalog_factory_, nullptr}; + client_ = std::make_unique( + /*is_bootstrap=*/false, + catalog_factories, + cluster_manager_, + /*bind_data_shard_with_ng=*/false, + service_.get()); + } + + ~PutAllFixture() + { + client_.reset(); + service_.reset(); + std::filesystem::remove_all(dir_); + } + + EloqDS::DataStoreServiceClient &Client() + { + return *client_; + } + + EloqDS::DataStoreService &Service() + { + return *service_; + } + +private: + std::filesystem::path dir_; + EloqRangeCatalogFactory range_catalog_factory_; + EloqHashCatalogFactory hash_catalog_factory_; + EloqDS::DataStoreServiceClusterManager cluster_manager_; + std::unique_ptr service_; + std::unique_ptr client_; +}; +} // namespace + +TEST_CASE("CcRequestList preserves intrusive links across every removal shape", + "[checkpoint-flush][cc-request-list]") +{ + CcRequestList list; + ListRequest first; + ListRequest second; + ListRequest third; + + REQUIRE(list.Empty()); + REQUIRE(list.Size() == 0); + REQUIRE(list.Front() == nullptr); + REQUIRE_FALSE(list.Contains(&first)); + + list.PushBack(&first); + REQUIRE(list.Contains(&first)); + REQUIRE_FALSE(list.Contains(&second)); + list.PushBack(&second); + list.PushBack(&third); + REQUIRE(list.Size() == 3); + REQUIRE(list.Front() == &first); + REQUIRE(list.Contains(&first)); + REQUIRE(list.Contains(&second)); + REQUIRE(list.Contains(&third)); + REQUIRE(CcRequestList::NextOf(&first) == &second); + REQUIRE(CcRequestList::NextOf(&second) == &third); + + list.Remove(&second); + REQUIRE(list.Size() == 2); + REQUIRE_FALSE(list.Contains(&second)); + REQUIRE(CcRequestList::NextOf(&first) == &third); + + REQUIRE(list.PopFront() == &first); + REQUIRE(list.Front() == &third); + REQUIRE(list.PopFront() == &third); + REQUIRE(list.PopFront() == nullptr); + REQUIRE(list.Empty()); + + // Removal clears both intrusive links, so the same request can safely park + // again on this or a different wait collection. + list.PushBack(&second); + REQUIRE(list.Contains(&second)); + REQUIRE(list.PopFront() == &second); + REQUIRE_FALSE(list.Contains(&second)); + REQUIRE(list.Empty()); + + second.Use(); + REQUIRE(second.InUse()); + second.Free(); + REQUIRE_FALSE(second.InUse()); +} + +TEST_CASE("shard cleaner preserves a wake received during an active pass", + "[checkpoint-flush][shard-clean][sticky-wake]") +{ + // These requests outlive local_shards, whose unprocessed queue retains + // their addresses until fixture teardown. + ShardRequest parked_request; + ShardCleanCc clean_pass; + ShardCleanerFixture fixture(/*node_memory_limit_mb=*/0); + CcShard &shard = fixture.Shard(); + + // Initialization may already have queued the cleaner when the zero-byte + // heap became full. Either way, this call leaves the owned cleaner in-use + // and queued without a processor consuming it. + shard.WakeUpShardCleanCc(); + const size_t active_cleaner_queue_size = shard.QueueSize(); + REQUIRE(active_cleaner_queue_size > 0); + + // This second call must record a sticky wake instead of enqueueing another + // copy of the cleaner. + shard.WakeUpShardCleanCc(); + REQUIRE(shard.QueueSize() == active_cleaner_queue_size); + + parked_request.Use(); + shard.EnqueueWaitListIfMemoryFull(&parked_request); + REQUIRE(shard.WaitListSizeForMemory() == 1); + + // A zero-byte heap is deterministically full. Marking defrag in-flight + // keeps the unproductive-pass branch from notifying a checkpointer, which + // is intentionally absent from this standalone fixture. + shard.GetShardHeap()->SetDefragHeapCcOnFly(true); + clean_pass.Use(); + REQUIRE_FALSE(clean_pass.Execute(shard)); + + // Execute consumed the sticky wake and queued another pass rather than + // returning true while the ordinary request remained parked. + REQUIRE(shard.WaitListSizeForMemory() == 1); + REQUIRE(shard.QueueSize() == active_cleaner_queue_size + 1); + REQUIRE_FALSE(shard.TakeShardCleanCcWakeUp()); +} + +TEST_CASE("shard cleaner clears a sticky wake after draining parked requests", + "[checkpoint-flush][shard-clean][sticky-wake]") +{ + ShardRequest parked_request; + ShardCleanCc clean_pass; + ShardCleanerFixture fixture(/*node_memory_limit_mb=*/1000); + CcShard &shard = fixture.Shard(); + + shard.WakeUpShardCleanCc(); + const size_t active_cleaner_queue_size = shard.QueueSize(); + REQUIRE(active_cleaner_queue_size > 0); + shard.WakeUpShardCleanCc(); + REQUIRE(shard.QueueSize() == active_cleaner_queue_size); + + parked_request.Use(); + shard.EnqueueWaitListIfMemoryFull(&parked_request); + clean_pass.Use(); + REQUIRE(clean_pass.Execute(shard)); + + // Available memory released the parked request. The concurrent wake has + // therefore already been served and must not survive into a later pass. + REQUIRE(shard.WaitListSizeForMemory() == 0); + REQUIRE(shard.QueueSize() == active_cleaner_queue_size + 1); + REQUIRE_FALSE(shard.TakeShardCleanCcWakeUp()); +} + +TEST_CASE("checkpoint publication honors backend and MVCC durability bounds", + "[checkpoint-flush][durability]") +{ + REQUIRE_FALSE(DeferCkptTsUpdate(/*need_persist_kv=*/false, + /*enable_mvcc=*/false)); + REQUIRE(DeferCkptTsUpdate(/*need_persist_kv=*/true, + /*enable_mvcc=*/false)); + REQUIRE(DeferCkptTsUpdate(/*need_persist_kv=*/false, + /*enable_mvcc=*/true)); + REQUIRE(DeferCkptTsUpdate(/*need_persist_kv=*/true, + /*enable_mvcc=*/true)); +} + +TEST_CASE("UpdateCceCkptTsCc fan-in publishes one completion", + "[checkpoint-flush][ckpt-fan-in]") +{ + Watchdog watchdog(20s); + constexpr size_t kCoreCount = 4; + absl::flat_hash_map> + entries; + for (size_t core = 0; core < kCoreCount; ++core) + { + entries[core].emplace_back(nullptr, 10 + core, 0); + } + + const TableName table{ + std::string_view("fan_in"), TableType::Primary, TableEngine::EloqKv}; + UpdateCceCkptTsCc request(/*node_group_id=*/1, + /*term=*/7, + table, + entries); + std::atomic callback_count{0}; + request.SetOnFinished( + [&callback_count] + { callback_count.fetch_add(1, std::memory_order_relaxed); }); + + std::vector finishers; + for (size_t core = 0; core < kCoreCount; ++core) + { + finishers.emplace_back([&request] { request.SetFinished(); }); + } + for (auto &finisher : finishers) + { + finisher.join(); + } + + REQUIRE(request.IsFinished()); + REQUIRE(callback_count.load(std::memory_order_relaxed) == 1); +} + +TEST_CASE("UpdateCceCkptTsCc blocking waiter waits for every core", + "[checkpoint-flush][ckpt-fan-in]") +{ + Watchdog watchdog(20s); + absl::flat_hash_map> + entries; + entries[0].emplace_back(nullptr, 10, 0); + entries[1].emplace_back(nullptr, 11, 0); + + const TableName table{ + std::string_view("wait"), TableType::Primary, TableEngine::EloqKv}; + + SECTION("waiter starts before completion") + { + std::thread first; + std::thread second; + bool finished = false; + { + UpdateCceCkptTsCc request(1, 7, table, entries); + first = std::thread([&request] { request.SetFinished(); }); + second = std::thread( + [&request] + { + std::this_thread::sleep_for(2ms); + request.SetFinished(); + }); + + request.Wait(); + finished = request.IsFinished(); + } + // Wait returning must be a sufficient lifetime barrier; callers do not + // join cc-shard threads before destroying a stack-owned request. + first.join(); + second.join(); + REQUIRE(finished); + } + + SECTION("completion arrives before Wait") + { + UpdateCceCkptTsCc request(1, 7, table, entries); + request.SetFinished(); + request.SetFinished(); + request.Wait(); + REQUIRE(request.IsFinished()); + } +} + +TEST_CASE("UpdateCceCkptTsCc coroutine wakeup covers both completion races", + "[checkpoint-flush][ckpt-fan-in]") +{ + Watchdog watchdog(20s); + absl::flat_hash_map> + entries; + entries[0].emplace_back(nullptr, 10, 0); + const TableName table{ + std::string_view("coro_wait"), TableType::Primary, TableEngine::EloqKv}; + + SECTION("waiter arms before completion") + { + UpdateCceCkptTsCc request(1, 7, table, entries); + std::mutex scheduler_mutex; + std::condition_variable scheduler_cv; + bool yielded = false; + bool resume_permit = false; + const std::function yield = [&] + { + std::unique_lock lk(scheduler_mutex); + yielded = true; + scheduler_cv.notify_all(); + scheduler_cv.wait(lk, [&] { return resume_permit; }); + }; + const std::function resume = [&] + { + std::lock_guard lk(scheduler_mutex); + resume_permit = true; + scheduler_cv.notify_all(); + }; + request.SetCoroCallbacks(&yield, &resume); + std::thread finisher( + [&] + { + std::unique_lock lk(scheduler_mutex); + scheduler_cv.wait(lk, [&] { return yielded; }); + lk.unlock(); + request.SetFinished(); + }); + + request.Wait(); + finisher.join(); + REQUIRE(request.IsFinished()); + } + + SECTION("completion arrives before waiter arms") + { + int yield_count = 0; + const std::function yield = [&] { ++yield_count; }; + const std::function resume = [] {}; + std::thread finisher; + bool finished = false; + { + UpdateCceCkptTsCc request(1, 7, table, entries); + request.SetCoroCallbacks(&yield, &resume); + finisher = std::thread([&request] { request.SetFinished(); }); + while (!request.IsFinished()) + { + std::this_thread::yield(); + } + request.Wait(); + finished = request.IsFinished(); + } + // The terminal shard may still be returning from SetFinished(), but it + // must not access the request after publishing zero for an unarmed + // coroutine waiter. + finisher.join(); + REQUIRE(finished); + REQUIRE(yield_count == 0); + } +} + +TEST_CASE("UpdateCceCkptTsCc coroutine fan-in resumes exactly once", + "[checkpoint-flush][ckpt-fan-in]") +{ + Watchdog watchdog(20s); + constexpr size_t kCoreCount = 4; + absl::flat_hash_map> + entries; + for (size_t core = 0; core < kCoreCount; ++core) + { + entries[core].emplace_back(nullptr, 10 + core, 0); + } + const TableName table{std::string_view("coro_fan_in"), + TableType::Primary, + TableEngine::EloqKv}; + + std::mutex scheduler_mutex; + std::condition_variable scheduler_cv; + bool yielded = false; + bool resume_permit = false; + int resume_count = 0; + const std::function yield = [&] + { + std::unique_lock lk(scheduler_mutex); + yielded = true; + scheduler_cv.notify_all(); + scheduler_cv.wait(lk, [&] { return resume_permit; }); + }; + const std::function resume = [&] + { + std::lock_guard lk(scheduler_mutex); + ++resume_count; + resume_permit = true; + scheduler_cv.notify_all(); + }; + + std::vector finishers; + bool finished = false; + { + UpdateCceCkptTsCc request(1, 7, table, entries); + request.SetCoroCallbacks(&yield, &resume); + for (size_t core = 0; core < kCoreCount; ++core) + { + finishers.emplace_back( + [&] + { + { + std::unique_lock lk(scheduler_mutex); + scheduler_cv.wait(lk, [&] { return yielded; }); + } + request.SetFinished(); + }); + } + + request.Wait(); + finished = request.IsFinished(); + } + for (auto &finisher : finishers) + { + finisher.join(); + } + + REQUIRE(finished); + REQUIRE(resume_count == 1); +} + +TEST_CASE("SyncPutAllData releases progress per completion and wakes once", + "[checkpoint-flush][partition-progress]") +{ + Watchdog watchdog(20s); + EloqDS::SyncPutAllData sync; + sync.Reset(); + sync.total_partitions_ = 3; + sync.total_bytes_ = 60; + + std::mutex scheduler_mutex; + std::condition_variable scheduler_cv; + int yield_count = 0; + int resume_permits = 0; + std::vector> progress; + + const std::function yield = [&] + { + std::unique_lock lk(scheduler_mutex); + ++yield_count; + scheduler_cv.notify_all(); + scheduler_cv.wait(lk, [&] { return resume_permits > 0; }); + --resume_permits; + }; + const std::function resume = [&] + { + std::lock_guard lk(scheduler_mutex); + ++resume_permits; + scheduler_cv.notify_all(); + }; + const std::function report = + [&](uint64_t done, uint64_t total) + { progress.emplace_back(done, total); }; + sync.SetCoroCallbacks(&yield, &resume); + sync.SetProgressCallback(&report); + + // The quota release runs inside OnPartitionCompleted on the completing + // thread; the waiter is only woken by the final completion. Progress must + // therefore accumulate without any intermediate waiter wake-ups. + std::thread completions( + [&] + { + { + std::unique_lock lk(scheduler_mutex); + scheduler_cv.wait(lk, [&] { return yield_count >= 1; }); + } + sync.OnPartitionCompleted(10); + sync.OnPartitionCompleted(20); + sync.OnPartitionCompleted(30); + }); + + sync.Wait(&yield, &resume); + completions.join(); + + // One suspension, one wake: intermediate completions released quota + // directly instead of resuming the waiter. + REQUIRE(yield_count == 1); + REQUIRE(progress.size() == 3); + REQUIRE((progress[0] == std::pair{10, 60})); + REQUIRE((progress[1] == std::pair{30, 60})); + REQUIRE((progress[2] == std::pair{60, 60})); +} + +TEST_CASE("SyncPutAllData clears progress callbacks before pool reuse", + "[checkpoint-flush][partition-progress][pool-lifecycle]") +{ + EloqDS::SyncPutAllData sync; + sync.Reset(); + const std::function report = [](uint64_t, + uint64_t) {}; + + sync.SetProgressCallback(&report); + REQUIRE(sync.progress_fn_ == &report); + sync.Reset(); + REQUIRE(sync.progress_fn_ == nullptr); + + sync.SetProgressCallback(&report); + sync.Clear(); + REQUIRE(sync.progress_fn_ == nullptr); +} + +TEST_CASE("one partition combines checkpoint entries from one term", + "[checkpoint-flush][partition-term]") +{ + const TableName table{ + std::string_view("one_term"), TableType::Primary, TableEngine::EloqKv}; + DataSyncTask first_task = MakeTask(table, 10); + DataSyncTask second_task = MakeTask(table, 10); + + EloqDS::PartitionFlushState partition; + partition.Reset(/*pid=*/7, /*is_range_partitioned=*/false); + partition.serialized_bytes_ = 99; + partition.AddCkptTsEntry(&first_task, 0, nullptr, 100, 1); + partition.AddCkptTsEntry(&first_task, 0, nullptr, 101, 2); + partition.AddCkptTsEntry(&second_task, 0, nullptr, 102, 3); + + EloqDS::SyncPutAllData sync; + sync.Reset(); + sync.total_partitions_ = 1; + sync.total_bytes_ = 99; + partition.ArmCkptTsUpdate(&sync); + + REQUIRE(partition.ckpt_ts_task_ == &first_task); + REQUIRE(partition.ckpt_ts_entries_.at(0).size() == 3); + REQUIRE(partition.ckpt_ts_update_.has_value()); + partition.ckpt_ts_update_->SetFinished(); + REQUIRE(sync.completed_partitions_ == 1); + REQUIRE(sync.completed_bytes_ == 99); +} + +TEST_CASE("deferred checkpoint publication aggregates only newest-term entries", + "[checkpoint-flush][partition-term][deferred-publication]") +{ + const TableName hash_table{std::string_view("deferred_hash"), + TableType::Primary, + TableEngine::EloqKv}; + const TableName stale_table{std::string_view("deferred_stale"), + TableType::Primary, + TableEngine::EloqKv}; + FlushTaskEntryMap flush_task; + + auto add_hash_record = [&](std::string_view kv_table_name, + const TableName &table_name, + int64_t term, + NodeGroupId node_group_id, + int32_t core_id, + uintptr_t cce_address, + uint64_t commit_ts, + bool need_update_ckpt_ts = true) + { + auto task = MakeTaskPtr(table_name, term, node_group_id, core_id); + task->need_update_ckpt_ts_ = need_update_ckpt_ts; + auto records = std::make_unique>(); + records->push_back(MakeObjectRecord(static_cast(commit_ts), + "value", + RecordStatus::Normal, + commit_ts, + /*ttl=*/UINT64_MAX, + /*partition_id=*/core_id)); + records->back().cce_ = reinterpret_cast(cce_address); + records->back().post_flush_size_ = commit_ts; + flush_task[kv_table_name].push_back( + MakeFlushEntry(std::move(task), std::move(records))); + }; + + // Node group 1 retains both term-31 entries and combines their shard maps. + // The older entries surround them to ensure selection is independent of + // input order. Node group 2 has an unrelated, lower numeric term and must + // remain eligible. + add_hash_record("eloqkv_deferred_hash", hash_table, 30, 1, 0, 0x10, 100); + add_hash_record("eloqkv_deferred_hash", hash_table, 31, 1, 0, 0x20, 101); + add_hash_record("eloqkv_deferred_hash", hash_table, 30, 1, 1, 0x30, 102); + add_hash_record("eloqkv_deferred_hash", hash_table, 31, 1, 1, 0x40, 103); + add_hash_record("eloqkv_deferred_hash", hash_table, 8, 2, 2, 0x50, 104); + add_hash_record("eloqkv_deferred_hash", hash_table, 31, 1, 3, 0, 105); + add_hash_record("eloqkv_deferred_hash", + hash_table, + 31, + 1, + 3, + 0x60, + 106, + /*need_update_ckpt_ts=*/false); + auto null_vector_entry = MakeFlushEntry( + MakeTaskPtr(hash_table, /*term=*/31, /*node_group_id=*/1, /*id=*/3), + std::make_unique>()); + null_vector_entry->data_sync_vec_.reset(); + flush_task["eloqkv_deferred_hash"].push_back(std::move(null_vector_entry)); + // Term selection is batch-wide. This other table has no retained entry + // because node group 1's term 31 appears above. + add_hash_record("eloqkv_deferred_stale", stale_table, 30, 1, 3, 0x70, 107); + + const NewestTermByNodeGroup newest_terms = FindNewestTerms(flush_task); + REQUIRE(newest_terms.at(1) == 31); + REQUIRE(newest_terms.at(2) == 8); + + std::vector groups = CollectCkptTsUpdateGroups( + flush_task.at("eloqkv_deferred_hash"), newest_terms, 4); + REQUIRE(groups.size() == 2); + + const CkptTsUpdateGroup *node_group_1 = nullptr; + const CkptTsUpdateGroup *node_group_2 = nullptr; + for (const CkptTsUpdateGroup &group : groups) + { + if (group.node_group_id_ == 1) + { + node_group_1 = &group; + } + else if (group.node_group_id_ == 2) + { + node_group_2 = &group; + } + } + + REQUIRE(node_group_1 != nullptr); + REQUIRE(node_group_1->node_group_term_ == 31); + REQUIRE(node_group_1->cce_entries_.size() == 2); + REQUIRE(node_group_1->cce_entries_.at(0).size() == 1); + REQUIRE(node_group_1->cce_entries_.at(0).front().cce_ == + reinterpret_cast(uintptr_t{0x20})); + REQUIRE(node_group_1->cce_entries_.at(1).size() == 1); + REQUIRE(node_group_1->cce_entries_.at(1).front().cce_ == + reinterpret_cast(uintptr_t{0x40})); + + REQUIRE(node_group_2 != nullptr); + REQUIRE(node_group_2->node_group_term_ == 8); + REQUIRE(node_group_2->cce_entries_.size() == 1); + REQUIRE(node_group_2->cce_entries_.at(2).size() == 1); + REQUIRE(CollectCkptTsUpdateGroups( + flush_task.at("eloqkv_deferred_stale"), newest_terms, 4) + .empty()); + + std::vector> empty_entries; + REQUIRE(CollectCkptTsUpdateGroups(empty_entries, newest_terms, 4).empty()); + REQUIRE(FindNewestTerms(FlushTaskEntryMap{}).empty()); +} + +TEST_CASE("deferred range checkpoint publication maps every retained range", + "[checkpoint-flush][partition-term][deferred-publication]") +{ + const TableName table{std::string_view("deferred_range"), + TableType::Primary, + TableEngine::InternalRange}; + FlushTaskEntryMap flush_task; + auto add_range_record = [&](int64_t term, + int32_t range_id, + uintptr_t cce_address, + uint64_t commit_ts) + { + auto records = std::make_unique>(); + records->push_back(MakeSerializedRecord(static_cast(commit_ts), + "value", + RecordStatus::Normal, + commit_ts, + range_id)); + records->back().cce_ = reinterpret_cast(cce_address); + flush_task["irange_deferred_range"].push_back(MakeFlushEntry( + MakeTaskPtr(table, term, /*node_group_id=*/3, range_id), + std::move(records))); + }; + + add_range_record(/*term=*/11, /*range_id=*/1026, 0x10, 200); + add_range_record(/*term=*/12, /*range_id=*/1027, 0x20, 201); + add_range_record(/*term=*/12, /*range_id=*/1028, 0x30, 202); + + const NewestTermByNodeGroup newest_terms = FindNewestTerms(flush_task); + std::vector groups = CollectCkptTsUpdateGroups( + flush_task.at("irange_deferred_range"), newest_terms, 4); + + REQUIRE(groups.size() == 1); + REQUIRE(groups.front().node_group_term_ == 12); + REQUIRE(groups.front().cce_entries_.size() == 2); + REQUIRE(groups.front().cce_entries_.at(3).front().commit_ts_ == 201); + REQUIRE(groups.front().cce_entries_.at(0).front().commit_ts_ == 202); +} + +TEST_CASE("deferred flush dispatches one newest-term update per table shard", + "[checkpoint-flush][partition-term][deferred-publication][dispatch]") +{ + Watchdog watchdog(20s); + PutAllFixture fixture; + std::unordered_map> ng_configs{ + {1, {NodeConfig(0, "127.0.0.1", 8600)}}}; + std::map tx_cnf{ + {"node_memory_limit_mb", 1000}, + {"enable_key_cache", 0}, + {"reltime_sampling", 0}, + {"range_split_worker_num", 1}, + {"range_slice_memory_limit_percent", 20}, + {"core_num", 1}, + {"realtime_sampling", 0}, + {"checkpointer_interval", 10}, + {"checkpointer_delay_seconds", 0}, + {"checkpointer_min_ckpt_request_interval", 5}, + {"enable_shard_heap_defragment", 0}, + {"node_log_limit_mb", 1000}, + {"collect_active_tx_ts_interval_seconds", 2}, + {"rep_group_cnt", 1}}; + MockCatalogFactory catalog_factory; + CatalogFactory *catalog_factories[NUM_EXTERNAL_ENGINES]{ + &catalog_factory, &catalog_factory, &catalog_factory}; + + // RocksDB-backed builds defer because PersistKV is required. EloqStore + // normally publishes base records per partition, so enable MVCC there to + // exercise its deferred-after-archives boundary with the same test. + const bool enable_mvcc = !fixture.Client().NeedPersistKV(); + LocalCcShards local_shards(/*node_id=*/0, + /*ng_id=*/1, + tx_cnf, + catalog_factories, + /*system_handler=*/nullptr, + &ng_configs, + /*cluster_config_version=*/2, + &fixture.Client(), + /*tx_service=*/nullptr, + enable_mvcc); + local_shards.BindThreadToFastMetaDataShard(0); + local_shards.GetCcShard(0)->Init(); + + const TableName table{std::string_view("deferred_dispatch"), + TableType::Primary, + TableEngine::EloqKv}; + FlushDataTask flush_task; + auto add_record = + [&](int64_t term, int key, uintptr_t cce_address, uint64_t commit_ts) + { + auto task = MakeTaskPtr(table, term, /*node_group_id=*/1, /*id=*/0); + // Post-processing is outside this test's concern. Keep one synthetic + // flight outstanding so it does not finish a task with no test status. + task->flight_task_cnt_ = 2; + auto records = std::make_unique>(); + records->push_back(MakeObjectRecord(key, + "value", + RecordStatus::Normal, + commit_ts, + /*ttl=*/UINT64_MAX, + /*partition_id=*/0)); + records->back().cce_ = reinterpret_cast(cce_address); + flush_task.flush_task_entries_["eloqkv_deferred_dispatch"].push_back( + MakeFlushEntry(std::move(task), std::move(records))); + }; + + add_record(/*term=*/998, /*key=*/301, /*cce=*/0x10, /*commit_ts=*/601); + add_record(/*term=*/999, /*key=*/302, /*cce=*/0x20, /*commit_ts=*/602); + add_record(/*term=*/999, /*key=*/303, /*cce=*/0x30, /*commit_ts=*/603); + + size_t processed_cc_requests = 0; + const std::function yield = [&] + { + size_t processed = 0; + do + { + processed = local_shards.ProcessRequests(/*thd_id=*/0); + processed_cc_requests += processed; + } while (processed > 0); + std::this_thread::yield(); + }; + const std::function resume = [] {}; + const std::function sync_yield = [] {}; + + Sharder &sharder = Sharder::Instance(); + const int64_t saved_leader_term = sharder.LeaderTerm(/*ng_id=*/1); + const int64_t saved_standby_term = sharder.StandbyNodeTerm(); + // Keep the sentinel CCE pointers opaque: the stale request must finish at + // its term fence before UpdateCceCkptTsCc dereferences any of them. + sharder.SetLeaderTerm(/*ng_id=*/1, /*term=*/-1); + sharder.SetStandbyNodeTerm(/*standby_term=*/-1); + local_shards.FlushDataImpl( + &flush_task, /*worker_idx=*/0, sync_yield, yield, resume); + sharder.SetLeaderTerm(/*ng_id=*/1, saved_leader_term); + sharder.SetStandbyNodeTerm(saved_standby_term); + + // Both retained entries target shard 0 and are published by one aggregated + // request. The obsolete term-998 entry was neither stored nor published. + REQUIRE(processed_cc_requests == 1); + uint64_t record_ts = 0; + REQUIRE(ReadKey(fixture.Service(), + "eloqkv_deferred_dispatch", + /*partition_id=*/0, + "301", + record_ts) == + EloqDS::remote::DataStoreError::KEY_NOT_FOUND); + REQUIRE(ReadKey(fixture.Service(), + "eloqkv_deferred_dispatch", + /*partition_id=*/0, + "302", + record_ts) == EloqDS::remote::DataStoreError::NO_ERROR); + REQUIRE(record_ts == 602); + REQUIRE(ReadKey(fixture.Service(), + "eloqkv_deferred_dispatch", + /*partition_id=*/0, + "303", + record_ts) == EloqDS::remote::DataStoreError::NO_ERROR); + REQUIRE(record_ts == 603); +} + +TEST_CASE("PersistKV failure does not publish deferred checkpoint timestamps", + "[checkpoint-flush][deferred-publication][persist-failure]") +{ + Watchdog watchdog(20s); + PutAllFixture fixture(/*fail_flush_data=*/true); + if (!fixture.Client().NeedPersistKV()) + { + SKIP("EloqStore has no post-PutAll persistence boundary"); + } + + std::unordered_map> ng_configs{ + {1, {NodeConfig(0, "127.0.0.1", 8600)}}}; + std::map tx_cnf{ + {"node_memory_limit_mb", 1000}, + {"enable_key_cache", 0}, + {"reltime_sampling", 0}, + {"range_split_worker_num", 1}, + {"range_slice_memory_limit_percent", 20}, + {"core_num", 1}, + {"realtime_sampling", 0}, + {"checkpointer_interval", 10}, + {"checkpointer_delay_seconds", 0}, + {"checkpointer_min_ckpt_request_interval", 5}, + {"enable_shard_heap_defragment", 0}, + {"node_log_limit_mb", 1000}, + {"collect_active_tx_ts_interval_seconds", 2}, + {"rep_group_cnt", 1}}; + MockCatalogFactory catalog_factory; + CatalogFactory *catalog_factories[NUM_EXTERNAL_ENGINES]{ + &catalog_factory, &catalog_factory, &catalog_factory}; + LocalCcShards local_shards(/*node_id=*/0, + /*ng_id=*/1, + tx_cnf, + catalog_factories, + /*system_handler=*/nullptr, + &ng_configs, + /*cluster_config_version=*/2, + &fixture.Client(), + /*tx_service=*/nullptr, + /*enable_mvcc=*/false); + local_shards.BindThreadToFastMetaDataShard(0); + local_shards.GetCcShard(0)->Init(); + + const TableName table{std::string_view("persist_failure"), + TableType::Primary, + TableEngine::EloqKv}; + auto task = MakeTaskPtr(table, /*term=*/999, /*node_group_id=*/1, /*id=*/0); + // Keep one synthetic flight outstanding so post-processing records the + // flush error without trying to finish a task whose status is omitted by + // this focused fixture. + task->flight_task_cnt_ = 2; + auto records = std::make_unique>(); + records->push_back(MakeObjectRecord(/*key=*/401, + "value", + RecordStatus::Normal, + /*commit_ts=*/701, + /*ttl=*/UINT64_MAX, + /*partition_id=*/0)); + records->back().cce_ = reinterpret_cast(uintptr_t{0x10}); + + FlushDataTask flush_task; + flush_task.flush_task_entries_["eloqkv_persist_failure"].push_back( + MakeFlushEntry(task, std::move(records))); + + size_t processed_cc_requests = 0; + const std::function yield = [&] + { + size_t processed = 0; + do + { + processed = local_shards.ProcessRequests(/*thd_id=*/0); + processed_cc_requests += processed; + } while (processed > 0); + std::this_thread::yield(); + }; + const std::function resume = [] {}; + const std::function sync_yield = [] {}; + + Sharder &sharder = Sharder::Instance(); + const int64_t saved_leader_term = sharder.LeaderTerm(/*ng_id=*/1); + const int64_t saved_standby_term = sharder.StandbyNodeTerm(); + // If a regression dispatches the update despite failed persistence, the + // stale term stops it before dereferencing the sentinel CCE and the + // request count below still exposes the incorrect publication attempt. + sharder.SetLeaderTerm(/*ng_id=*/1, /*term=*/-1); + sharder.SetStandbyNodeTerm(/*standby_term=*/-1); + local_shards.FlushDataImpl( + &flush_task, /*worker_idx=*/0, sync_yield, yield, resume); + sharder.SetLeaderTerm(/*ng_id=*/1, saved_leader_term); + sharder.SetStandbyNodeTerm(saved_standby_term); + + REQUIRE(processed_cc_requests == 0); + REQUIRE(task->ckpt_err_ == DataSyncTask::CkptErrorCode::FLUSH_ERROR); +} + +TEST_CASE("partition callback reports success and failure exactly once", + "[checkpoint-flush][partition-callback]") +{ + txservice::CatalogFactory *catalog_factories[3]{nullptr, nullptr, nullptr}; + EloqDS::DataStoreServiceClusterManager cluster_manager; + EloqDS::DataStoreServiceClient client(/*is_bootstrap=*/false, + catalog_factories, + cluster_manager, + /*bind_data_shard_with_ng=*/false); + + EloqDS::PartitionFlushState partition; + EloqDS::SyncPutAllData sync; + EloqDS::PartitionCallbackData callback; + EloqDS::remote::CommonResult result; + + SECTION("successful partition with no checkpoint entries") + { + partition.Reset(/*pid=*/3, /*is_range_partitioned=*/false); + partition.serialized_bytes_ = 17; + sync.Reset(); + sync.total_partitions_ = 1; + sync.total_bytes_ = 17; + callback.Reset(&partition, &sync, "table"); + result.set_error_code(EloqDS::remote::DataStoreError::NO_ERROR); + + EloqDS::PartitionBatchCallback(&callback, nullptr, client, result); + + REQUIRE_FALSE(partition.IsFailed()); + REQUIRE(sync.completed_partitions_ == 1); + REQUIRE(sync.completed_bytes_ == 17); + } + + SECTION("failed partition") + { + partition.Reset(/*pid=*/4, /*is_range_partitioned=*/false); + partition.serialized_bytes_ = 23; + sync.Reset(); + sync.total_partitions_ = 1; + sync.total_bytes_ = 23; + callback.Reset(&partition, &sync, "table"); + result.set_error_code(EloqDS::remote::DataStoreError::WRITE_FAILED); + result.set_error_msg("injected write failure"); + + EloqDS::PartitionBatchCallback(&callback, nullptr, client, result); + + REQUIRE(partition.IsFailed()); + REQUIRE(partition.result.error_code() == + EloqDS::remote::DataStoreError::WRITE_FAILED); + REQUIRE(sync.completed_partitions_ == 1); + REQUIRE(sync.completed_bytes_ == 23); + } +} + +TEST_CASE("PutAll discards lower-term records from a merged table batch", + "[checkpoint-flush][partition-term][put-all]") +{ + PutAllFixture fixture; + + SECTION("hash partition") + { + const TableName table{std::string_view("hash_terms"), + TableType::Primary, + TableEngine::EloqKv}; + auto old_before = MakeTaskPtr(table, 30); + auto newest = MakeTaskPtr(table, 31); + auto old_after = MakeTaskPtr(table, 30); + auto same_term = MakeTaskPtr(table, 31); + auto old_only_partition = MakeTaskPtr(table, 30); + auto other_node_group = MakeTaskPtr(table, 10, /*node_group_id=*/2); + + std::unordered_map>> + flush_task; + auto &entries = flush_task["eloqkv_hash_terms"]; + auto add_record = [&](std::shared_ptr task, + int key, + int32_t partition_id = 0) + { + auto records = std::make_unique>(); + records->push_back(MakeObjectRecord(key, + "value", + RecordStatus::Normal, + /*commit_ts=*/100 + key, + /*ttl=*/UINT64_MAX, + partition_id)); + entries.push_back( + MakeFlushEntry(std::move(task), std::move(records))); + }; + add_record(old_before, 1); + add_record(newest, 2); + add_record(old_after, 3); + add_record(same_term, 4); + add_record(old_only_partition, 5, /*partition_id=*/1); + add_record(other_node_group, 6, /*partition_id=*/2); + + const TableName stale_other_table{ + std::string_view("hash_terms_stale_table"), + TableType::Primary, + TableEngine::EloqKv}; + auto stale_other_records = std::make_unique>(); + stale_other_records->push_back(MakeObjectRecord(7, + "stale-table-value", + RecordStatus::Normal, + /*commit_ts=*/107, + /*ttl=*/UINT64_MAX, + /*partition_id=*/3)); + flush_task["eloqkv_hash_terms_stale_table"].push_back( + MakeFlushEntry(MakeTaskPtr(stale_other_table, /*term=*/30), + std::move(stale_other_records))); + + REQUIRE(fixture.Client().PutAll(flush_task)); + REQUIRE(fixture.Service().GetApproxStoreKeyCount(/*shard_id=*/0) == 3); + uint64_t record_ts = 0; + REQUIRE( + ReadKey( + fixture.Service(), "eloqkv_hash_terms", 0, "1", record_ts) == + EloqDS::remote::DataStoreError::KEY_NOT_FOUND); + REQUIRE( + ReadKey( + fixture.Service(), "eloqkv_hash_terms", 0, "2", record_ts) == + EloqDS::remote::DataStoreError::NO_ERROR); + REQUIRE(record_ts == 102); + REQUIRE( + ReadKey( + fixture.Service(), "eloqkv_hash_terms", 0, "3", record_ts) == + EloqDS::remote::DataStoreError::KEY_NOT_FOUND); + REQUIRE( + ReadKey( + fixture.Service(), "eloqkv_hash_terms", 0, "4", record_ts) == + EloqDS::remote::DataStoreError::NO_ERROR); + REQUIRE(record_ts == 104); + REQUIRE( + ReadKey( + fixture.Service(), "eloqkv_hash_terms", 1, "5", record_ts) == + EloqDS::remote::DataStoreError::KEY_NOT_FOUND); + REQUIRE( + ReadKey( + fixture.Service(), "eloqkv_hash_terms", 2, "6", record_ts) == + EloqDS::remote::DataStoreError::NO_ERROR); + REQUIRE(record_ts == 106); + REQUIRE(ReadKey(fixture.Service(), + "eloqkv_hash_terms_stale_table", + 3, + "7", + record_ts) == + EloqDS::remote::DataStoreError::KEY_NOT_FOUND); + } + + SECTION("range partition") + { + const TableName table{std::string_view("range_terms"), + TableType::Primary, + TableEngine::InternalRange}; + auto old_before = MakeTaskPtr(table, 40); + auto newest = MakeTaskPtr(table, 41); + auto old_after = MakeTaskPtr(table, 40); + auto same_term = MakeTaskPtr(table, 41); + auto old_only_partition = MakeTaskPtr(table, 40); + auto other_node_group = MakeTaskPtr(table, 7, /*node_group_id=*/2); + + std::unordered_map>> + flush_task; + auto &entries = flush_task["irange_range_terms"]; + auto add_record = [&](std::shared_ptr task, + int key, + int32_t partition_id = 5) + { + auto records = std::make_unique>(); + records->push_back(MakeSerializedRecord(key, + "value", + RecordStatus::Normal, + /*commit_ts=*/200 + key, + partition_id)); + entries.push_back( + MakeFlushEntry(std::move(task), std::move(records))); + }; + add_record(old_before, 10); + add_record(newest, 11); + add_record(old_after, 12); + add_record(same_term, 13); + add_record(old_only_partition, 14, /*partition_id=*/6); + add_record(other_node_group, 15, /*partition_id=*/7); + + REQUIRE(fixture.Client().PutAll(flush_task)); + REQUIRE(fixture.Service().GetApproxStoreKeyCount(/*shard_id=*/0) == 3); + uint64_t record_ts = 0; + REQUIRE( + ReadKey( + fixture.Service(), "irange_range_terms", 5, "10", record_ts) == + EloqDS::remote::DataStoreError::KEY_NOT_FOUND); + REQUIRE( + ReadKey( + fixture.Service(), "irange_range_terms", 5, "11", record_ts) == + EloqDS::remote::DataStoreError::NO_ERROR); + REQUIRE(record_ts == 211); + REQUIRE( + ReadKey( + fixture.Service(), "irange_range_terms", 5, "12", record_ts) == + EloqDS::remote::DataStoreError::KEY_NOT_FOUND); + REQUIRE( + ReadKey( + fixture.Service(), "irange_range_terms", 5, "13", record_ts) == + EloqDS::remote::DataStoreError::NO_ERROR); + REQUIRE(record_ts == 213); + REQUIRE( + ReadKey( + fixture.Service(), "irange_range_terms", 6, "14", record_ts) == + EloqDS::remote::DataStoreError::KEY_NOT_FOUND); + REQUIRE( + ReadKey( + fixture.Service(), "irange_range_terms", 7, "15", record_ts) == + EloqDS::remote::DataStoreError::NO_ERROR); + REQUIRE(record_ts == 215); + } + + SECTION("newest task may have no base records") + { + const TableName old_table{std::string_view("old_nonempty"), + TableType::Primary, + TableEngine::EloqKv}; + auto old_records = std::make_unique>(); + old_records->push_back(MakeObjectRecord(/*key=*/80, + "obsolete", + RecordStatus::Normal, + /*commit_ts=*/480, + /*ttl=*/UINT64_MAX, + /*partition_id=*/0)); + + const TableName newest_table{std::string_view("newest_empty"), + TableType::Primary, + TableEngine::EloqKv}; + std::unordered_map>> + flush_task; + flush_task["eloqkv_old_nonempty"].push_back(MakeFlushEntry( + MakeTaskPtr(old_table, /*term=*/80), std::move(old_records))); + flush_task["eloqkv_newest_empty"].push_back( + MakeFlushEntry(MakeTaskPtr(newest_table, /*term=*/81), + std::make_unique>())); + + REQUIRE(fixture.Client().PutAll(flush_task)); + REQUIRE(fixture.Service().GetApproxStoreKeyCount(/*shard_id=*/0) == 0); + uint64_t record_ts = 0; + REQUIRE(ReadKey(fixture.Service(), + "eloqkv_old_nonempty", + /*partition_id=*/0, + "80", + record_ts) == + EloqDS::remote::DataStoreError::KEY_NOT_FOUND); + } +} + +TEST_CASE("PutAll handles empty entry slots and object deletes", + "[checkpoint-flush][put-all]") +{ + PutAllFixture fixture; + const TableName table{ + std::string_view("objects"), TableType::Primary, TableEngine::EloqKv}; + const std::string kv_table_name = "eloqkv_objects"; + auto empty_task = MakeTaskPtr(table, 21); + auto data_task = MakeTaskPtr(table, 21); + + std::unordered_map>> + flush_task; + flush_task.try_emplace("empty_table"); + auto &entries = flush_task[kv_table_name]; + entries.push_back(MakeFlushEntry( + empty_task, std::make_unique>())); + + auto records = std::make_unique>(); + records->push_back(MakeObjectRecord(1, + "live", + RecordStatus::Normal, + /*commit_ts=*/101, + /*ttl=*/UINT64_MAX, + /*partition_id=*/0)); + records->push_back(MakeObjectRecord(2, + "expired", + RecordStatus::Normal, + /*commit_ts=*/102, + /*ttl=*/0, + /*partition_id=*/0)); + records->push_back(MakeObjectRecord(3, + "", + RecordStatus::Deleted, + /*commit_ts=*/103, + /*ttl=*/0, + /*partition_id=*/0)); + entries.push_back(MakeFlushEntry(data_task, std::move(records))); + + REQUIRE(fixture.Client().PutAll(flush_task)); + REQUIRE(fixture.Service().GetApproxStoreKeyCount(/*shard_id=*/0) == 1); + + const TableName internal_table{std::string_view("internal"), + TableType::Primary, + TableEngine::InternalHash}; + auto internal_task = MakeTaskPtr(internal_table, 22); + auto internal_records = std::make_unique>(); + internal_records->push_back(MakeSerializedRecord(10, + "serialized", + RecordStatus::Normal, + /*commit_ts=*/104, + /*partition_id=*/1)); + // Non-object hash tables retain tombstones as encoded PUTs with a retired + // TTL, rather than issuing a physical DELETE. + internal_records->push_back(MakeSerializedRecord(11, + "", + RecordStatus::Deleted, + /*commit_ts=*/105, + /*partition_id=*/1)); + std::unordered_map>> + internal_flush; + internal_flush["internal"].push_back( + MakeFlushEntry(internal_task, std::move(internal_records))); + + REQUIRE(fixture.Client().PutAll(internal_flush)); + REQUIRE(fixture.Service().GetApproxStoreKeyCount(/*shard_id=*/0) == 3); +} + +TEST_CASE("PutAll chains partition batches past the 64 MiB boundary", + "[checkpoint-flush][put-all][batch-rollover]") +{ + PutAllFixture fixture; + constexpr size_t kBatchBoundary = 64ULL * 1024 * 1024; + + SECTION("hash partition") + { + const TableName table{std::string_view("hash_batch_rollover"), + TableType::Primary, + TableEngine::EloqKv}; + auto records = std::make_unique>(); + records->push_back(MakeObjectRecord(/*key=*/100, + std::string(kBatchBoundary, 'h'), + RecordStatus::Normal, + /*commit_ts=*/600, + /*ttl=*/UINT64_MAX, + /*partition_id=*/0)); + // Batch selection happens before serializing the current record. This + // second record therefore closes the oversized first batch and must be + // sent by the callback chain as a separate request. + records->push_back(MakeObjectRecord(/*key=*/101, + "tail", + RecordStatus::Normal, + /*commit_ts=*/601, + /*ttl=*/UINT64_MAX, + /*partition_id=*/0)); + + std::unordered_map>> + flush_task; + flush_task["eloqkv_hash_batch_rollover"].push_back(MakeFlushEntry( + MakeTaskPtr(table, /*term=*/90), std::move(records))); + + REQUIRE(fixture.Client().PutAll(flush_task)); + uint64_t record_ts = 0; + REQUIRE(ReadKey(fixture.Service(), + "eloqkv_hash_batch_rollover", + /*partition_id=*/0, + "100", + record_ts) == EloqDS::remote::DataStoreError::NO_ERROR); + REQUIRE(record_ts == 600); + REQUIRE(ReadKey(fixture.Service(), + "eloqkv_hash_batch_rollover", + /*partition_id=*/0, + "101", + record_ts) == EloqDS::remote::DataStoreError::NO_ERROR); + REQUIRE(record_ts == 601); + } + + SECTION("range partition") + { + const TableName table{std::string_view("range_batch_rollover"), + TableType::Primary, + TableEngine::InternalRange}; + auto records = std::make_unique>(); + records->push_back( + MakeSerializedRecord(/*key=*/110, + std::string(kBatchBoundary, 'r'), + RecordStatus::Normal, + /*commit_ts=*/610, + /*partition_id=*/9)); + records->push_back(MakeSerializedRecord(/*key=*/111, + "tail", + RecordStatus::Normal, + /*commit_ts=*/611, + /*partition_id=*/9)); + + std::unordered_map>> + flush_task; + flush_task["irange_range_batch_rollover"].push_back(MakeFlushEntry( + MakeTaskPtr(table, /*term=*/91), std::move(records))); + + REQUIRE(fixture.Client().PutAll(flush_task)); + uint64_t record_ts = 0; + REQUIRE(ReadKey(fixture.Service(), + "irange_range_batch_rollover", + /*partition_id=*/9, + "110", + record_ts) == EloqDS::remote::DataStoreError::NO_ERROR); + REQUIRE(record_ts == 610); + REQUIRE(ReadKey(fixture.Service(), + "irange_range_batch_rollover", + /*partition_id=*/9, + "111", + record_ts) == EloqDS::remote::DataStoreError::NO_ERROR); + REQUIRE(record_ts == 611); + } +} + +TEST_CASE("archive flush discards every lower-term task before datastore IO", + "[checkpoint-flush][partition-term][archive]") +{ + PutAllFixture fixture; + const TableName table{std::string_view("archive_terms"), + TableType::Primary, + TableEngine::InternalHash}; + + std::unordered_map>> + flush_task; + flush_task.try_emplace("empty_archive_bucket"); + auto &entries = flush_task["ihash_archive_terms"]; + auto add_record = [&](int64_t term, + NodeGroupId node_group_id, + int key, + int32_t partition_id) + { + auto records = std::make_unique>(); + records->push_back(MakeSerializedRecord(key, + "archive-value", + RecordStatus::Normal, + /*commit_ts=*/300 + key, + partition_id)); + entries.push_back(MakeArchiveEntry( + MakeTaskPtr(table, term, node_group_id), std::move(records))); + }; + + add_record(/*term=*/50, /*node_group_id=*/1, /*key=*/20, /*pid=*/0); + add_record(/*term=*/51, /*node_group_id=*/1, /*key=*/21, /*pid=*/0); + add_record(/*term=*/50, /*node_group_id=*/1, /*key=*/22, /*pid=*/0); + add_record(/*term=*/51, /*node_group_id=*/1, /*key=*/23, /*pid=*/0); + // An older-only source partition is still obsolete because selection is + // across the whole node-group batch, not per source or archive partition. + add_record(/*term=*/50, /*node_group_id=*/1, /*key=*/24, /*pid=*/1); + // Terms are scoped to a node group, so this lower numeric term is valid. + add_record(/*term=*/8, /*node_group_id=*/2, /*key=*/25, /*pid=*/2); + + const TableName stale_other_table{ + std::string_view("archive_terms_stale_table"), + TableType::Primary, + TableEngine::InternalHash}; + auto stale_other_records = std::make_unique>(); + stale_other_records->push_back(MakeSerializedRecord(26, + "stale-archive-value", + RecordStatus::Normal, + /*commit_ts=*/326, + /*partition_id=*/3)); + flush_task["ihash_archive_terms_stale_table"].push_back( + MakeArchiveEntry(MakeTaskPtr(stale_other_table, /*term=*/50), + std::move(stale_other_records))); + + REQUIRE(fixture.Client().PutArchivesAll(flush_task)); + REQUIRE(fixture.Service().GetApproxStoreKeyCount(/*shard_id=*/0) == 3); +} + +TEST_CASE("base-to-archive copy does not read lower-term task keys", + "[checkpoint-flush][partition-term][archive]") +{ + PutAllFixture fixture; + const TableName table{std::string_view("copy_terms"), + TableType::Primary, + TableEngine::InternalHash}; + + auto old_bases = std::make_unique>>(); + old_bases->emplace_back(TxKey(std::make_unique("obsolete")), + 0); + + std::unordered_map>> + flush_task; + flush_task.try_emplace("empty_copy_bucket"); + flush_task["ihash_copy_terms"].push_back( + MakeMvBaseEntry(MakeTaskPtr(table, /*term=*/60), std::move(old_bases))); + const TableName newer_table{std::string_view("copy_terms_newer_table"), + TableType::Primary, + TableEngine::InternalHash}; + flush_task["ihash_copy_terms_newer_table"].push_back(MakeMvBaseEntry( + MakeTaskPtr(newer_table, /*term=*/61), + std::make_unique>>())); + + REQUIRE(fixture.Client().CopyBaseToArchive(flush_task)); + REQUIRE(fixture.Service().GetApproxStoreKeyCount(/*shard_id=*/0) == 0); +} + +TEST_CASE("base-to-archive copy retains only the newest task metadata", + "[checkpoint-flush][partition-term][archive]") +{ + PutAllFixture fixture; + const TableName table{std::string_view("copy_retained"), + TableType::Primary, + TableEngine::EloqSql}; + const std::string_view kv_table_name = "eloqsql_copy_retained"; + + auto seed_records = std::make_unique>(); + seed_records->push_back(MakeSerializedRecord(/*key=*/70, + "base-version", + RecordStatus::Normal, + /*commit_ts=*/370, + /*partition_id=*/4)); + std::unordered_map>> + seed; + seed[kv_table_name].push_back(MakeFlushEntry( + MakeTaskPtr(table, /*term=*/70), std::move(seed_records))); + REQUIRE(fixture.Client().PutAll(seed)); + REQUIRE(fixture.Service().GetApproxStoreKeyCount(/*shard_id=*/0) == 1); + + auto obsolete_bases = + std::make_unique>>(); + obsolete_bases->emplace_back( + TxKey(std::make_unique("must-not-be-read")), 4); + auto newest_bases = + std::make_unique>>(); + newest_bases->emplace_back(TxKey(std::make_unique("70")), 4); + + auto old_task = MakeTaskPtr(table, /*term=*/69); + auto newest_task = MakeTaskPtr(table, /*term=*/70); + std::unordered_map>> + flush_task; + auto &entries = flush_task[kv_table_name]; + entries.push_back(MakeMvBaseEntry(old_task, std::move(obsolete_bases))); + entries.push_back(MakeMvBaseEntry(newest_task, std::move(newest_bases))); + + REQUIRE(fixture.Client().CopyBaseToArchive(flush_task)); + // The base record remains and exactly one newest-term archive version is + // added. Reading the obsolete key would synthesize a second tombstone. + REQUIRE(fixture.Service().GetApproxStoreKeyCount(/*shard_id=*/0) == 2); +} + +TEST_CASE("live shard dispatch covers checkpoint and intrusive wait lists", + "[checkpoint-flush][partition-dispatch][cc-wait-list]") +{ + Watchdog watchdog(20s); + txservice::test::TestNode node( + txservice::test::TestNodeOptions{}.CoreNum(4).EnableMvcc(false)); + + // On EloqStore builds this exercises PutAll's live hash/range collection + // path. The deliberately stale node group makes UpdateCceCkptTsCc reject + // the request before dereferencing the sentinel CCE addresses. On + // RocksDB-backed builds publication is deferred and the same input checks + // that PutAll does not retain or touch those addresses. + { + PutAllFixture fixture; + auto sentinel_cce = reinterpret_cast(uintptr_t{1}); + const TableName hash_table{std::string_view("collect_hash"), + TableType::Primary, + TableEngine::EloqKv}; + auto hash_records = std::make_unique>(); + hash_records->push_back(MakeObjectRecord(/*key=*/90, + "hash", + RecordStatus::Normal, + /*commit_ts=*/590, + /*ttl=*/UINT64_MAX, + /*partition_id=*/0)); + hash_records->back().cce_ = sentinel_cce; + hash_records->back().post_flush_size_ = 11; + + const TableName range_table{std::string_view("collect_range"), + TableType::Primary, + TableEngine::InternalRange}; + auto range_records = std::make_unique>(); + range_records->push_back(MakeSerializedRecord(/*key=*/91, + "range", + RecordStatus::Normal, + /*commit_ts=*/591, + /*partition_id=*/1)); + range_records->back().cce_ = sentinel_cce; + range_records->back().post_flush_size_ = 12; + + std::unordered_map>> + flush_task; + flush_task["eloqkv_collect_hash"].push_back(MakeFlushEntry( + MakeTaskPtr(hash_table, /*term=*/999), std::move(hash_records))); + flush_task["irange_collect_range"].push_back(MakeFlushEntry( + MakeTaskPtr(range_table, /*term=*/999), std::move(range_records))); + const std::function report_progress = + [](uint64_t, uint64_t) {}; + + REQUIRE(fixture.Client().PutAll(flush_task, + /*yield_fptr=*/nullptr, + /*resume_fptr=*/nullptr, + /*sync_yield_fptr=*/nullptr, + &report_progress)); + } + + const TableName table{ + std::string_view("dispatch"), TableType::Primary, TableEngine::EloqKv}; + DataSyncTask stale_task = MakeTask(table, /*term=*/1); + + EloqDS::PartitionFlushState partition; + partition.Reset(/*pid=*/0, /*is_range_partitioned=*/false); + partition.serialized_bytes_ = 31; + partition.AddCkptTsEntry( + &stale_task, /*core_idx=*/0, nullptr, /*commit_ts=*/10, 0); + partition.AddCkptTsEntry( + &stale_task, /*core_idx=*/1, nullptr, /*commit_ts=*/11, 0); + + EloqDS::SyncPutAllData sync; + sync.Reset(); + sync.total_partitions_ = 1; + sync.total_bytes_ = 31; + partition.ArmCkptTsUpdate(&sync); + + EloqDS::PartitionCallbackData callback; + callback.Reset(&partition, &sync, "eloqkv_dispatch"); + EloqDS::remote::CommonResult result; + result.set_error_code(EloqDS::remote::DataStoreError::NO_ERROR); + + txservice::CatalogFactory *catalog_factories[3]{nullptr, nullptr, nullptr}; + EloqDS::DataStoreServiceClusterManager cluster_manager; + EloqDS::DataStoreServiceClient client(/*is_bootstrap=*/false, + catalog_factories, + cluster_manager, + /*bind_data_shard_with_ng=*/false); + EloqDS::PartitionBatchCallback(&callback, nullptr, client, result); + + sync.Wait(); + REQUIRE(partition.ckpt_ts_update_->IsFinished()); + REQUIRE(sync.completed_partitions_ == 1); + REQUIRE(sync.completed_bytes_ == 31); + + auto *shards = Sharder::Instance().GetLocalCcShards(); + REQUIRE(shards != nullptr); + CcShard *shard = shards->GetCcShard(0); + REQUIRE(shard != nullptr); + + ShardRequest abort_first(/*abort_if_oom=*/true); + ShardRequest retained; + ShardRequest abort_last(/*abort_if_oom=*/true); + abort_first.Use(); + retained.Use(); + abort_last.Use(); + shard->EnqueueWaitListIfMemoryFull(&abort_first); + shard->EnqueueWaitListIfMemoryFull(&retained); + shard->EnqueueWaitListIfMemoryFull(&abort_last); + REQUIRE(shard->WaitListSizeForMemory() == 3); + + shard->AbortRequestsAfterMemoryFree(); + REQUIRE(abort_first.AbortError() == CcErrorCode::OUT_OF_MEMORY); + REQUIRE(abort_last.AbortError() == CcErrorCode::OUT_OF_MEMORY); + REQUIRE(shard->WaitListSizeForMemory() == 1); + REQUIRE(shard->DequeueWaitListAfterMemoryFree(/*deque_all=*/true)); + + ShardRequest schema_waiter; + schema_waiter.Use(); + shard->EnqueueWaitListIfSchemaMismatch(&schema_waiter); + shard->DequeueWaitListAfterSchemaUpdated(); + + const auto deadline = std::chrono::steady_clock::now() + 5s; + while ( + (retained.ExecuteCount() != 1 || schema_waiter.ExecuteCount() != 1) && + std::chrono::steady_clock::now() < deadline) + { + std::this_thread::sleep_for(1ms); + } + REQUIRE(retained.ExecuteCount() == 1); + REQUIRE(schema_waiter.ExecuteCount() == 1); + REQUIRE_FALSE(retained.InUse()); + REQUIRE_FALSE(schema_waiter.InUse()); + + // ApplyCc used to bypass the base Free() state transition for remote + // requests, leaving a recycled request permanently marked in use. + ApplyCc remote_apply(/*is_local=*/false); + remote_apply.Use(); + remote_apply.Free(); + REQUIRE_FALSE(remote_apply.InUse()); +} diff --git a/tx_service/tests/FetchRecordCc-Test.cpp b/tx_service/tests/FetchRecordCc-Test.cpp index 78efdc50a..c8e50273a 100644 --- a/tx_service/tests/FetchRecordCc-Test.cpp +++ b/tx_service/tests/FetchRecordCc-Test.cpp @@ -18,6 +18,7 @@ * and GNU General Public License V2 along with this program. If not, see * . */ +#include #include #include #include @@ -29,14 +30,22 @@ #include #include +// This test verifies the ownership boundary between CcShard's active-fetch +// index and its FetchRecordCc pool. +#define protected public +#define private public +#include "cc/cc_entry.h" #include "cc/cc_req_misc.h" #include "cc/cc_req_pool.h" #include "cc/cc_shard.h" #include "cc/local_cc_shards.h" +#include "cc/template_cc_map.h" #include "data_store_service_client.h" #include "include/mock/mock_catalog_factory.h" #include "sharder.h" #include "tx_key.h" +#undef private +#undef protected namespace txservice { @@ -415,3 +424,556 @@ TEST_CASE( } } } // namespace txservice + +namespace txservice +{ +namespace +{ +using TestKey = CompositeKey; +using TestRecord = CompositeRecord; +using TestCcMap = TemplateCcMap; +using TestCcEntry = CcEntry; + +MockCatalogFactory mock_catalog_factory; + +struct FetchRecordFixture +{ + std::unordered_map> ng_configs{ + {0, {NodeConfig(0, "127.0.0.1", 8600)}}}; + std::map tx_cnf{ + {"node_memory_limit_mb", 1000}, + {"enable_key_cache", 0}, + {"reltime_sampling", 0}, + {"range_split_worker_num", 1}, + {"range_slice_memory_limit_percent", 20}, + {"core_num", 1}, + {"realtime_sampling", 0}, + {"checkpointer_interval", 10}, + {"checkpointer_delay_seconds", 0}, + {"checkpointer_min_ckpt_request_interval", 5}, + {"enable_shard_heap_defragment", 0}, + {"node_log_limit_mb", 1000}, + {"collect_active_tx_ts_interval_seconds", 2}, + {"rep_group_cnt", 1}, + }; + CatalogFactory *catalog_factory[5] = { + &mock_catalog_factory, + &mock_catalog_factory, + &mock_catalog_factory, + &mock_catalog_factory, + &mock_catalog_factory, + }; + LocalCcShards local_cc_shards; + CcShard shard; + std::string raft_path; + + explicit FetchRecordFixture(store::DataStoreHandler *store_hd = nullptr) + : local_cc_shards(0, + 0, + tx_cnf, + catalog_factory, + nullptr, + &ng_configs, + 2, + store_hd, + nullptr, + true), + shard(0, + 1, + 10000, + false, + 0, + local_cc_shards, + catalog_factory, + nullptr, + &ng_configs, + 2) + { + local_cc_shards.BindThreadToFastMetaDataShard(0); + shard.Init(); + Sharder::Instance(0, + &ng_configs, + 0, + nullptr, + nullptr, + &local_cc_shards, + nullptr, + &raft_path); + // Sharder::Instance's legacy arguments are ignored; lightweight shard + // tests wire the state they exercise instead of running + // Sharder::Init(). + Sharder &sharder = Sharder::Instance(); + sharder.node_id_ = 0; + sharder.native_ng_ = 0; + sharder.local_shards_ = &local_cc_shards; + sharder.ng_leader_cache_[0].store(0, std::memory_order_relaxed); + sharder.ng_leader_term_cache_[0].store(-1, std::memory_order_relaxed); + sharder.leader_term_cache_[0].store(-1, std::memory_order_relaxed); + sharder.candidate_leader_term_cache_[0].store( + -1, std::memory_order_relaxed); + sharder.standby_node_term_cache_.store(-1, std::memory_order_relaxed); + sharder.candidate_standby_node_term_cache_.store( + -1, std::memory_order_relaxed); + } + + ~FetchRecordFixture() + { + local_cc_shards.Terminate(); + Sharder::Instance().local_shards_ = nullptr; + } +}; + +class ReentrantRequester : public CcRequestBase +{ +public: + ReentrantRequester(LruEntry *cce, + FetchRecordCc *finishing_fetch, + const size_t *destroyed_fetches) + : cce_(cce), + finishing_fetch_(finishing_fetch), + destroyed_fetches_(destroyed_fetches) + { + } + + bool Execute(CcShard &ccs) override + { + ++execute_count_; + active_fetch_removed_ = !ccs.fetch_record_reqs_.contains(cce_); + finishing_fetch_alive_ = *destroyed_fetches_ == 0; + + // Force the pool scan to start at the finishing request. It must skip + // that object while the completion still owns it. + ccs.fetch_record_cc_pool_.head_ = 0; + FetchRecordCc *nested_fetch = ccs.fetch_record_cc_pool_.NextRequest(); + finishing_fetch_not_reused_ = nested_fetch != finishing_fetch_; + nested_fetch->Free(); + + // Mirror a requester resumed after FetchRecord: CcShard::FetchRecord + // added this pin on its behalf. + cce_->GetKeyGapLockAndExtraData()->ReleasePin(); + cce_->RecycleKeyLock(ccs); + return true; + } + + int execute_count_{0}; + bool active_fetch_removed_{false}; + bool finishing_fetch_not_reused_{false}; + bool finishing_fetch_alive_{false}; + +private: + LruEntry *cce_; + FetchRecordCc *finishing_fetch_; + const size_t *destroyed_fetches_; +}; + +class AbortTrackingRequester : public CcRequestBase +{ +public: + bool Execute(CcShard &) override + { + ++execute_count_; + return true; + } + + void AbortCcRequest(CcErrorCode error) override + { + abort_error_ = error; + Free(); + } + + int execute_count_{0}; + CcErrorCode abort_error_{CcErrorCode::NO_ERROR}; +}; + +class StubStoreHandler : public EloqDS::DataStoreServiceClient +{ +public: + StubStoreHandler( + CatalogFactory *catalog_factories[3], + const EloqDS::DataStoreServiceClusterManager &cluster_manager, + store::DataStoreHandler::DataStoreOpStatus result) + : DataStoreServiceClient( + false, catalog_factories, cluster_manager, false), + result_(result) + { + } + + store::DataStoreHandler::DataStoreOpStatus FetchRecord( + FetchRecordCc *fetch_record_cc, FetchSnapshotCc *) override + { + ++fetch_count_; + last_fetch_ = fetch_record_cc; + return result_; + } + + store::DataStoreHandler::DataStoreOpStatus result_; + size_t fetch_count_{0}; + FetchRecordCc *last_fetch_{nullptr}; +}; + +class QueuedRequester : public CcRequestBase +{ +public: + explicit QueuedRequester(LruEntry *cce) : cce_(cce) + { + } + + bool Execute(CcShard &ccs) override + { + ++execute_count_; + active_fetch_present_ = ccs.fetch_record_reqs_.contains(cce_); + cce_->GetKeyGapLockAndExtraData()->ReleasePin(); + cce_->RecycleKeyLock(ccs); + return true; + } + + size_t execute_count_{0}; + bool active_fetch_present_{false}; + +private: + LruEntry *cce_; +}; + +int64_t CurrentTerm() +{ + return std::max({Sharder::Instance().CandidateLeaderTerm(0), + Sharder::Instance().LeaderTerm(0), + Sharder::Instance().StandbyNodeTerm()}); +} +} // namespace + +TEST_CASE("FetchRecordCc inline completion retains pooled and temporary owners", + "[fetch-record][pool]") +{ + const bool temporary = GENERATE(false, true); + size_t destroyed_fetches = 0; + FetchRecordFixture fixture; + const TableName table{std::string_view("fetch_record_pool"), + TableType::Primary, + TableEngine::EloqSql}; + TestCcMap cc_map(&fixture.shard, 0, table, 1, nullptr, true); + + TestKey key = std::make_tuple(std::string("key"), 1); + bool emplaced = false; + auto it = cc_map.FindEmplace(key, &emplaced, false, false); + REQUIRE(emplaced); + auto *cce = static_cast(it->second); + REQUIRE(cce != nullptr); + + cce->GetOrCreateKeyLock(&fixture.shard, &cc_map, it.GetPage()); + KeyGapLockAndExtraData *lock = cce->GetKeyGapLockAndExtraData(); + REQUIRE(lock != nullptr); + // BackFill releases the pin held by the active datastore operation. + lock->AddPin(); + // The resumed requester owns a second pin until its Execute call. + lock->AddPin(); + + FetchRecordCc *fetch = + temporary ? new ObservedFetchRecordCc(&destroyed_fetches) + : fixture.shard.fetch_record_cc_pool_.NextRequest(); + if (temporary) + { + fetch->Use(); + } + REQUIRE(fetch->InUse()); + fetch->ccs_ = &fixture.shard; + fetch->cc_ng_id_ = 0; + fetch->cc_ng_term_ = CurrentTerm(); + fetch->cce_ = cce; + fetch->lock_ = lock; + fetch->rec_ts_ = 2; + fetch->rec_status_ = RecordStatus::Deleted; + fetch->error_code_ = 0; + fetch->only_fetch_archives_ = false; + + ReentrantRequester requester(cce, fetch, &destroyed_fetches); + requester.Use(); + fetch->AddRequester(&requester); + fixture.shard.fetch_record_reqs_.try_emplace( + cce, FetchRecordCc::uptr(fetch, FetchRecordCc::Deleter{!temporary})); + + fixture.shard.Enqueue(fetch); + const size_t processed = fixture.shard.ProcessRequests(); + + REQUIRE(processed == 1); + REQUIRE(requester.execute_count_ == 1); + REQUIRE(requester.active_fetch_removed_); + REQUIRE(requester.finishing_fetch_not_reused_); + REQUIRE(requester.finishing_fetch_alive_); + REQUIRE_FALSE(requester.InUse()); + REQUIRE_FALSE(fixture.shard.fetch_record_reqs_.contains(cce)); + // The completion owner releases the request after inline requesters finish. + if (temporary) + { + REQUIRE(destroyed_fetches == 1); + } + else + { + REQUIRE_FALSE(fetch->InUse()); + } +} + +TEST_CASE("FetchRecordCc term-change completion releases its active owner", + "[fetch-record][pool]") +{ + FetchRecordFixture fixture; + const TableName table{std::string_view("fetch_record_term_change"), + TableType::Primary, + TableEngine::EloqSql}; + TestCcMap cc_map(&fixture.shard, 0, table, 1, nullptr, true); + + TestKey key = std::make_tuple(std::string("key"), 1); + bool emplaced = false; + auto it = cc_map.FindEmplace(key, &emplaced, false, false); + REQUIRE(emplaced); + auto *cce = static_cast(it->second); + REQUIRE(cce != nullptr); + + FetchRecordCc *fetch = fixture.shard.fetch_record_cc_pool_.NextRequest(); + fetch->ccs_ = &fixture.shard; + fetch->cc_ng_id_ = 0; + fetch->cc_ng_term_ = CurrentTerm() + 1; + fetch->cce_ = cce; + + AbortTrackingRequester requester; + requester.Use(); + fetch->AddRequester(&requester); + fixture.shard.fetch_record_reqs_.try_emplace( + cce, FetchRecordCc::uptr(fetch, FetchRecordCc::Deleter{true})); + + fixture.shard.Enqueue(fetch); + REQUIRE(fixture.shard.ProcessRequests() == 1); + + REQUIRE(requester.abort_error_ == CcErrorCode::NG_TERM_CHANGED); + REQUIRE_FALSE(requester.InUse()); + REQUIRE_FALSE(fixture.shard.fetch_record_reqs_.contains(cce)); + REQUIRE_FALSE(fetch->InUse()); +} + +TEST_CASE("FetchRecordCc abandoned before enqueue is freed explicitly", + "[fetch-record][pool]") +{ + FetchRecordFixture fixture; + const TableName table{std::string_view("fetch_record_start_failure"), + TableType::Primary, + TableEngine::EloqSql}; + MockTableSchema schema(table, "", 1); + TestCcMap cc_map(&fixture.shard, 0, table, 1, &schema, true); + + TestKey key = std::make_tuple(std::string("key"), 1); + bool emplaced = false; + auto it = cc_map.FindEmplace(key, &emplaced, false, false); + REQUIRE(emplaced); + auto *cce = static_cast(it->second); + REQUIRE(cce != nullptr); + cce->GetOrCreateKeyLock(&fixture.shard, &cc_map, it.GetPage()); + + fixture.shard.fetch_record_cc_pool_.head_ = 0; + FetchRecordCc *first_pool_entry = + fixture.shard.fetch_record_cc_pool_.pool_.front().get(); + AbortTrackingRequester requester; + requester.Use(); + + const auto result = + fixture.shard.FetchRecord(table, + &schema, + TxKey(std::make_unique(key)), + cce, + 0, + CurrentTerm(), + &requester, + 0, + true); + + REQUIRE(result == store::DataStoreHandler::DataStoreOpStatus::Retry); + REQUIRE_FALSE(fixture.shard.fetch_record_reqs_.contains(cce)); + REQUIRE_FALSE(first_pool_entry->InUse()); + // The failed start happened before FetchRecord took a requester pin or + // transferred requester ownership to the shard queue. + REQUIRE(requester.InUse()); + requester.Free(); +} + +TEST_CASE("FetchRecordCc datastore start retry frees the pooled request", + "[fetch-record][pool]") +{ + CatalogFactory *catalog_factories[3] = { + &mock_catalog_factory, &mock_catalog_factory, &mock_catalog_factory}; + EloqDS::DataStoreServiceClusterManager cluster_manager; + StubStoreHandler store_handler( + catalog_factories, + cluster_manager, + store::DataStoreHandler::DataStoreOpStatus::Retry); + FetchRecordFixture fixture(&store_handler); + const TableName table{std::string_view("fetch_record_store_retry"), + TableType::Primary, + TableEngine::EloqSql}; + MockTableSchema schema(table, "", 1); + TestCcMap cc_map(&fixture.shard, 0, table, 1, &schema, true); + + TestKey key = std::make_tuple(std::string("key"), 1); + bool emplaced = false; + auto it = cc_map.FindEmplace(key, &emplaced, false, false); + REQUIRE(emplaced); + auto *cce = static_cast(it->second); + REQUIRE(cce != nullptr); + cce->GetOrCreateKeyLock(&fixture.shard, &cc_map, it.GetPage()); + + fixture.shard.fetch_record_cc_pool_.head_ = 0; + FetchRecordCc *first_pool_entry = + fixture.shard.fetch_record_cc_pool_.pool_.front().get(); + AbortTrackingRequester requester; + requester.Use(); + + const auto result = + fixture.shard.FetchRecord(table, + &schema, + TxKey(std::make_unique(key)), + cce, + 0, + CurrentTerm(), + &requester, + 0); + + REQUIRE(result == store::DataStoreHandler::DataStoreOpStatus::Retry); + REQUIRE_FALSE(fixture.shard.fetch_record_reqs_.contains(cce)); + REQUIRE_FALSE(first_pool_entry->InUse()); + REQUIRE(requester.InUse()); + requester.Free(); +} + +#ifdef DATA_STORE_TYPE_ELOQDSS_ELOQSTORE +TEST_CASE("FetchRecordCc keeps ownership while EloqStore reopen is in flight", + "[fetch-record][pool][eloqstore]") +{ + CatalogFactory *catalog_factories[3] = { + &mock_catalog_factory, &mock_catalog_factory, &mock_catalog_factory}; + EloqDS::DataStoreServiceClusterManager cluster_manager; + StubStoreHandler store_handler( + catalog_factories, + cluster_manager, + store::DataStoreHandler::DataStoreOpStatus::Success); + FetchRecordFixture fixture(&store_handler); + const TableName table{std::string_view("fetch_record_reopen_in_flight"), + TableType::Primary, + TableEngine::EloqSql}; + TestCcMap cc_map(&fixture.shard, 0, table, 1, nullptr, true); + + TestKey key = std::make_tuple(std::string("key"), 1); + bool emplaced = false; + auto it = cc_map.FindEmplace(key, &emplaced, false, false); + REQUIRE(emplaced); + auto *cce = static_cast(it->second); + REQUIRE(cce != nullptr); + + cce->GetOrCreateKeyLock(&fixture.shard, &cc_map, it.GetPage()); + KeyGapLockAndExtraData *lock = cce->GetKeyGapLockAndExtraData(); + REQUIRE(lock != nullptr); + lock->BufferedCommandList().txn_cmd_list_.emplace_back( + 1, 2, false, 0, std::vector>{}); + lock->AddPin(); + lock->AddPin(); + + FetchRecordCc *fetch = fixture.shard.fetch_record_cc_pool_.NextRequest(); + fetch->ccs_ = &fixture.shard; + fetch->cc_ng_id_ = 0; + fetch->cc_ng_term_ = CurrentTerm(); + fetch->cce_ = cce; + fetch->lock_ = lock; + fetch->rec_ts_ = 2; + fetch->rec_status_ = RecordStatus::Deleted; + fetch->error_code_ = 0; + fetch->only_fetch_archives_ = false; + + QueuedRequester requester(cce); + requester.Use(); + fetch->AddRequester(&requester); + fixture.shard.fetch_record_reqs_.try_emplace( + cce, FetchRecordCc::uptr(fetch, FetchRecordCc::Deleter{true})); + fixture.shard.Enqueue(fetch); + + REQUIRE(fixture.shard.ProcessRequests() == 1); + REQUIRE(fixture.shard.ProcessRequests() == 1); + REQUIRE(store_handler.fetch_count_ == 1); + REQUIRE(store_handler.last_fetch_ == fetch); + REQUIRE(requester.execute_count_ == 1); + REQUIRE(requester.active_fetch_present_); + REQUIRE_FALSE(requester.InUse()); + REQUIRE(fixture.shard.fetch_record_reqs_.at(cce).get() == fetch); + REQUIRE(fetch->InUse()); + + // Model EloqStore completing the reopened operation after it consumes the + // buffered command. This second completion has no reason to reopen again. + lock->BufferedCommandList().Clear(); + fetch->rec_ts_ = 3; + fetch->rec_status_ = RecordStatus::Deleted; + fetch->SetFinish(0); + REQUIRE(fixture.shard.ProcessRequests() == 1); + REQUIRE_FALSE(fixture.shard.fetch_record_reqs_.contains(cce)); + REQUIRE_FALSE(fetch->InUse()); +} + +TEST_CASE("FetchRecordCc reopen retry releases its active owner", + "[fetch-record][pool][eloqstore]") +{ + CatalogFactory *catalog_factories[3] = { + &mock_catalog_factory, &mock_catalog_factory, &mock_catalog_factory}; + EloqDS::DataStoreServiceClusterManager cluster_manager; + StubStoreHandler store_handler( + catalog_factories, + cluster_manager, + store::DataStoreHandler::DataStoreOpStatus::Retry); + FetchRecordFixture fixture(&store_handler); + const TableName table{std::string_view("fetch_record_reopen_retry"), + TableType::Primary, + TableEngine::EloqSql}; + TestCcMap cc_map(&fixture.shard, 0, table, 1, nullptr, true); + + TestKey key = std::make_tuple(std::string("key"), 1); + bool emplaced = false; + auto it = cc_map.FindEmplace(key, &emplaced, false, false); + REQUIRE(emplaced); + auto *cce = static_cast(it->second); + REQUIRE(cce != nullptr); + + cce->GetOrCreateKeyLock(&fixture.shard, &cc_map, it.GetPage()); + KeyGapLockAndExtraData *lock = cce->GetKeyGapLockAndExtraData(); + REQUIRE(lock != nullptr); + lock->BufferedCommandList().txn_cmd_list_.emplace_back( + 1, 2, false, 0, std::vector>{}); + lock->AddPin(); + lock->AddPin(); + + FetchRecordCc *fetch = fixture.shard.fetch_record_cc_pool_.NextRequest(); + fetch->ccs_ = &fixture.shard; + fetch->cc_ng_id_ = 0; + fetch->cc_ng_term_ = CurrentTerm(); + fetch->cce_ = cce; + fetch->lock_ = lock; + fetch->rec_ts_ = 2; + fetch->rec_status_ = RecordStatus::Deleted; + fetch->error_code_ = 0; + fetch->only_fetch_archives_ = false; + + QueuedRequester requester(cce); + requester.Use(); + fetch->AddRequester(&requester); + fixture.shard.fetch_record_reqs_.try_emplace( + cce, FetchRecordCc::uptr(fetch, FetchRecordCc::Deleter{true})); + fixture.shard.Enqueue(fetch); + + REQUIRE(fixture.shard.ProcessRequests() == 1); + REQUIRE(fixture.shard.ProcessRequests() == 1); + REQUIRE(store_handler.fetch_count_ == 1); + REQUIRE(requester.execute_count_ == 1); + REQUIRE_FALSE(requester.active_fetch_present_); + REQUIRE_FALSE(requester.InUse()); + REQUIRE_FALSE(fixture.shard.fetch_record_reqs_.contains(cce)); + REQUIRE_FALSE(fetch->InUse()); +} +#endif + +} // namespace txservice diff --git a/tx_service/tests/RealDataStore-Test.cpp b/tx_service/tests/RealDataStore-Test.cpp new file mode 100644 index 000000000..c50905304 --- /dev/null +++ b/tx_service/tests/RealDataStore-Test.cpp @@ -0,0 +1,519 @@ +/** + * Copyright (C) 2025 EloqData Inc. + * + * This program is free software: you can redistribute it and/or modify it + * under either GNU Affero General Public License v3 or GNU General Public + * License v2. + */ + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#if defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_S3) +#include +#endif + +#include "INIReader.h" +#include "data_store_service_client.h" +#include "data_sync_task.h" +#include "eloq_basic_catalog_factory.h" +#include "eloq_data_store_service/data_store_service.h" +#if defined(DATA_STORE_TYPE_ELOQDSS_ELOQSTORE) +#include "eloq_data_store_service/eloq_store_config.h" +#include "eloq_data_store_service/eloq_store_data_store_factory.h" +#elif defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_S3) +#include "eloq_data_store_service/rocksdb_cloud_data_store_factory.h" +#include "eloq_data_store_service/rocksdb_config.h" +#endif +#include "eloq_string_key_record.h" +#include "harness/port_util.h" + +using namespace txservice; + +namespace +{ +constexpr std::string_view kRunRealStoreEnv = "ELOQ_RUN_REAL_STORE_TEST"; +#if defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_S3) +constexpr std::string_view kObjectStoreUrlEnv = "ELOQ_TEST_OBJECT_STORE_URL"; + +std::string RequireEnvironment(std::string_view name) +{ + const char *value = std::getenv(std::string(name).c_str()); + if (value == nullptr || *value == '\0') + { + throw std::runtime_error("missing required environment variable " + + std::string(name)); + } + return value; +} +#endif + +std::shared_ptr MakeTask(const TableName &table_name, + int64_t term) +{ + return std::make_shared(table_name, + /*id=*/0, + /*range_version=*/0, + /*ng_id=*/1, + term, + /*data_sync_ts=*/100, + /*status=*/nullptr, + /*is_dirty=*/false, + /*need_adjust_ts=*/false, + /*hres=*/nullptr); +} + +std::unique_ptr MakeFlushEntry( + std::shared_ptr task, + std::unique_ptr> records) +{ + return std::make_unique( + std::move(records), + std::make_unique>(), + std::make_unique>>(), + /*data_sync_txm=*/nullptr, + std::move(task), + /*table_schema=*/nullptr, + /*size=*/0); +} + +FlushRecord MakeObjectRecord(int key, + RecordStatus status, + uint64_t commit_ts, + int32_t partition_id) +{ + std::shared_ptr payload; + if (status == RecordStatus::Normal) + { + auto record = std::make_shared(); + record->value_ = "value-" + std::to_string(key); + record->ttl_ = UINT64_MAX; + payload = std::move(record); + } + return FlushRecord( + TxKey(std::make_unique(std::to_string(key))), + std::move(payload), + status, + commit_ts, + /*cce=*/nullptr, + /*post_flush_size=*/0, + partition_id); +} + +FlushRecord MakeSerializedRecord(int key, + uint64_t commit_ts, + int32_t partition_id) +{ + auto record = std::make_shared(); + const std::string value = "value-" + std::to_string(key); + record->SetEncodedBlob( + reinterpret_cast(value.data()), value.size()); + return FlushRecord( + TxKey(std::make_unique(std::to_string(key))), + std::move(record), + RecordStatus::Normal, + commit_ts, + /*cce=*/nullptr, + /*post_flush_size=*/0, + partition_id); +} + +struct ReadCompletionState +{ + std::mutex mutex_; + std::condition_variable cv_; + bool done_{false}; +}; + +class ReadCompletionClosure final : public google::protobuf::Closure +{ +public: + explicit ReadCompletionClosure( + std::shared_ptr completion) + : completion_(std::move(completion)) + { + } + + void Run() override + { + // Protobuf callbacks may be self-deleting. Destroy the callback before + // waking the test thread so no callback member outlives the wait. + auto completion = std::move(completion_); + delete this; + { + std::lock_guard lock(completion->mutex_); + completion->done_ = true; + } + completion->cv_.notify_one(); + } + +private: + std::shared_ptr completion_; +}; + +EloqDS::remote::DataStoreError ReadKey(EloqDS::DataStoreService &service, + std::string_view table_name, + int32_t partition_id, + int key, + uint64_t &record_ts) +{ + std::string record; + uint64_t ttl = 0; + EloqDS::remote::CommonResult result; + const std::string key_string = std::to_string(key); + auto completion = std::make_shared(); + service.Read(table_name, + partition_id, + /*shard_id=*/0, + key_string, + /*reopen=*/false, + &record, + &record_ts, + &ttl, + &result, + new ReadCompletionClosure(completion)); + std::unique_lock lock(completion->mutex_); + completion->cv_.wait(lock, [&] { return completion->done_; }); + return static_cast(result.error_code()); +} + +class RealStoreFixture +{ +public: + RealStoreFixture() + { + GFLAGS_NAMESPACE::SetCommandLineOption("bthread_concurrency", "4"); + const auto unique = + std::to_string(::getpid()) + "-" + + std::to_string( + std::chrono::steady_clock::now().time_since_epoch().count()); + dir_ = std::filesystem::temp_directory_path() / + ("real-datastore-test-" + unique); + std::filesystem::create_directories(dir_); + config_path_ = dir_ / "store.ini"; + + std::ofstream config(config_path_); + if (!config) + { + throw std::runtime_error("failed to create real-store test config"); + } + config << "[store]\n"; +#if defined(DATA_STORE_TYPE_ELOQDSS_ELOQSTORE) + config << "eloq_store_worker_num=4\n" + << "eloq_store_init_page_count=1024\n" + << "eloq_store_root_meta_cache_size=16MB\n" + << "eloq_store_buffer_pool_size=64MB\n" + << "eloq_store_local_space_limit=1GB\n"; +#elif defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_S3) + config << "rocksdb_write_buffer_size=4MB\n" + << "rocksdb_target_file_size_base=4MB\n" + << "rocksdb_cloud_sst_file_cache_size=64MB\n" + << "rocksdb_cloud_sst_file_cache_num_shard_bits=0\n" + << "rocksdb_cloud_db_ready_timeout_sec=30\n" + << "rocksdb_cloud_db_file_deletion_delay_sec=0\n" + << "rocksdb_cloud_run_purger=false\n" + << "rocksdb_cloud_warm_up_thread_num=1\n"; +#endif + config.close(); + +#if defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_S3) + std::string base_url = RequireEnvironment(kObjectStoreUrlEnv); + while (!base_url.empty() && base_url.back() == '/') + { + base_url.pop_back(); + } + object_store_url_ = base_url + "/run-" + unique; + Aws::InitAPI(aws_options_); + aws_initialized_ = true; +#endif + Start(/*create_if_missing=*/true); + } + + ~RealStoreFixture() + { + Stop(); +#if defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_S3) + if (aws_initialized_) + { + Aws::ShutdownAPI(aws_options_); + } +#endif + std::error_code ec; + std::filesystem::remove_all(dir_, ec); + } + + EloqDS::DataStoreServiceClient &Client() + { + return *client_; + } + + EloqDS::DataStoreService &Service() + { + return *service_; + } + + void RestartWithoutVolatileCloudState() + { + Stop(); +#if defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_S3) + // Force the reopened DB to recover its durable state through S3Proxy + // instead of reusing the local RocksDB directory. + std::filesystem::remove_all(dir_ / "rocksdb_data"); +#endif + Start(/*create_if_missing=*/false); + } + +private: + std::unique_ptr MakeFactory() + { + INIReader config(config_path_.string()); + if (config.ParseError() < 0) + { + throw std::runtime_error("failed to parse real-store test config"); + } + +#if defined(DATA_STORE_TYPE_ELOQDSS_ELOQSTORE) + uint32_t node_memory_mb = 256; + EloqDS::EloqStoreConfig store_config(config, + dir_.string(), + node_memory_mb, + /*core_number=*/4, + /*standalone=*/true); + return std::make_unique( + std::move(store_config)); +#elif defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_S3) + EloqDS::RocksDBConfig rocksdb_config(config, dir_.string()); + EloqDS::RocksDBCloudConfig cloud_config(config); + cloud_config.aws_access_key_id_ = + RequireEnvironment("AWS_ACCESS_KEY_ID"); + cloud_config.aws_secret_key_ = + RequireEnvironment("AWS_SECRET_ACCESS_KEY"); + cloud_config.oss_url_ = object_store_url_; + cloud_config.region_ = "us-east-1"; + cloud_config.branch_name_ = "main"; + return std::make_unique( + rocksdb_config, + cloud_config, + /*tx_enable_cache_replacement=*/false); +#else + throw std::runtime_error("unsupported real-store test backend"); +#endif + } + + void Start(bool create_if_missing) + { + constexpr int kMaxBindRetries = 16; + for (int attempt = 0; attempt < kMaxBindRetries && !service_; ++attempt) + { + auto [fd, port] = txservice::test::BindEphemeralPort(); + ::close(fd); + cluster_manager_ = + std::make_unique(); + cluster_manager_->Initialize("127.0.0.1", port); + auto candidate = std::make_unique( + *cluster_manager_, + config_path_.string(), + (dir_ / "DSMigrateLog").string(), + MakeFactory()); + if (candidate->StartService(create_if_missing)) + { + service_ = std::move(candidate); + } + else + { + cluster_manager_.reset(); + } + } + if (!service_) + { + throw std::runtime_error("failed to start production datastore"); + } + + txservice::CatalogFactory *catalog_factories[3]{ + &range_catalog_factory_, &hash_catalog_factory_, nullptr}; + client_ = std::make_unique( + /*is_bootstrap=*/false, + catalog_factories, + *cluster_manager_, + /*bind_data_shard_with_ng=*/false, + service_.get()); + } + + void Stop() + { + client_.reset(); + service_.reset(); + cluster_manager_.reset(); + } + + std::filesystem::path dir_; + std::filesystem::path config_path_; + EloqRangeCatalogFactory range_catalog_factory_; + EloqHashCatalogFactory hash_catalog_factory_; + std::unique_ptr cluster_manager_; + std::unique_ptr service_; + std::unique_ptr client_; +#if defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_S3) + std::string object_store_url_; + Aws::SDKOptions aws_options_; + bool aws_initialized_{false}; +#endif +}; +} // namespace + +TEST_CASE("production datastore persists only newest-term checkpoint data", + "[real-datastore][checkpoint-flush]") +{ + const char *run_real_store = + std::getenv(std::string(kRunRealStoreEnv).c_str()); + if (run_real_store == nullptr || std::string_view(run_real_store) != "1") + { + SKIP("set ELOQ_RUN_REAL_STORE_TEST=1 to run production datastore IO"); + } + +#if !defined(DATA_STORE_TYPE_ELOQDSS_ELOQSTORE) && \ + !defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_S3) + SKIP("the configured datastore is not covered by this integration test"); +#else + RealStoreFixture fixture; + + const TableName hash_table{std::string_view("real_hash_terms"), + TableType::Primary, + TableEngine::EloqKv}; + std::unordered_map>> + hash_flush; + auto &hash_entries = hash_flush["eloqkv_real_hash_terms"]; + auto add_hash = [&](int64_t term, int key, int32_t partition_id) + { + auto records = std::make_unique>(); + records->push_back(MakeObjectRecord(key, + RecordStatus::Normal, + /*commit_ts=*/1000 + key, + partition_id)); + hash_entries.push_back( + MakeFlushEntry(MakeTask(hash_table, term), std::move(records))); + }; + add_hash(/*term=*/10, /*key=*/1, /*partition_id=*/0); + add_hash(/*term=*/11, /*key=*/2, /*partition_id=*/0); + add_hash(/*term=*/10, /*key=*/3, /*partition_id=*/1); + add_hash(/*term=*/11, /*key=*/4, /*partition_id=*/1); + REQUIRE(fixture.Client().PutAll(hash_flush)); + + const TableName range_table{std::string_view("real_range_terms"), + TableType::Primary, + TableEngine::InternalRange}; + std::unordered_map>> + range_flush; + auto &range_entries = range_flush["irange_real_range_terms"]; + auto add_range = [&](int64_t term, int key) + { + auto records = std::make_unique>(); + records->push_back(MakeSerializedRecord( + key, /*commit_ts=*/2000 + key, /*partition_id=*/5)); + range_entries.push_back( + MakeFlushEntry(MakeTask(range_table, term), std::move(records))); + }; + add_range(/*term=*/20, /*key=*/10); + add_range(/*term=*/21, /*key=*/11); + add_range(/*term=*/20, /*key=*/12); + add_range(/*term=*/21, /*key=*/13); + REQUIRE(fixture.Client().PutAll(range_flush)); + + std::unordered_map>> + delete_flush; + auto delete_records = std::make_unique>(); + delete_records->push_back(MakeObjectRecord(/*key=*/4, + RecordStatus::Deleted, + /*commit_ts=*/3004, + /*partition_id=*/1)); + delete_flush["eloqkv_real_hash_terms"].push_back(MakeFlushEntry( + MakeTask(hash_table, /*term=*/12), std::move(delete_records))); + REQUIRE(fixture.Client().PutAll(delete_flush)); + + if (fixture.Client().NeedPersistKV()) + { + // Checkpoint writes bypass RocksDB's WAL. Exercise the same explicit + // durability boundary used by FlushDataImpl before deleting all local + // RocksDB state and reopening solely from object storage. + REQUIRE(fixture.Client().PersistKV( + {"eloqkv_real_hash_terms", "irange_real_range_terms"})); + } + + auto verify = [&] + { + uint64_t record_ts = 0; + REQUIRE(ReadKey(fixture.Service(), + "eloqkv_real_hash_terms", + /*partition_id=*/0, + /*key=*/1, + record_ts) == + EloqDS::remote::DataStoreError::KEY_NOT_FOUND); + REQUIRE(ReadKey(fixture.Service(), + "eloqkv_real_hash_terms", + /*partition_id=*/0, + /*key=*/2, + record_ts) == EloqDS::remote::DataStoreError::NO_ERROR); + REQUIRE(record_ts == 1002); + REQUIRE(ReadKey(fixture.Service(), + "eloqkv_real_hash_terms", + /*partition_id=*/1, + /*key=*/3, + record_ts) == + EloqDS::remote::DataStoreError::KEY_NOT_FOUND); + REQUIRE(ReadKey(fixture.Service(), + "eloqkv_real_hash_terms", + /*partition_id=*/1, + /*key=*/4, + record_ts) == + EloqDS::remote::DataStoreError::KEY_NOT_FOUND); + + REQUIRE(ReadKey(fixture.Service(), + "irange_real_range_terms", + /*partition_id=*/5, + /*key=*/10, + record_ts) == + EloqDS::remote::DataStoreError::KEY_NOT_FOUND); + REQUIRE(ReadKey(fixture.Service(), + "irange_real_range_terms", + /*partition_id=*/5, + /*key=*/11, + record_ts) == EloqDS::remote::DataStoreError::NO_ERROR); + REQUIRE(record_ts == 2011); + REQUIRE(ReadKey(fixture.Service(), + "irange_real_range_terms", + /*partition_id=*/5, + /*key=*/12, + record_ts) == + EloqDS::remote::DataStoreError::KEY_NOT_FOUND); + REQUIRE(ReadKey(fixture.Service(), + "irange_real_range_terms", + /*partition_id=*/5, + /*key=*/13, + record_ts) == EloqDS::remote::DataStoreError::NO_ERROR); + REQUIRE(record_ts == 2013); + }; + + verify(); + fixture.RestartWithoutVolatileCloudState(); + verify(); +#endif +} diff --git a/tx_service/tests/harness/mem_data_store.cpp b/tx_service/tests/harness/mem_data_store.cpp index 4a01262c4..feb5d1851 100644 --- a/tx_service/tests/harness/mem_data_store.cpp +++ b/tx_service/tests/harness/mem_data_store.cpp @@ -137,7 +137,15 @@ void MemDataStore::FlushData(FlushDataRequest *req) PoolableGuard poolable_guard(req); ::EloqDS::remote::CommonResult result; - result.set_error_code(::EloqDS::remote::DataStoreError::NO_ERROR); + if (fail_flush_data_) + { + result.set_error_code(::EloqDS::remote::DataStoreError::FLUSH_FAILED); + result.set_error_msg("injected FlushData failure"); + } + else + { + result.set_error_code(::EloqDS::remote::DataStoreError::NO_ERROR); + } req->SetFinish(result); } diff --git a/tx_service/tests/harness/mem_data_store.h b/tx_service/tests/harness/mem_data_store.h index 7dffaf83f..045282f45 100644 --- a/tx_service/tests/harness/mem_data_store.h +++ b/tx_service/tests/harness/mem_data_store.h @@ -15,8 +15,11 @@ namespace EloqDS class MemDataStore : public DataStore { public: - MemDataStore(uint32_t shard_id, DataStoreService *data_store_service) - : DataStore(shard_id, data_store_service) + MemDataStore(uint32_t shard_id, + DataStoreService *data_store_service, + bool fail_flush_data = false) + : DataStore(shard_id, data_store_service), + fail_flush_data_(fail_flush_data) { } @@ -59,5 +62,6 @@ class MemDataStore : public DataStore std::mutex mux_; std::map store_; bool read_only_{false}; + const bool fail_flush_data_{false}; }; } // namespace EloqDS diff --git a/tx_service/tests/harness/mem_data_store_factory.h b/tx_service/tests/harness/mem_data_store_factory.h index ba6dbbe47..0d167dafb 100644 --- a/tx_service/tests/harness/mem_data_store_factory.h +++ b/tx_service/tests/harness/mem_data_store_factory.h @@ -11,6 +11,11 @@ namespace EloqDS class MemDataStoreFactory : public DataStoreFactory { public: + explicit MemDataStoreFactory(bool fail_flush_data = false) + : fail_flush_data_(fail_flush_data) + { + } + std::unique_ptr CreateDataStore( bool /*create_if_missing*/, uint32_t shard_id, @@ -18,7 +23,8 @@ class MemDataStoreFactory : public DataStoreFactory bool start_db = true, int64_t term = 0) override { - auto ds = std::make_unique(shard_id, data_store_service); + auto ds = std::make_unique( + shard_id, data_store_service, fail_flush_data_); // Surface startup failures immediately (mirrors // RocksDBDataStoreFactory) instead of returning a half-initialized // store that fails later in request paths. @@ -90,5 +96,8 @@ class MemDataStoreFactory : public DataStoreFactory { return false; } + +private: + const bool fail_flush_data_{false}; }; } // namespace EloqDS From 830ed3156a579355a421203d6f8898c57a34d84c Mon Sep 17 00:00:00 2001 From: Liang Jeff Chen Date: Mon, 17 Aug 2026 01:46:29 -0700 Subject: [PATCH 2/7] test: report Catch2 skips as skipped, not as ctest failures Catch2 exits 4 when every selected test case skipped. ctest treats any non-zero exit as a failure, so a test that correctly opts out was reported red: the PersistKV-failure case skips on builds whose backend has no post-PutAll persistence boundary, and the production-datastore case skips unless ELOQ_RUN_REAL_STORE_TEST=1 allows it to touch real storage. Set SKIP_RETURN_CODE on the discovered tests so ctest reports those as skipped. Both still pass when their precondition is met. Co-Authored-By: Claude Opus 5 (1M context) --- tx_service/tests/CMakeLists.txt | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tx_service/tests/CMakeLists.txt b/tx_service/tests/CMakeLists.txt index dc6de2620..84bb1d7b1 100644 --- a/tx_service/tests/CMakeLists.txt +++ b/tx_service/tests/CMakeLists.txt @@ -103,7 +103,12 @@ function(add_substrate_test name catch_lib) target_link_libraries(${name} PRIVATE test_harness ${catch_lib}) set_property(TARGET ${name} PROPERTY CXX_STANDARD 20) set_property(TARGET ${name} PROPERTY CXX_EXTENSIONS OFF) - catch_discover_tests(${name} PROPERTIES LABELS "${name}") + # Catch2 exits 4 when every selected test skipped. Without this, a test that + # correctly opts out -- because the build lacks the backend it covers, or + # because it needs an env var to touch real storage -- is reported as a + # failure. + catch_discover_tests(${name} + PROPERTIES LABELS "${name}" SKIP_RETURN_CODE 4) endfunction() foreach(name ${CATCH_MAIN_TESTS}) @@ -138,4 +143,5 @@ target_link_libraries(ClusterCrossNg-Test PRIVATE set_property(TARGET ClusterCrossNg-Test PROPERTY CXX_STANDARD 20) set_property(TARGET ClusterCrossNg-Test PROPERTY CXX_EXTENSIONS OFF) add_dependencies(ClusterCrossNg-Test txnode) -catch_discover_tests(ClusterCrossNg-Test PROPERTIES LABELS "ClusterCrossNg-Test") +catch_discover_tests(ClusterCrossNg-Test + PROPERTIES LABELS "ClusterCrossNg-Test" SKIP_RETURN_CODE 4) From 29dd186e24773d852f9bec3ab97f6098af6da595 Mon Sep 17 00:00:00 2001 From: Liang Jeff Chen Date: Fri, 21 Aug 2026 00:14:03 -0700 Subject: [PATCH 3/7] fix: complete checkpoint accounting after range split A flush task created during a range split was flagged no-ckpt-ts-report when the child range mapped to another core or another node group. The rationale was that those CCEs will be force-evicted after the split, so publishing was pointless -- but an entry the flush still references carries in-flight checkpoint state (BeingCkpt) that only the publication path clears, so the skipped entries could never be evicted and migration never finished. Remove the flag: every flushed record publishes, which both clears the in-flight state and truthfully advances the cce's ckpt ts (the record was durably written by this very flush). What the flag was papering over is a routing bug: a split-child task's id names the destination range, but the scanned CCEs stay in the source range's CcMap until split cleanup, so the update must be enqueued to the parent range's core. CheckpointCceOwnerCore() encapsulates that derivation and replaces the open-coded (id & 0x3FF) % core_cnt at both call sites (the deferred publication grouping and the per-partition publication path). This supersedes the mechanism of #556, which routed a ClearBeingCkpt-only request to the parent core while keeping the flag: the full update at the correct core covers the same cases -- the cce leaves BeingCkpt either way -- without a second UpdateCceCkptTsCc mode, and additionally lets split-source entries be evicted as clean instead of waiting for forced cleanup. Ported from fix/range-split-checkpoint-accounting (ea1868e), adapted to the per-partition/deferred publication split introduced by this branch. RangeSplitCheckpoint-Test drives a real split-child task end to end and asserts the update lands on the source core and fully cleans the entry. Co-Authored-By: Claude Fable 5 --- store_handler/data_store_service_client.cpp | 15 +- tx_service/include/data_sync_task.h | 12 +- tx_service/src/data_sync_task.cpp | 45 +++-- tx_service/tests/CMakeLists.txt | 1 + tx_service/tests/CheckpointFlush-Test.cpp | 21 +- .../tests/RangeSplitCheckpoint-Test.cpp | 189 ++++++++++++++++++ 6 files changed, 239 insertions(+), 44 deletions(-) create mode 100644 tx_service/tests/RangeSplitCheckpoint-Test.cpp diff --git a/store_handler/data_store_service_client.cpp b/store_handler/data_store_service_client.cpp index b32d79485..1bde3b99e 100644 --- a/store_handler/data_store_service_client.cpp +++ b/store_handler/data_store_service_client.cpp @@ -6019,8 +6019,7 @@ void DataStoreServiceClient::PreparePartitionBatches( // Remember which cc entry this record came from, so the partition can // advance its ckpt ts as soon as it is durable. A hash-partitioned // task's id is the owning cc shard. - if (publish_ckpt_ts_on_complete && ckpt_rec.cce_ != nullptr && - flush_entry->data_sync_task_->need_update_ckpt_ts_) + if (publish_ckpt_ts_on_complete && ckpt_rec.cce_ != nullptr) { partition_state.AddCkptTsEntry( flush_entry->data_sync_task_.get(), @@ -6146,17 +6145,15 @@ void DataStoreServiceClient::PrepareRangePartitionBatches( for (auto idx : flush_recs) { const auto &flush_entry = entries.at(idx); - const bool collect_ckpt_ts = - publish_ckpt_ts_on_complete && - flush_entry->data_sync_task_->need_update_ckpt_ts_; + const bool collect_ckpt_ts = publish_ckpt_ts_on_complete; size_t core_idx = 0; if (collect_ckpt_ts) { assert(local_shards != nullptr); - // A range task's id is the range id; its owning cc shard is - // derived from the low bits, matching how the scan sharded it. - core_idx = static_cast( - (flush_entry->data_sync_task_->id_ & 0x3FF) % + // The owning cc shard derives from the range id -- the parent + // range's id while a split is in flight, since the scanned CCEs + // stay in the source range's CcMap until split cleanup. + core_idx = flush_entry->data_sync_task_->CheckpointCceOwnerCore( local_shards->Count()); } diff --git a/tx_service/include/data_sync_task.h b/tx_service/include/data_sync_task.h index 65ebb2c12..ac305daf4 100644 --- a/tx_service/include/data_sync_task.h +++ b/tx_service/include/data_sync_task.h @@ -235,7 +235,6 @@ struct DataSyncTask is_dirty_(is_dirty), sync_ts_adjustable_(need_adjust_ts), task_res_(hres), - need_update_ckpt_ts_(true), high_priority_(high_priority) { } @@ -295,6 +294,16 @@ struct DataSyncTask return sync_ts_adjustable_; } + /** + * @brief The cc shard that owns the CCEs this task's flush collected. + * + * Range data is sharded by range id. A split-child task scans CCEs that + * remain in the source range's CcMap until split cleanup completes, so + * during a split the owning core derives from the parent range's id, not + * the child's. + */ + size_t CheckpointCceOwnerCore(size_t cc_shard_count) const; + void UnsetSyncTsAdjustable() { sync_ts_adjustable_ = false; @@ -360,7 +369,6 @@ struct DataSyncTask absl::flat_hash_map> cce_entries_; - bool need_update_ckpt_ts_{true}; bool high_priority_{false}; }; diff --git a/tx_service/src/data_sync_task.cpp b/tx_service/src/data_sync_task.cpp index 72e1d3009..3cc082887 100644 --- a/tx_service/src/data_sync_task.cpp +++ b/tx_service/src/data_sync_task.cpp @@ -109,7 +109,6 @@ std::vector CollectCkptTsUpdateGroups( const DataSyncTask *first_task = table_entries.front()->data_sync_task_.get(); assert(first_task != nullptr); - const bool hash_partitioned = first_task->table_name_.IsHashPartitioned(); std::unordered_map group_indices; for (const auto &entry : table_entries) @@ -118,15 +117,12 @@ std::vector CollectCkptTsUpdateGroups( assert(task != nullptr); assert(task->table_name_ == first_task->table_name_); if (!IsNewestTerm(*entry, newest_terms) || - !task->need_update_ckpt_ts_ || entry->data_sync_vec_ == nullptr) + entry->data_sync_vec_ == nullptr) { continue; } - const size_t core_idx = - hash_partitioned - ? static_cast(task->id_) - : static_cast((task->id_ & 0x3FF) % cc_shard_count); + const size_t core_idx = task->CheckpointCceOwnerCore(cc_shard_count); assert(core_idx < cc_shard_count); for (const FlushRecord &record : *entry->data_sync_vec_) @@ -259,22 +255,29 @@ DataSyncTask::DataSyncTask(const TableName &table_name, { id_ = range_entry_->GetRangeInfo()->GetKeyNewRangeId(start_key_); } +} + +size_t DataSyncTask::CheckpointCceOwnerCore(size_t cc_shard_count) const +{ + assert(cc_shard_count > 0); + assert(id_ >= 0); + + if (table_name_.IsHashPartitioned()) + { + // Hash-partition checkpoint tasks are created per local core. + return static_cast(id_) % cc_shard_count; + } + + int32_t range_id = id_; + if (during_split_range_) + { + assert(range_entry_ != nullptr); + // id_ is the child range ID, but the scanned CCEs are still owned by + // the parent range's CC shard. + range_id = range_entry_->GetRangeInfo()->PartitionId(); + } - // For a data sync task during range split, we only need to update the ckpt - // ts if the new range owner is the current node group. - NodeGroupId range_owner = Sharder::Instance() - .GetLocalCcShards() - ->GetRangeOwner(id_, ng_id) - ->BucketOwner(); - - size_t local_shard_count = Sharder::Instance().GetLocalCcShardsCount(); - int32_t old_range_id = range_entry_->GetRangeInfo()->PartitionId(); - uint16_t old_range_owner_shard = - static_cast((old_range_id & 0x3FF) % local_shard_count); - uint16_t new_range_owner_shard = - static_cast((id_ & 0x3FF) % local_shard_count); - need_update_ckpt_ts_ = - range_owner == ng_id && old_range_owner_shard == new_range_owner_shard; + return static_cast((range_id & 0x3FF) % cc_shard_count); } void DataSyncTask::SetFinish() diff --git a/tx_service/tests/CMakeLists.txt b/tx_service/tests/CMakeLists.txt index 84bb1d7b1..b2d318080 100644 --- a/tx_service/tests/CMakeLists.txt +++ b/tx_service/tests/CMakeLists.txt @@ -68,6 +68,7 @@ set(CATCH_MAIN_TESTS CheckpointMetricsState-Test CheckpointFlush-Test FetchRecordCc-Test + RangeSplitCheckpoint-Test RealDataStore-Test NonBlockingLock-Test AcquireAllError-Test diff --git a/tx_service/tests/CheckpointFlush-Test.cpp b/tx_service/tests/CheckpointFlush-Test.cpp index 2ecc4585a..a0b682072 100644 --- a/tx_service/tests/CheckpointFlush-Test.cpp +++ b/tx_service/tests/CheckpointFlush-Test.cpp @@ -882,11 +882,9 @@ TEST_CASE("deferred checkpoint publication aggregates only newest-term entries", NodeGroupId node_group_id, int32_t core_id, uintptr_t cce_address, - uint64_t commit_ts, - bool need_update_ckpt_ts = true) + uint64_t commit_ts) { auto task = MakeTaskPtr(table_name, term, node_group_id, core_id); - task->need_update_ckpt_ts_ = need_update_ckpt_ts; auto records = std::make_unique>(); records->push_back(MakeObjectRecord(static_cast(commit_ts), "value", @@ -910,14 +908,10 @@ TEST_CASE("deferred checkpoint publication aggregates only newest-term entries", add_hash_record("eloqkv_deferred_hash", hash_table, 31, 1, 1, 0x40, 103); add_hash_record("eloqkv_deferred_hash", hash_table, 8, 2, 2, 0x50, 104); add_hash_record("eloqkv_deferred_hash", hash_table, 31, 1, 3, 0, 105); - add_hash_record("eloqkv_deferred_hash", - hash_table, - 31, - 1, - 3, - 0x60, - 106, - /*need_update_ckpt_ts=*/false); + // Tasks flagged as no-ckpt-ts-report used to be excluded here; the flag is + // gone (every flushed record publishes so its cce leaves the in-flight + // checkpoint state), so this record must appear under core 3. + add_hash_record("eloqkv_deferred_hash", hash_table, 31, 1, 3, 0x60, 106); auto null_vector_entry = MakeFlushEntry( MakeTaskPtr(hash_table, /*term=*/31, /*node_group_id=*/1, /*id=*/3), std::make_unique>()); @@ -951,13 +945,16 @@ TEST_CASE("deferred checkpoint publication aggregates only newest-term entries", REQUIRE(node_group_1 != nullptr); REQUIRE(node_group_1->node_group_term_ == 31); - REQUIRE(node_group_1->cce_entries_.size() == 2); + REQUIRE(node_group_1->cce_entries_.size() == 3); REQUIRE(node_group_1->cce_entries_.at(0).size() == 1); REQUIRE(node_group_1->cce_entries_.at(0).front().cce_ == reinterpret_cast(uintptr_t{0x20})); REQUIRE(node_group_1->cce_entries_.at(1).size() == 1); REQUIRE(node_group_1->cce_entries_.at(1).front().cce_ == reinterpret_cast(uintptr_t{0x40})); + REQUIRE(node_group_1->cce_entries_.at(3).size() == 1); + REQUIRE(node_group_1->cce_entries_.at(3).front().cce_ == + reinterpret_cast(uintptr_t{0x60})); REQUIRE(node_group_2 != nullptr); REQUIRE(node_group_2->node_group_term_ == 8); diff --git a/tx_service/tests/RangeSplitCheckpoint-Test.cpp b/tx_service/tests/RangeSplitCheckpoint-Test.cpp new file mode 100644 index 000000000..420a0d8ad --- /dev/null +++ b/tx_service/tests/RangeSplitCheckpoint-Test.cpp @@ -0,0 +1,189 @@ +/** + * Regression coverage for checkpoint completion while a range is split. + */ +#include + +#define protected public +#define private public + +#include "cc_entry.h" +#include "cc_req_misc.h" +#include "cc_shard.h" +#include "data_sync_task.h" +#include "include/mock/mock_catalog_factory.h" +#include "local_cc_shards.h" +#include "range_record.h" +#include "template_cc_map.h" +#include "tx_key.h" +#include "tx_record.h" +#include "type.h" + +namespace txservice +{ +namespace +{ +using TestKey = CompositeKey; +using TestRecord = CompositeRecord; +using TestCcMap = TemplateCcMap; +using TestCcEntry = CcEntry; + +struct SplitCheckpointFixture +{ + std::unordered_map> ng_configs{ + {0, {NodeConfig(0, "127.0.0.1", 8600)}}}; + std::map tx_cnf{ + {"node_memory_limit_mb", 1000}, + {"enable_key_cache", 0}, + {"reltime_sampling", 0}, + {"range_split_worker_num", 1}, + {"range_slice_memory_limit_percent", 20}, + {"core_num", 2}, + {"realtime_sampling", 0}, + {"checkpointer_interval", 10}, + {"checkpointer_delay_seconds", 0}, + {"checkpointer_min_ckpt_request_interval", 5}, + {"enable_shard_heap_defragment", 0}, + {"node_log_limit_mb", 1000}, + {"collect_active_tx_ts_interval_seconds", 2}, + {"rep_group_cnt", 1}, + }; + MockCatalogFactory mock_catalog_factory; + CatalogFactory *catalog_factory[5] = { + &mock_catalog_factory, + &mock_catalog_factory, + &mock_catalog_factory, + &mock_catalog_factory, + &mock_catalog_factory, + }; + LocalCcShards local_cc_shards; + std::string raft_path; + + SplitCheckpointFixture() + : local_cc_shards(0, + 0, + tx_cnf, + catalog_factory, + nullptr, + &ng_configs, + 2, + nullptr, + nullptr, + true) + { + local_cc_shards.BindThreadToFastMetaDataShard(0); + local_cc_shards.GetCcShard(0)->Init(); + local_cc_shards.GetCcShard(1)->Init(); + auto &sharder = Sharder::Instance(0, + &ng_configs, + 0, + nullptr, + nullptr, + &local_cc_shards, + nullptr, + &raft_path); + // The lightweight fixture does not call Sharder::Init(), which is + // responsible for enabling leader-term lookups in a server process. + // Enable that gate explicitly so UpdateCceCkptTsCc takes its normal + // leader-term path. + sharder.cc_nodes_init_.store(true, std::memory_order_release); + sharder.SetStandbyNodeTerm(-1); + sharder.SetLeaderTerm(0, 1); + } + + ~SplitCheckpointFixture() + { + local_cc_shards.Terminate(); + } +}; +} // namespace + +TEST_CASE("split-range checkpoint updates source CCE shard", "[checkpoint]") +{ + SplitCheckpointFixture fixture; + TableName table_name(std::string("split_checkpoint"), + TableType::Primary, + TableEngine::EloqSql); + CcShard *source_shard = fixture.local_cc_shards.GetCcShard(0); + source_shard->native_ccms_.try_emplace( + table_name, + std::make_unique( + source_shard, 0, table_name, 1, nullptr, true)); + + // Source range 0 splits at this key into destination range 1. With two + // local shards, the child ID maps to core 1 while its CCE remains in the + // source CcMap on core 0 until split cleanup. + TestKey source_start = std::make_tuple(std::string("split_checkpoint"), 0); + TestKey child_start = std::make_tuple(std::string("split_checkpoint"), 100); + TemplateTableRangeEntry source_range(&source_start, 10, 0); + std::vector split_keys; + split_keys.emplace_back(&child_start); + source_range.UploadNewRangeInfo(std::move(split_keys), {1}, 20); + + TxKey child_start_key(&child_start); + TxKey source_end_key(&child_start); + DataSyncTask split_child_task(table_name, + 0, + 1, + nullptr, + &source_range, + child_start_key, + source_end_key, + 30, + true, + false, + 0, + nullptr, + nullptr); + REQUIRE(split_child_task.id_ == 1); + + // The key hash maps to core 1, but range data is owned by the source + // range's core (core 0), not by the key hash or child range ID. + int record_key_suffix = 0; + TestKey record_key = + std::make_tuple(std::string("split_checkpoint"), record_key_suffix); + while ((TxKey(&record_key).Hash() & 0x3FF) % 2 != 1) + { + record_key = std::make_tuple(std::string("split_checkpoint"), + ++record_key_suffix); + } + size_t cce_owner_core = split_child_task.CheckpointCceOwnerCore(2); + REQUIRE(cce_owner_core == 0); + REQUIRE(cce_owner_core != static_cast(split_child_task.id_)); + + TableName hash_table_name(std::string("split_checkpoint_hash"), + TableType::Primary, + TableEngine::EloqKv); + DataSyncTask hash_task( + hash_table_name, 3, 0, 0, 1, 0, nullptr, false, false, nullptr); + REQUIRE(hash_task.CheckpointCceOwnerCore(2) == 1); + + auto *source_map = + static_cast(source_shard->GetCcm(table_name, 0)); + REQUIRE(source_map != nullptr); + bool emplace = false; + auto it = source_map->FindEmplace(record_key, &emplace, false, false); + REQUIRE(emplace); + TestCcEntry *cce = it->second; + bool was_dirty = cce->IsDirty(); + cce->SetCommitTsPayloadStatus(40, RecordStatus::Normal); + source_map->OnCommittedUpdate(cce, was_dirty); + cce->SetBeingCkpt(); + REQUIRE(cce->IsDirty()); + REQUIRE(cce->GetBeingCkpt()); + REQUIRE(source_map->dirty_data_key_count_ == 1); + + absl::flat_hash_map> + updates; + updates[cce_owner_core].emplace_back(cce, 40, 321); + UpdateCceCkptTsCc update_req(0, 1, table_name, updates); + update_req.Execute(*source_shard); + + REQUIRE(update_req.IsFinished()); + REQUIRE(cce->CkptTs() == 40); + REQUIRE_FALSE(cce->GetBeingCkpt()); + REQUIRE_FALSE(cce->IsDirty()); + REQUIRE(cce->entry_info_.DataStoreSize() == 321); + REQUIRE(source_map->dirty_data_key_count_ == 0); +} + +} // namespace txservice From a432b8afbe584afcae38737bc74356fb7ef5ff72 Mon Sep 17 00:00:00 2001 From: Liang Jeff Chen Date: Sat, 22 Aug 2026 00:39:31 -0700 Subject: [PATCH 4/7] fix(ckpt): release flush quota only for memory actually freed Progressive quota release returned quota when a partition became durable, but durability frees nothing: the FlushRecord buffers stay resident until the flush task ends. The controller therefore under-counted resident flush memory, and freshly admitted scans could transiently push it to roughly twice ckpt_buffer_ratio. The failure path compounded the mismatch by reporting a partition's full serialized weight even when later batches were never sent. Free the memory for real instead. A record belongs to exactly one kv partition, and the batches reference its key/payload buffers zero-copy, so partition completion -- success or failure -- is the first moment nothing views them. Each partition now takes ownership of its records at batch-preparation time, charging FlushRecord::FlushSize(), the same unit DataSyncScan charged; on completion it frees their key/payload memory (FlushRecord::ReleaseMemory keeps the cce/timestamp metadata later stages read) and reports exactly the freed charge. FlushDataImpl releases that watermark directly -- no serialized-to-memory proportional conversion -- capped by the task's own charge. The unconditional tail release still covers vector footprints and failed, skipped, or deferred shares, and the round frees those buffers before returning it, so the amounts sum to what was taken and quota release never precedes the memory it stands for on any backend -- including stores that defer publication to PersistKV, whose remainder is the whole share. The interleaving vectors themselves still live until the task ends; only their per-record heap allocations are freed early, which is where the bytes are. Slice-metadata staging reads record keys at scan time, before flush, so nothing outside the partition needs the freed buffers. Reported-by: thweetkomputer (review on #555) Co-Authored-By: Claude Fable 5 --- docs/07-durability-and-recovery.md | 8 +- docs/09-store-handler.md | 8 +- store_handler/data_store_service_client.cpp | 24 +++- .../data_store_service_client_closure.cpp | 37 ++++- .../data_store_service_client_closure.h | 48 +++++-- tx_service/include/cc/cc_entry.h | 23 +++ tx_service/src/cc/local_cc_shards.cpp | 64 +++++---- tx_service/tests/CheckpointFlush-Test.cpp | 134 +++++++++++++++++- 8 files changed, 289 insertions(+), 57 deletions(-) diff --git a/docs/07-durability-and-recovery.md b/docs/07-durability-and-recovery.md index 4b8f1d922..e3e31a7be 100644 --- a/docs/07-durability-and-recovery.md +++ b/docs/07-durability-and-recovery.md @@ -64,6 +64,12 @@ Clean entries can be evicted after their checkpoint state is published. The NG checkpoint timestamp and log-truncation boundary advance only after every included task succeeds, independently of earlier entry-level progress. +Flush quota represents resident scan buffers. A completed partition releases +quota only after freeing the key and payload allocations it owns; shared +metadata and deferred buffers remain charged until the round releases them. +This keeps admission accounting bounded by memory actually retained across +successful, failed, and deferred writes. + The same data-sync machinery also supports range splits, index construction, bucket migration, snapshots, and shutdown flushing. Those callers can require a flush without allowing log truncation; a task's origin and completion state @@ -175,4 +181,4 @@ accepted-request event. | Replay restores metadata dependencies and resumes incomplete durable operations | `tx_service/include/cc/catalog_cc_map.h`; `tx_service/include/cc/range_cc_map.h`; `tx_service/include/cc/cluster_config_cc_map.h`; `tx_service/include/cc/range_bucket_cc_map.h` | | MVCC versions are archived, fetched, and reclaimed against active snapshots | `tx_service/include/cc/cc_entry.h`; `tx_service/include/tx_start_ts_collector.h`; `tx_service/include/store/data_store_handler.h` | | Standby and backup snapshots are coordinated above the storage interface | `tx_service/include/store/snapshot_manager.h`; `tx_service/src/store/snapshot_manager.cpp`; `tx_service/include/store/data_store_handler.h` | -| Checkpoint state and MVCC behavior have focused unit coverage | `tx_service/tests/CheckpointMetricsState-Test.cpp`; `tx_service/tests/CcEntry-Test.cpp`; `tx_service/tests/StartTsCollector-Test.cpp` | +| Checkpoint publication, split ownership, state and MVCC behavior have focused unit coverage | `tx_service/tests/CheckpointFlush-Test.cpp`; `tx_service/tests/RangeSplitCheckpoint-Test.cpp`; `tx_service/tests/CheckpointMetricsState-Test.cpp`; `tx_service/tests/CcEntry-Test.cpp`; `tx_service/tests/StartTsCollector-Test.cpp` | diff --git a/docs/09-store-handler.md b/docs/09-store-handler.md index 59cb373a3..f6940c014 100644 --- a/docs/09-store-handler.md +++ b/docs/09-store-handler.md @@ -96,6 +96,10 @@ This distinction between entry-level progress and the NG-wide truncation boundary is the central contract between `DataStoreHandler` and the durability layer. +Each completed partition relinquishes its record buffers before reporting the +freed memory to data-sync admission. Buffers still needed by the flush round +retain their quota until their consumers finish, including on failure paths. + Cache misses reverse the path: the handler returns a record or streams a bounded range slice, and the completion is installed on the owning shard. MVCC reads may request the latest archive version visible at a timestamp rather than the @@ -122,8 +126,8 @@ term writable again. - WAL decides transaction commit; the store handler supplies the later checkpoint durability boundary. -- `PutAll` alone is sufficient only for a backend that does not require - `PersistKV`. +- Base-partition completion permits checkpoint publication only when the + backend needs no further persistence barrier and MVCC archives are not pending. - Asynchronous storage completion mutates CC state only after returning to the owning shard. - Cache-miss coalescing retains fetch ownership through retries; pool saturation diff --git a/store_handler/data_store_service_client.cpp b/store_handler/data_store_service_client.cpp index 1bde3b99e..7c0963dff 100644 --- a/store_handler/data_store_service_client.cpp +++ b/store_handler/data_store_service_client.cpp @@ -492,7 +492,7 @@ bool DataStoreServiceClient::PutAllImpl( sync_putall->total_bytes_ = 0; for (const auto *ps : sync_putall->partition_states_) { - sync_putall->total_bytes_ += ps->serialized_bytes_; + sync_putall->total_bytes_ += ps->charged_mem_bytes_; } // Install coroutine callbacks before starting async writes: a local @@ -544,7 +544,7 @@ bool DataStoreServiceClient::PutAllImpl( { // No batches for this partition, mark as completed sync_putall->OnPartitionCompleted( - partition_state->serialized_bytes_); + partition_state->ReleaseFlushRecordsMemory()); } } // Wait for all partitions to complete @@ -6016,6 +6016,14 @@ void DataStoreServiceClient::PreparePartitionBatches( txservice::FlushRecord &ckpt_rec = flush_entry->data_sync_vec_->at(idx.second); + if (publish_ckpt_ts_on_complete) + { + // Every batch view into this record belongs to this partition, + // so the partition frees the record's buffers -- and returns + // their charged quota -- when it completes. + partition_state.AddFlushRecord(&ckpt_rec); + } + // Remember which cc entry this record came from, so the partition can // advance its ckpt ts as soon as it is durable. A hash-partitioned // task's id is the owning cc shard. @@ -6038,7 +6046,6 @@ void DataStoreServiceClient::PreparePartitionBatches( batch_request.record_tmp_mem_area.size() == batch_request.record_tmp_mem_area.capacity()) { - partition_state.serialized_bytes_ += write_batch_size; partition_state.AddBatch(std::move(batch_request)); batch_request.Reset( @@ -6062,7 +6069,6 @@ void DataStoreServiceClient::PreparePartitionBatches( // Add the last batch if it has data if (batch_request.key_parts.size() > 0) { - partition_state.serialized_bytes_ += write_batch_size; partition_state.AddBatch(std::move(batch_request)); } @@ -6159,6 +6165,14 @@ void DataStoreServiceClient::PrepareRangePartitionBatches( for (auto &ckpt_rec : *flush_entry->data_sync_vec_) { + if (collect_ckpt_ts) + { + // Every batch view into this record belongs to this + // partition, so the partition frees the record's buffers -- + // and returns their charged quota -- when it completes. + partition_state.AddFlushRecord(&ckpt_rec); + } + if (collect_ckpt_ts && ckpt_rec.cce_ != nullptr) { partition_state.AddCkptTsEntry( @@ -6178,7 +6192,6 @@ void DataStoreServiceClient::PrepareRangePartitionBatches( batch_request.record_tmp_mem_area.size() == batch_request.record_tmp_mem_area.capacity()) { - partition_state.serialized_bytes_ += write_batch_size; partition_state.AddBatch(std::move(batch_request)); batch_request.Reset( @@ -6198,7 +6211,6 @@ void DataStoreServiceClient::PrepareRangePartitionBatches( // Add the last batch if it has data if (batch_request.key_parts.size() > 0) { - partition_state.serialized_bytes_ += write_batch_size; partition_state.AddBatch(std::move(batch_request)); } diff --git a/store_handler/data_store_service_client_closure.cpp b/store_handler/data_store_service_client_closure.cpp index 66fdb0794..d07cf6017 100644 --- a/store_handler/data_store_service_client_closure.cpp +++ b/store_handler/data_store_service_client_closure.cpp @@ -604,9 +604,13 @@ void PartitionBatchCallback(void *data, if (result.error_code() != remote::DataStoreError::NO_ERROR) { partition_state->MarkFailed(result); + // The partition sends nothing more, so its record buffers are dead: + // free them and report only what was actually freed, no matter how + // many batches never went out. + const uint64_t freed_bytes = + partition_state->ReleaseFlushRecordsMemory(); // Notify the global coordinator that this partition failed - global_coordinator->OnPartitionCompleted( - partition_state->serialized_bytes_); + global_coordinator->OnPartitionCompleted(freed_bytes); return; } @@ -634,7 +638,13 @@ void PartitionBatchCallback(void *data, } else { - // Every batch of this partition is durable. Publish the ckpt ts of the + // Every batch of this partition is durable, so nothing views the + // record buffers again: free them here, on the storage thread rather + // than a cc shard, so the completion report below returns quota whose + // memory is already free. + const uint64_t freed_bytes = + partition_state->ReleaseFlushRecordsMemory(); + // Publish the ckpt ts of the // cc entries it carried before reporting completion, so those entries // become evictable now instead of when the slowest sibling partition in // the same flush task lands. The request was constructed and its @@ -668,8 +678,7 @@ void PartitionBatchCallback(void *data, { // No ckpt-ts entries were collected, so no request was armed. // Report completion directly. - global_coordinator->OnPartitionCompleted( - partition_state->serialized_bytes_); + global_coordinator->OnPartitionCompleted(freed_bytes); } } } @@ -1885,9 +1894,25 @@ void PartitionFlushState::ArmCkptTsUpdate(SyncPutAllData *sync_putall) ckpt_ts_task_->node_group_term_, ckpt_ts_task_->table_name_, ckpt_ts_entries_); - const uint64_t done_bytes = serialized_bytes_; + // The armed hook runs after the partition's buffers were freed on the + // storage thread (PartitionBatchCallback frees before enqueueing this + // request), so reporting the partition's full charge is accurate. + const uint64_t done_bytes = charged_mem_bytes_; ckpt_ts_update_->SetOnFinished( [sync_putall, done_bytes] { sync_putall->OnPartitionCompleted(done_bytes); }); } + +uint64_t PartitionFlushState::ReleaseFlushRecordsMemory() +{ + uint64_t freed = 0; + for (txservice::FlushRecord *rec : flush_records_) + { + rec->ReleaseMemory(); + } + flush_records_.clear(); + freed = charged_mem_bytes_; + charged_mem_bytes_ = 0; + return freed; +} } // namespace EloqDS diff --git a/store_handler/data_store_service_client_closure.h b/store_handler/data_store_service_client_closure.h index 4a77e9acb..f36572bde 100644 --- a/store_handler/data_store_service_client_closure.h +++ b/store_handler/data_store_service_client_closure.h @@ -188,12 +188,16 @@ struct PartitionFlushState : public Poolable remote::CommonResult result; mutable bthread::Mutex mux; - // Serialized bytes this partition carries across all of its batches, - // accumulated while the batches are prepared. Used as the weight of this - // partition when the flush releases its memory quota progressively: byte - // weights track the real (uneven) split of the flush task far better than - // partition counts. - uint64_t serialized_bytes_{0}; + // Records whose key/payload buffers this partition references + // exclusively -- every batch view into them belongs to this partition -- + // together with their charged flush-memory bytes + // (FlushRecord::FlushSize(), the unit DataSyncScan charged against the + // flush memory quota). When the partition completes, + // ReleaseFlushRecordsMemory() frees these buffers, and the completion + // report returns charged_mem_bytes_ of quota: what is released is what + // has actually been freed. + std::vector flush_records_; + uint64_t charged_mem_bytes_{0}; // PutAll discards lower-term tasks per node group before partition // grouping. All entries retained in one kv partition therefore share one @@ -221,7 +225,8 @@ struct PartitionFlushState : public Poolable } failed = false; result.Clear(); - serialized_bytes_ = 0; + flush_records_.clear(); + charged_mem_bytes_ = 0; ckpt_ts_update_.reset(); ckpt_ts_entries_.clear(); ckpt_ts_task_ = nullptr; @@ -234,12 +239,31 @@ struct PartitionFlushState : public Poolable { pending_batches.pop(); } - serialized_bytes_ = 0; + flush_records_.clear(); + charged_mem_bytes_ = 0; ckpt_ts_update_.reset(); ckpt_ts_entries_.clear(); ckpt_ts_task_ = nullptr; } + /** + * @brief Takes ownership of a record's key/payload buffers for release + * when this partition completes, charging its FlushSize() to the + * partition. + */ + void AddFlushRecord(txservice::FlushRecord *rec) + { + flush_records_.push_back(rec); + charged_mem_bytes_ += rec->FlushSize(); + } + + /** + * @brief Frees the key/payload memory of every record this partition + * carried and returns the charged bytes now actually free. Idempotent: + * the list is cleared, so a second call returns 0 and frees nothing. + */ + uint64_t ReleaseFlushRecordsMemory(); + /** * @brief Records a cc entry made durable by this partition. * @@ -343,10 +367,10 @@ struct SyncPutAllData : public Poolable /** * @brief Installs the flush-progress consumer, invoked directly from * OnPartitionCompleted() on whichever thread completes a partition, with - * the cumulative serialized bytes of the finished partitions and the - * flush's total. Byte weights, not partition counts: partitions are - * unevenly sized, and the caller releases flush memory quota in - * proportion. + * the cumulative charged flush-memory bytes whose buffers the finished + * partitions have actually freed, and the flush's total charge. The + * caller returns exactly that much flush memory quota: release equals + * what has been freed. * * The callback must be safe to run from any completion context (a * storage callback thread, the flush coroutine, or a cc shard via the diff --git a/tx_service/include/cc/cc_entry.h b/tx_service/include/cc/cc_entry.h index c87fb20d3..a7a1ac8ab 100644 --- a/tx_service/include/cc/cc_entry.h +++ b/tx_service/include/cc/cc_entry.h @@ -293,6 +293,29 @@ struct FlushRecord return sizeof(size_t) + PayloadSize(); } } + + /** + * @brief Frees the key and payload heap memory, keeping the flush + * metadata (cce_, commit_ts_, post_flush_size_, payload_status_, + * partition_id_) intact. + * + * Callable once nothing references this record's buffers any more -- for + * a partitioned flush, when the record's kv partition has completed and + * no batch views its memory again. Freeing per record is what lets the + * flush return memory quota as partitions land: the surrounding vector + * interleaves records of many partitions and is only destroyed when the + * whole flush task ends. + */ + void ReleaseMemory() + { + // Destroys whichever alternative is held: drops a shared payload's + // reference, or frees a blob payload's buffer. + payload_.emplace>(nullptr); + if (std::holds_alternative(flush_key_)) + { + flush_key_.emplace(); + } + } }; struct LruEntry diff --git a/tx_service/src/cc/local_cc_shards.cpp b/tx_service/src/cc/local_cc_shards.cpp index 45d9ecd15..317b972fc 100644 --- a/tx_service/src/cc/local_cc_shards.cpp +++ b/tx_service/src/cc/local_cc_shards.cpp @@ -6121,20 +6121,21 @@ void LocalCcShards::FlushDataImpl(FlushDataTask *cur_work, // duration of PutAll, so with several tasks in flight the quota is what // stalls DataSyncScan in AllocateFlushMemQuota. // - // The release is proportional by serialized bytes -- quota * done_bytes / - // total_bytes. The store handler reports each finished partition's - // serialized weight, which tracks the real (uneven) split of the flush - // task; the quota itself was charged in in-memory bytes as one lump, so a - // proportion is still needed to convert between the two units. The - // progress is a cumulative watermark rather than a fixed amount per - // report: one wake-up may coalesce several partition completions, so - // `done_bytes` can jump arbitrarily between reports. Each report releases - // the difference between the new watermark and what has already been - // released; a report whose (truncated) watermark has not advanced is a - // no-op, which also makes duplicate reports harmless. The remainder -- - // integer truncation, or the whole share when PutAll fails or is skipped - // -- is released by the unconditional DeallocateFlushMemQuota below, so - // the amounts always sum to exactly what was taken. + // Quota returned tracks memory actually freed: when a partition + // completes, the store handler frees the key/payload buffers of the + // records that partition carried and reports the cumulative + // FlushRecord::FlushSize() bytes freed so far -- the same unit + // DataSyncScan charged -- so releasing quota never lets resident flush + // memory exceed what the quota claims. The progress is a cumulative + // watermark: one wake-up may coalesce several partition completions, and + // a report whose watermark has not advanced is a no-op, which also makes + // duplicate reports harmless. The watermark is capped by this task's + // charge, so a bookkeeping discrepancy can never release another task's + // quota; the remainder -- vector footprints, sizes not covered by + // partition reports, or the whole share when PutAll fails or is skipped + // -- is released by the unconditional DeallocateFlushMemQuota below, + // whose buffers are freed with the flush task right after. The amounts + // always sum to exactly what was taken. // The state is bundled behind a single captured reference so the lambda // fits std::function's small-buffer optimization (16 bytes on libstdc++) // and constructing partition_progress_func does not heap-allocate. @@ -6146,17 +6147,11 @@ void LocalCcShards::FlushDataImpl(FlushDataTask *cur_work, } quota_progress{ data_sync_mem_controller_, cur_work->pending_flush_size_, 0}; const std::function partition_progress_func = - ["a_progress](uint64_t done_bytes, uint64_t total_bytes) + ["a_progress](uint64_t freed_bytes, uint64_t total_bytes) { - if (total_bytes == 0 || done_bytes == 0) - { - return; - } - // 128-bit intermediate: quota and byte totals are both full-width - // uint64 counters, so the product can exceed 64 bits. - const uint64_t target = static_cast( - static_cast(quota_progress.task_flush_quota_) * - done_bytes / total_bytes); + (void) total_bytes; + const uint64_t target = + std::min(freed_bytes, quota_progress.task_flush_quota_); if (target > quota_progress.released_) { quota_progress.mem_controller_.DeallocateFlushMemQuota( @@ -6310,6 +6305,27 @@ void LocalCcShards::FlushDataImpl(FlushDataTask *cur_work, auto ckpt_err = succ ? DataSyncTask::CkptErrorCode::NO_ERROR : DataSyncTask::CkptErrorCode::FLUSH_ERROR; + // Free every remaining flush buffer before returning the remainder of the + // quota, so the release below never precedes the memory it stands for: + // the interleaving vectors (holding the husks of records the partitions + // already freed), the records the per-partition path did not cover -- + // lower-term discards, deferred and failed shares -- and the archive and + // move-base vectors. This matters most for stores that defer publication + // to PersistKV, where no per-partition release happened and the remainder + // is the task's whole share. Nothing past this point reads the vectors: + // the deferred publication block and the MarkDataStoreWrite scan above + // are their last readers, and PostProcessFlushTaskEntries and task + // finalization consume only the task and txm fields. + for (auto &[_, entries] : flush_task_entries) + { + for (auto &entry : entries) + { + entry->data_sync_vec_.reset(); + entry->archive_vec_.reset(); + entry->mv_base_vec_.reset(); + } + } + // notify waiting data sync scan thread // Whatever the per-partition reports did not cover -- a failed or skipped // PutAll, or integer division remainder. diff --git a/tx_service/tests/CheckpointFlush-Test.cpp b/tx_service/tests/CheckpointFlush-Test.cpp index a0b682072..be9cb8d97 100644 --- a/tx_service/tests/CheckpointFlush-Test.cpp +++ b/tx_service/tests/CheckpointFlush-Test.cpp @@ -846,7 +846,7 @@ TEST_CASE("one partition combines checkpoint entries from one term", EloqDS::PartitionFlushState partition; partition.Reset(/*pid=*/7, /*is_range_partitioned=*/false); - partition.serialized_bytes_ = 99; + partition.charged_mem_bytes_ = 99; partition.AddCkptTsEntry(&first_task, 0, nullptr, 100, 1); partition.AddCkptTsEntry(&first_task, 0, nullptr, 101, 2); partition.AddCkptTsEntry(&second_task, 0, nullptr, 102, 3); @@ -1241,7 +1241,7 @@ TEST_CASE("partition callback reports success and failure exactly once", SECTION("successful partition with no checkpoint entries") { partition.Reset(/*pid=*/3, /*is_range_partitioned=*/false); - partition.serialized_bytes_ = 17; + partition.charged_mem_bytes_ = 17; sync.Reset(); sync.total_partitions_ = 1; sync.total_bytes_ = 17; @@ -1258,10 +1258,13 @@ TEST_CASE("partition callback reports success and failure exactly once", SECTION("failed partition") { partition.Reset(/*pid=*/4, /*is_range_partitioned=*/false); - partition.serialized_bytes_ = 23; + FlushRecord failed_record = MakeObjectRecord( + 9, "failed-payload", RecordStatus::Normal, 109, UINT64_MAX, 0); + const uint64_t failed_charge = failed_record.FlushSize(); + partition.AddFlushRecord(&failed_record); sync.Reset(); sync.total_partitions_ = 1; - sync.total_bytes_ = 23; + sync.total_bytes_ = failed_charge; callback.Reset(&partition, &sync, "table"); result.set_error_code(EloqDS::remote::DataStoreError::WRITE_FAILED); result.set_error_msg("injected write failure"); @@ -1272,7 +1275,126 @@ TEST_CASE("partition callback reports success and failure exactly once", REQUIRE(partition.result.error_code() == EloqDS::remote::DataStoreError::WRITE_FAILED); REQUIRE(sync.completed_partitions_ == 1); - REQUIRE(sync.completed_bytes_ == 23); + REQUIRE(sync.completed_bytes_ == failed_charge); + REQUIRE(failed_record.Payload() == nullptr); + REQUIRE(failed_record.Key().KeyPtr() == nullptr); + } +} + +TEST_CASE("partition completion frees record buffers and returns their charge", + "[checkpoint-flush][partition-memory]") +{ + FlushRecord rec_a = MakeObjectRecord( + 1, "payload-aaaaaaaa", RecordStatus::Normal, 100, UINT64_MAX, 0); + FlushRecord rec_b = MakeObjectRecord(2, + "payload-bbbbbbbbbbbbbbbb", + RecordStatus::Normal, + 101, + UINT64_MAX, + 0); + rec_a.cce_ = reinterpret_cast(uintptr_t{0x10}); + rec_a.post_flush_size_ = 77; + + BlobTxRecord blob_payload; + blob_payload.value_ = "payload-owned-by-flush-record"; + FlushRecord rec_c(TxKey(std::make_unique("3")), + blob_payload, + RecordStatus::Normal, + /*commit_ts=*/102, + /*cce=*/nullptr, + /*post_flush_size=*/0, + /*partition_id=*/0); + REQUIRE_FALSE(rec_c.HoldsVersionedPayload()); + + EloqDS::PartitionFlushState partition; + partition.Reset(/*pid=*/5, /*is_range_partitioned=*/false); + const uint64_t expected_charge = + rec_a.FlushSize() + rec_b.FlushSize() + rec_c.FlushSize(); + REQUIRE(expected_charge > 0); + partition.AddFlushRecord(&rec_a); + partition.AddFlushRecord(&rec_b); + partition.AddFlushRecord(&rec_c); + REQUIRE(partition.charged_mem_bytes_ == expected_charge); + + // Completion frees the key/payload buffers and returns the exact charge. + REQUIRE(partition.ReleaseFlushRecordsMemory() == expected_charge); + REQUIRE(rec_a.Payload() == nullptr); + REQUIRE(rec_b.Payload() == nullptr); + REQUIRE(rec_c.Payload() == nullptr); + REQUIRE(rec_a.Key().KeyPtr() == nullptr); + REQUIRE(rec_c.Key().KeyPtr() == nullptr); + // Metadata needed by later stages survives the release. + REQUIRE(rec_a.cce_ == reinterpret_cast(uintptr_t{0x10})); + REQUIRE(rec_a.commit_ts_ == 100); + REQUIRE(rec_a.post_flush_size_ == 77); + + // Idempotent: a failure report after a success path frees nothing more + // and releases no additional quota. + REQUIRE(partition.ReleaseFlushRecordsMemory() == 0); + REQUIRE(partition.charged_mem_bytes_ == 0); +} + +TEST_CASE( + "PutAll releases record memory only at a per-partition durability " + "boundary", + "[checkpoint-flush][partition-memory][put-all]") +{ + PutAllFixture fixture; + const TableName table{std::string_view("partition_memory"), + TableType::Primary, + TableEngine::EloqKv}; + auto records = std::make_unique>(); + BlobTxRecord blob_payload; + blob_payload.value_ = "checkpoint-owned-payload"; + records->emplace_back(TxKey(std::make_unique("memory-key")), + blob_payload, + RecordStatus::Normal, + /*commit_ts=*/200, + /*cce=*/nullptr, + /*post_flush_size=*/0, + /*partition_id=*/0); + FlushRecord *record = &records->front(); + const uint64_t charge = record->FlushSize(); + + std::unordered_map>> + flush_task; + flush_task["eloqkv_partition_memory"].push_back( + MakeFlushEntry(MakeTaskPtr(table, /*term=*/1), std::move(records))); + + std::vector> progress; + const std::function report_progress = + [&](uint64_t done, uint64_t total) + { progress.emplace_back(done, total); }; + + // Match FlushDataImpl: RocksDB-backed stores do not install partition + // progress because PutAll is not their durability boundary. + const auto *progress_fptr = + fixture.Client().NeedPersistKV() ? nullptr : &report_progress; + REQUIRE(fixture.Client().PutAll(flush_task, + /*yield_fptr=*/nullptr, + /*resume_fptr=*/nullptr, + /*sync_yield_fptr=*/nullptr, + progress_fptr)); + + if (fixture.Client().NeedPersistKV()) + { + // RocksDB-backed stores cannot report partition durability before + // PersistKV, so FlushDataImpl keeps the callback disabled and the + // record buffers alive. + REQUIRE(progress.empty()); + REQUIRE(record->Payload() != nullptr); + REQUIRE(record->Key().KeyPtr() != nullptr); + } + else + { + // EloqStore makes the partition durable in BatchWriteRecords, so the + // callback frees the owned record buffers and reports their exact + // charge before PutAll returns. + REQUIRE(progress == + std::vector>{{charge, charge}}); + REQUIRE(record->Payload() == nullptr); + REQUIRE(record->Key().KeyPtr() == nullptr); } } @@ -1830,7 +1952,7 @@ TEST_CASE("live shard dispatch covers checkpoint and intrusive wait lists", EloqDS::PartitionFlushState partition; partition.Reset(/*pid=*/0, /*is_range_partitioned=*/false); - partition.serialized_bytes_ = 31; + partition.charged_mem_bytes_ = 31; partition.AddCkptTsEntry( &stale_task, /*core_idx=*/0, nullptr, /*commit_ts=*/10, 0); partition.AddCkptTsEntry( From b37bf0e5ba6479721dce946f235898eb04387de4 Mon Sep 17 00:00:00 2001 From: Liang Jeff Chen Date: Sat, 17 Oct 2026 01:44:33 +0800 Subject: [PATCH 5/7] fix(log-service): isolate snapshot I/O and preserve retryability Run snapshot filesystem work on a detached pthread after capturing state on the serialized FSM. Release captured state and cloud DB ownership before notifying braft completion so node join also waits for worker cleanup. Check metadata streams, checkpoint creation, file enumeration and file registration. Sync local RocksDB snapshot metadata and directory entries, and report application snapshot failures with EPERM so braft can retry without moving the node into STATE_ERROR. Preserve the existing checkpoint format, including empty databases. Add shared standalone and parent-build snapshot tests for all four log-state backends, including real-braft retry, shutdown/join and restart coverage. Update snapshot behavior and test documentation. Validation: all four standalone snapshot suites passed (12 cases, 293 assertions), plus the parent-build RocksDB suite (6 cases, 89 assertions). clang-format-18 and git diff --check passed. Full transaction-service tests, clean dependency rebuilds and live cloud-storage integration were not run. --- docs/10-log-service.md | 27 +- eloq_log_service/CMakeLists.txt | 6 + eloq_log_service/README.md | 18 + eloq_log_service/include/log_instance.h | 40 +- eloq_log_service/include/log_state.h | 3 +- .../include/log_state_rocksdb_impl.h | 6 +- eloq_log_service/src/log_instance.cpp | 60 +- .../src/log_state_memory_impl.cpp | 11 +- .../src/log_state_rocksdb_cloud_impl.cpp | 16 +- .../src/log_state_rocksdb_impl.cpp | 70 +- .../test/log_snapshot_stream_test.cpp | 167 ++++ eloq_log_service/test/log_snapshot_test.cpp | 730 ++++++++++++++++++ eloq_log_service/test/snapshot/CMakeLists.txt | 13 + tx_service/tests/CMakeLists.txt | 5 + 14 files changed, 1124 insertions(+), 48 deletions(-) create mode 100644 eloq_log_service/test/log_snapshot_stream_test.cpp create mode 100644 eloq_log_service/test/log_snapshot_test.cpp create mode 100644 eloq_log_service/test/snapshot/CMakeLists.txt diff --git a/docs/10-log-service.md b/docs/10-log-service.md index 9974fe742..e3c583c4d 100644 --- a/docs/10-log-service.md +++ b/docs/10-log-service.md @@ -35,7 +35,8 @@ checkpoint, replay, transaction-recovery, and membership operations through the The log service may be started in process by Data Substrate or reached through configured external endpoints. Both modes use the same protobuf and `LogAgent` boundary. A log group is an independent braft replication group and therefore -an independent availability and throughput unit. +an independent availability and throughput unit. The in-process service also +maintains recovery metadata when transaction WAL is disabled. Each transaction writes its durable record to one selected log group, even when its affected data spans several CC NGs. Consequently, recovery of one CC NG @@ -91,10 +92,24 @@ to distinguish an uncommitted transaction from a committed one. For a committed transaction it can stream the relevant record back for lock-scoped replay; without conclusive evidence, the tx side retains the lock and retries later. -Raft snapshots preserve the log state machine's control and recovery metadata -across restart. Persistent data logs remain governed by `LogState`; snapshot -installation and backend reopening must reconstruct a coherent term, -checkpoint, and staged-operation view before the replica serves as leader. +Raft snapshots preserve the log state machine's control and recovery metadata, +including when transaction WAL is disabled. Local RocksDB snapshots include a +checkpoint of the data logs even for an empty database, and restore uses that +checkpoint together with the metadata. Snapshot installation and backend +reopening reconstruct a coherent term, checkpoint, and staged-operation view +before the replica serves as leader. + +Snapshot metadata is captured on the serialized state machine, then filesystem +work runs on a dedicated pthread. Captured state remains owned until the worker +finishes cleanup, before braft completion permits another snapshot or node +shutdown to finish. Braft retains responsibility for snapshot finalization. + +An application snapshot is publishable only after its complete file set is +written and registered; local RocksDB metadata and directory entries are synced +before publication. Failed application attempts leave the previous snapshot +intact and allow the node to keep serving and retry. They use a non-fatal braft +status; braft's own storage or publication failures may still disable the +replica until restart or recreation. ## Cross-cutting invariants @@ -118,4 +133,4 @@ checkpoint, and staged-operation view before the replica serves as leader. | WAL, replay, checkpoint, recovery, and staged-operation wire contracts are defined in protobuf | `tx_service/tx-log-protos/log.proto` | | Replay records are selected and shipped back to tx recovery services | `eloq_log_service/include/log_shipping_agent.h`; `eloq_log_service/src/log_instance.cpp`; `tx_service/src/fault/log_replay_service.cpp` | | Startup selects in-process or external log-service deployment | `core/src/log_init.cpp`; `build_eloq_log_service.cmake`; `CMakeLists.txt` | -| Replicated writes and replay behavior have focused service tests | `eloq_log_service/test/write_log_test.cpp`; `eloq_log_service/test/async_write_log_test.cpp`; `eloq_log_service/test/rocksdb_test.cpp`; `eloq_log_service/test/log_server_rocksdb_cloud_tests.cpp` | +| Replicated writes, replay and snapshot recovery have focused service tests | `eloq_log_service/test/write_log_test.cpp`; `eloq_log_service/test/async_write_log_test.cpp`; `eloq_log_service/test/rocksdb_test.cpp`; `eloq_log_service/test/log_server_rocksdb_cloud_tests.cpp`; `eloq_log_service/test/log_snapshot_test.cpp`; `eloq_log_service/test/log_snapshot_stream_test.cpp` | diff --git a/eloq_log_service/CMakeLists.txt b/eloq_log_service/CMakeLists.txt index 4754acf6a..9a391622f 100644 --- a/eloq_log_service/CMakeLists.txt +++ b/eloq_log_service/CMakeLists.txt @@ -26,6 +26,7 @@ message(NOTICE "log_service source dir: ${LOG_SOURCE_DIR}") option(TEST_LOG_SERVICE "Test log service" ON) message(NOTICE "TEST_LOG_SERVICE : ${TEST_LOG_SERVICE}") +option(TEST_LOG_SNAPSHOT "Build log snapshot tests (requires Catch2)" OFF) option(BRPC_WITH_GLOG "With glog" ON) message(NOTICE "BRPC_WITH_GLOG : ${BRPC_WITH_GLOG}") @@ -335,6 +336,11 @@ target_include_directories(logservice PUBLIC ${LOG_INCLUDE_DIR}) target_link_libraries(logservice PUBLIC ${LOG_LIB}) +if (TEST_LOG_SNAPSHOT) + enable_testing() + add_subdirectory(test/snapshot) +endif () + if (TEST_LOG_SERVICE) add_executable(launch_sv ${LOG_SOURCE_DIR}/src/launch_sv.cpp) target_link_libraries(launch_sv PUBLIC logservice) diff --git a/eloq_log_service/README.md b/eloq_log_service/README.md index 9ed068ab6..6a94772a3 100644 --- a/eloq_log_service/README.md +++ b/eloq_log_service/README.md @@ -3,3 +3,21 @@ `eloq_log_service/` implements Data Substrate's replicated WAL service. It owns the braft-backed log groups and the server side of the log protocol; transaction nodes use the client and protobuf contract under `tx_service/tx-log-protos/`. The current design, durability boundary and source map are maintained in [`docs/10-log-service.md`](../docs/10-log-service.md). Build integration is defined by `build_eloq_log_service.cmake`, and module tests live under `test/`. + +## Snapshot regression tests + +The selected `WITH_LOG_STATE` backend's snapshot suite is included in a parent +build configured with `WITH_TESTS=ON` and `WITH_LOG_SERVICE=ON`: + +```sh +cmake --build bld --target log_snapshot_tests +ctest --test-dir bld -R '^log_snapshot_tests$' --output-on-failure +``` + +For a standalone `eloq_log_service` configure, set `TEST_LOG_SNAPSHOT=ON` to +build the same suite. This option defaults to OFF; the default local RocksDB +`TEST_LOG_SERVICE` build does not require Catch2. + +The suites use temporary storage. Local RocksDB instance tests allocate +loopback ports and cover snapshot retry, shutdown/join and restart. Cloud +stream tests do not contact object storage. diff --git a/eloq_log_service/include/log_instance.h b/eloq_log_service/include/log_instance.h index 5e2ec53bc..f7957bae5 100644 --- a/eloq_log_service/include/log_instance.h +++ b/eloq_log_service/include/log_instance.h @@ -605,19 +605,41 @@ class LogInstance : public braft::StateMachine LogResponse *response); }; +/** + * Owns the captured snapshot until its worker finishes. + * + * The referenced state, writer, and completion belong to the LogInstance and + * braft and outlive this task because Node::join waits for the completion + * callback. Every pointer defaults to null so that a closure destroyed before + * on_snapshot_save finished wiring it is harmless. + */ struct SnapshotClosure { - LogState *log_state_ref; - LogInstance *log_instance_ref; - braft::SnapshotWriter *writer; - braft::Closure *done; + /** + * Releases the captured snapshot state, if any was attached. + * @note Runs before the completion is notified: completion permits another + * snapshot or destruction of the LogInstance after node join. + */ + ~SnapshotClosure() + { + if (log_state_ref != nullptr) + { + log_state_ref->CleanSnapshotState(); + } + } + + LogState *log_state_ref{nullptr}; ///< state being snapshotted + LogInstance *log_instance_ref{nullptr}; ///< owning raft state machine + braft::SnapshotWriter *writer{nullptr}; ///< braft's writer for this save + braft::Closure *done{nullptr}; ///< braft's completion callback #if defined(LOG_STATE_TYPE_RKDB_CLOUD) - // copy the dbc_ from log_state at the time of on_snapshot_save to prevent - // leader step down during the save_snapshot + /** + * Copy of the log state's cloud DB container taken at on_snapshot_save, + * so that a leader step-down during save_snapshot cannot release it. + */ std::shared_ptr dbc_flush; - // leader term when on_snapshot_save - int64_t leader_term; - uint64_t last_tx_num; + int64_t leader_term{-1}; ///< leader term at on_snapshot_save, -1 if none + uint64_t last_tx_num{0}; ///< last applied tx number at on_snapshot_save #endif }; diff --git a/eloq_log_service/include/log_state.h b/eloq_log_service/include/log_state.h index c61ec7949..e55ec855f 100644 --- a/eloq_log_service/include/log_state.h +++ b/eloq_log_service/include/log_state.h @@ -945,7 +945,8 @@ class LogState /** * write snapshot files to snapshot_path and return the filenames in - * relative path + * relative path. An empty result indicates failure; a successful snapshot + * always contains the metadata file, even if there are no data logs. */ virtual std::vector WriteSnapshot( const std::string &snapshot_path) = 0; diff --git a/eloq_log_service/include/log_state_rocksdb_impl.h b/eloq_log_service/include/log_state_rocksdb_impl.h index 70eb22053..a52a7916a 100644 --- a/eloq_log_service/include/log_state_rocksdb_impl.h +++ b/eloq_log_service/include/log_state_rocksdb_impl.h @@ -333,7 +333,8 @@ class LogStateRocksDBImpl : public LogState const std::vector &files) override; /** - * write snapshot files to snapshot_path and return the relative filenames + * Write recovery metadata and a RocksDB checkpoint on a dedicated pthread. + * Return relative filenames on success, an empty vector on I/O failure. * @param snapshot_path * @return */ @@ -695,7 +696,8 @@ class LogStateRocksDBCloudImpl : public LogState const std::vector &files) override; /** - * write snapshot files to snapshot_path and return the relative filenames + * Write snapshot files and return their relative filenames. Return an + * empty vector if either output stream fails, including on close. * @param snapshot_path * @return */ diff --git a/eloq_log_service/src/log_instance.cpp b/eloq_log_service/src/log_instance.cpp index 97dec2ed6..22f6e3739 100644 --- a/eloq_log_service/src/log_instance.cpp +++ b/eloq_log_service/src/log_instance.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -1143,9 +1144,9 @@ void LogInstance::LogGroupLeaderUpdate() void LogInstance::on_snapshot_save(braft::SnapshotWriter *writer, braft::Closure *done) { - // Save current StateMachine in memory and starts a new bthread to avoid - // blocking StateMachine since it's a bit slow to write data to disk - // file. + // Capture metadata on the serialized FSM before allowing more on_apply + // calls. Filesystem work, including RocksDB's synchronous checkpoint fsync, + // must run on an OS thread rather than pinning a shared brpc worker. auto start = std::chrono::high_resolution_clock::now(); SnapshotClosure *sc = new SnapshotClosure; sc->writer = writer; @@ -1178,24 +1179,62 @@ void LogInstance::on_snapshot_save(braft::SnapshotWriter *writer, std::chrono::duration_cast(stop - start) .count(); LOG(INFO) << "log server generate snapshot cost " << ms << " millseconds"; - bthread_t tid; - bthread_start_urgent(&tid, NULL, save_snapshot, sc); + pthread_attr_t attr; + int rc = pthread_attr_init(&attr); + if (rc == 0) + { + rc = pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); + if (rc == 0) + { + pthread_t tid; + rc = pthread_create(&tid, &attr, save_snapshot, sc); + } + pthread_attr_destroy(&attr); + } + if (rc != 0) + { + // There is no worker to own sc on failure. Never fall back to doing + // blocking I/O on the FSM. braft can retry a failed snapshot later. + brpc::ClosureGuard done_guard(done); + std::unique_ptr sc_guard(sc); + done->status().set_error(rc, "Failed to start snapshot pthread"); + } } void *LogInstance::save_snapshot(void *arg) { - LOG(INFO) << "Save snapshot start"; + pthread_setname_np(pthread_self(), "log_snapshot"); + LOG(INFO) << "Save snapshot start on dedicated pthread"; SnapshotClosure *sc = static_cast(arg); - std::unique_ptr sc_guard(sc); brpc::ClosureGuard done_guard(sc->done); + // braft Node::join waits for done. Destroy sc (and all borrowed state) + // before invoking it, and never touch the LogInstance after completion. + std::unique_ptr sc_guard(sc); const std::string &snapshot_path = sc->writer->get_path(); + // Error-code choice: braft treats EIO from on_snapshot_save as a storage + // failure, calls report_error, and moves the node to STATE_ERROR for good. + // A failed checkpoint or metadata write should instead fail only this + // attempt and leave the node serving writes; braft discards the temp + // directory and retries at the next snapshot interval. EPERM, like the + // cloud flush path below, is reported but is not fatal to the node. auto snapshot_files = sc->log_state_ref->WriteSnapshot(snapshot_path); + if (snapshot_files.empty()) + { + sc->done->status().set_error(EPERM, "Failed to write snapshot files"); + return nullptr; + } for (const auto &file_path : snapshot_files) { LOG(INFO) << "adding snapshot file: " << file_path; - CHECK_EQ(0, sc->writer->add_file(file_path)); + if (sc->writer->add_file(file_path) != 0) + { + sc->done->status().set_error(EPERM, + "Failed to register snapshot file %s", + file_path.c_str()); + return nullptr; + } } #if defined(LOG_STATE_TYPE_RKDB_CLOUD) @@ -1218,7 +1257,6 @@ void *LogInstance::save_snapshot(void *arg) { // rocksdb cloud rolling up timeout sc->done->status().set_error(EPERM, "Flush RocksDB Cloud failed"); - sc->log_state_ref->CleanSnapshotState(); return nullptr; } @@ -1238,7 +1276,6 @@ void *LogInstance::save_snapshot(void *arg) LOG(ERROR) << "Flush RocksDB Cloud Failed! Error: " << status.ToString(); sc->done->status().set_error(EPERM, "Flush RocksDB Cloud failed"); - sc->log_state_ref->CleanSnapshotState(); return nullptr; } uint64_t max_sst_file_num = @@ -1264,7 +1301,6 @@ void *LogInstance::save_snapshot(void *arg) LOG(ERROR) << "Failed to serialize the PostSnapshotRequest request"; sc->done->status().set_error( EPERM, "Failed to serialize the PostSnapshotRequest request"); - sc->log_state_ref->CleanSnapshotState(); return nullptr; } else @@ -1294,7 +1330,6 @@ void *LogInstance::save_snapshot(void *arg) LOG(ERROR) << "Failed to apply PostSnapshotRequest request"; sc->done->status().set_error( EPERM, "Failed to apply PostSnapshotRequest request"); - sc->log_state_ref->CleanSnapshotState(); return nullptr; } @@ -1309,7 +1344,6 @@ void *LogInstance::save_snapshot(void *arg) } #endif - sc->log_state_ref->CleanSnapshotState(); LOG(INFO) << "Save snapshot done"; return nullptr; } diff --git a/eloq_log_service/src/log_state_memory_impl.cpp b/eloq_log_service/src/log_state_memory_impl.cpp index 1313f7927..5e4743b10 100644 --- a/eloq_log_service/src/log_state_memory_impl.cpp +++ b/eloq_log_service/src/log_state_memory_impl.cpp @@ -115,7 +115,14 @@ std::vector LogStateMemoryImpl::WriteSnapshot( std::ofstream os(path.c_str(), std::ios::binary); WriteSnapshotNgInfoAndCatalogOpsTo(os); WriteSnapshotStatesTo(os); - } // close file + // Check after close as well: buffered output may fail during flush. + os.close(); + if (!os) + { + LOG(ERROR) << "Failed to write snapshot: " << path; + return {}; + } + } return std::vector{"data"}; } @@ -243,4 +250,4 @@ void LogStateMemoryImpl::WriteSnapshotStatesTo(std::ofstream &os) } } } // namespace txlog -#endif \ No newline at end of file +#endif diff --git a/eloq_log_service/src/log_state_rocksdb_cloud_impl.cpp b/eloq_log_service/src/log_state_rocksdb_cloud_impl.cpp index ded7e55a9..431eec71a 100644 --- a/eloq_log_service/src/log_state_rocksdb_cloud_impl.cpp +++ b/eloq_log_service/src/log_state_rocksdb_cloud_impl.cpp @@ -1210,13 +1210,27 @@ std::vector LogStateRocksDBCloudImpl::WriteSnapshot( auto path = snapshot_path + "/nginfo_and_catalog"; std::ofstream os(path.c_str()); WriteSnapshotNgInfoAndCatalogOpsTo(os); - } // close file + // Buffered write errors may surface only when close flushes them. + // Do not publish a file list until both streams close successfully. + os.close(); + if (!os) + { + LOG(ERROR) << "Failed to write snapshot metadata: " << path; + return {}; + } + } res.emplace_back("nginfo_and_catalog"); { auto path = snapshot_path + "/in_mem_data_log"; std::ofstream os(path.c_str()); WriteSnapshotInMemState(os); + os.close(); + if (!os) + { + LOG(ERROR) << "Failed to write snapshot data logs: " << path; + return {}; + } } res.emplace_back("in_mem_data_log"); } diff --git a/eloq_log_service/src/log_state_rocksdb_impl.cpp b/eloq_log_service/src/log_state_rocksdb_impl.cpp index d5b15f9be..e12b36487 100644 --- a/eloq_log_service/src/log_state_rocksdb_impl.cpp +++ b/eloq_log_service/src/log_state_rocksdb_impl.cpp @@ -1,10 +1,12 @@ #include "log_state_rocksdb_impl.h" #if defined(LOG_STATE_TYPE_RKDB) +#include #include #include #include #include +#include #include #include @@ -19,6 +21,35 @@ namespace txlog { +namespace +{ +/** + * Flushes a snapshot file or directory to stable storage. + * @param path File or directory to fsync; a directory persists its entries. + * @return true on success, false after logging the failing call. + */ +bool SyncSnapshotPath(const std::string &path) +{ + int fd = open(path.c_str(), O_RDONLY | O_CLOEXEC); + if (fd < 0) + { + PLOG(ERROR) << "Open snapshot for sync failed: " << path; + return false; + } + int rc; + do + { + rc = fsync(fd); + } while (rc != 0 && errno == EINTR); + if (rc != 0) + { + PLOG(ERROR) << "Sync snapshot failed: " << path; + } + close(fd); + return rc == 0; +} +} // namespace + LogStateRocksDBImpl::LogStateRocksDBImpl(const std::string &rocksdb_path, const size_t sst_files_size_limit, const size_t rocksdb_scan_threads) @@ -239,10 +270,14 @@ std::vector LogStateRocksDBImpl::WriteSnapshot( { // write ng terms to file 'data' auto path = snapshot_path + "/data"; + std::ofstream os(path.c_str()); + WriteSnapshotNgInfoAndCatalogOpsTo(os); + os.close(); + if (!os || !SyncSnapshotPath(path)) { - std::ofstream os(path.c_str()); - WriteSnapshotNgInfoAndCatalogOpsTo(os); - } // close file + LOG(ERROR) << "Failed to write snapshot metadata: " << path; + return {}; + } res.emplace_back("data"); } @@ -253,14 +288,8 @@ std::vector LogStateRocksDBImpl::WriteSnapshot( if (!status.ok()) { LOG(ERROR) << "Create checkpoint object failed: " << status.ToString(); + return {}; } - assert(status.ok()); - - // do compaction before checkpoint, so we get smaller checkpoint file? - // rocksdb::CompactRangeOptions options; - // options.change_level = true; - // options.target_level = -1; - // db_->CompactRange(options, nullptr, nullptr); LOG(INFO) << "log state rocksdbImpl doing checkpoint"; std::string ckpt_path = snapshot_path + "/rocksdb_ckpt"; @@ -270,29 +299,42 @@ std::vector LogStateRocksDBImpl::WriteSnapshot( if (error_code.value() != 0) { LOG(ERROR) << error_code.value() << " " << error_code.message(); + return {}; } status = checkpoint->CreateCheckpoint(ckpt_path); if (!status.ok()) { LOG(ERROR) << "Create checkpoint failed: " << status.ToString(); + return {}; } - assert(status.ok()); std::filesystem::directory_iterator dir_ite(ckpt_path, error_code); if (error_code.value() != 0) { LOG(ERROR) << error_code.value() << " " << error_code.message(); + return {}; } else { - for (const auto &entry : std::filesystem::directory_iterator(ckpt_path)) + for (const auto end = std::filesystem::directory_iterator(); + dir_ite != end; + dir_ite.increment(error_code)) { + // A failed increment leaves the iterator at end; the error is + // checked once after the loop. // add relative path std::string path("rocksdb_ckpt/"); - res.emplace_back(path.append(entry.path().filename())); + res.emplace_back(path.append(dir_ite->path().filename())); + } + if (error_code) + { + LOG(ERROR) << "List checkpoint failed: " << error_code.message(); + return {}; } } - return res; + // braft syncs its manifest, not application files. Persist our directory + // entries before handing the file list to braft for snapshot publication. + return SyncSnapshotPath(snapshot_path) ? res : std::vector{}; } /** diff --git a/eloq_log_service/test/log_snapshot_stream_test.cpp b/eloq_log_service/test/log_snapshot_stream_test.cpp new file mode 100644 index 000000000..9d407e25d --- /dev/null +++ b/eloq_log_service/test/log_snapshot_stream_test.cpp @@ -0,0 +1,167 @@ +#define CATCH_CONFIG_PREFIX_ALL + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "log_state.h" +#include "log_state_memory_impl.h" +#include "log_state_rocksdb_impl.h" + +namespace +{ +namespace fs = std::filesystem; + +/** + * A log state of the configured backend in a temporary directory, optionally + * seeded with a node-group term, a data log and a schema op. + */ +class SnapshotFixture +{ +public: + /** + * @param payload_size Size of the seeded data log and table name; zero + * leaves the state empty. + */ + explicit SnapshotFixture(size_t payload_size = 0) + { + char name[] = "/tmp/eloq-log-stream-XXXXXX"; + char *created = mkdtemp(name); + CATCH_REQUIRE(created != nullptr); + path_ = created; +#if defined(LOG_STATE_TYPE_RKDB_CLOUD) + // Exercise the follower's local snapshot streams without opening a + // cloud DB or contacting object storage. The leader term reference + // outlives the state, including its background purger threads. + txlog::RocksDBCloudConfig config{}; + state_ = std::make_unique( + (path_ / "db").string(), + config, + term_, + nullptr, + 1024, + 8, + 8, + 64 * 1024 * 1024, + 1); + files_ = {"nginfo_and_catalog", "in_mem_data_log"}; +#elif defined(LOG_STATE_TYPE_MEM) + state_ = std::make_unique(); + files_ = {"data"}; +#else +#error "Stream tests require MEMORY or a RocksDB Cloud log state" +#endif + fs::create_directory(path_ / "snapshot"); + if (payload_size == 0) + { + return; + } + CATCH_REQUIRE(state_->UpdateNgTerm(1, 7, "127.0.0.1", 9000)); + const std::string payload(payload_size, 'x'); + state_->AddLogItem(1, 99, 100, payload); + txlog::SchemaOpMessage schema; + schema.set_table_name_str(payload); + schema.set_table_type(txlog::CcTableType::Primary); + schema.set_stage(txlog::SchemaOpMessage::PrepareSchema); + state_->UpsertSchemaOp(123, 50, schema); + } + + ~SnapshotFixture() + { + state_.reset(); + std::error_code error; + fs::remove_all(path_, error); + } + + /** + * Runs a full snapshot cycle outside braft. + * @param path Directory to write into; created if missing. + * @return The relative file names WriteSnapshot reported, empty on failure. + */ + std::vector Save(const fs::path &path) + { + fs::create_directories(path); + state_->BeginSnapshot(); + auto files = state_->WriteSnapshot(path); + state_->CleanSnapshotState(); + return files; + } + + fs::path path_; ///< temporary directory + std::atomic term_{-1}; ///< leader term for cloud + std::unique_ptr state_; ///< state under test + std::vector files_; ///< expected snapshot files +}; + +/** + * @param path File to read. + * @return The whole file as bytes; fails the test if it cannot be opened. + */ +std::string ReadFile(const fs::path &path) +{ + std::ifstream input(path, std::ios::binary); + CATCH_REQUIRE(input.is_open()); + return {std::istreambuf_iterator(input), + std::istreambuf_iterator()}; +} + +CATCH_TEST_CASE("Snapshot stream output preserves metadata and data logs") +{ + SnapshotFixture source(GENERATE(size_t{4}, size_t{16384})); + auto files = source.Save(source.path_ / "snapshot"); + CATCH_REQUIRE(files == source.files_); + // A fresh follower can restore and serialize the same complete file set. + SnapshotFixture restored; + CATCH_REQUIRE( + restored.state_->ReadSnapshot(source.path_ / "snapshot", files) == 0); + CATCH_REQUIRE(restored.state_->GetNgLeaderTerm(1) == 7); + CATCH_REQUIRE(restored.state_->SearchTxSchemaLog(123).first); + CATCH_REQUIRE(restored.Save(restored.path_ / "roundtrip") == files); + for (const auto &file : files) + { + auto original = ReadFile(source.path_ / "snapshot" / file); + CATCH_REQUIRE_FALSE(original.empty()); + CATCH_REQUIRE(ReadFile(restored.path_ / "roundtrip" / file) == + original); + } +} + +CATCH_TEST_CASE( + "Snapshot stream errors reject the entire file set and allow retry") +{ + SnapshotFixture fixture(GENERATE(size_t{4}, size_t{16384})); +#if defined(LOG_STATE_TYPE_RKDB_CLOUD) + const auto file = GENERATE("nginfo_and_catalog", "in_mem_data_log"); +#else + const auto file = "data"; +#endif + const auto failed_path = fixture.path_ / "snapshot" / file; + CATCH_SECTION("File cannot be opened") + { + fs::create_directory(failed_path); + } + CATCH_SECTION("Write or buffered close fails with ENOSPC") + { + // /dev/full opens successfully but rejects output. Small payloads + // exercise buffered close; large payloads also fail during writing. + // A symlink confines this injection to this fixture's output path. + fs::create_symlink("/dev/full", failed_path); + } + CATCH_REQUIRE(fixture.Save(fixture.path_ / "snapshot").empty()); + fs::remove(failed_path); + CATCH_REQUIRE(fixture.Save(fixture.path_ / "snapshot") == fixture.files_); + for (const auto &name : fixture.files_) + { + CATCH_REQUIRE(fs::file_size(fixture.path_ / "snapshot" / name) > 0); + } +} +} // namespace diff --git a/eloq_log_service/test/log_snapshot_test.cpp b/eloq_log_service/test/log_snapshot_test.cpp new file mode 100644 index 000000000..a840ab0d1 --- /dev/null +++ b/eloq_log_service/test/log_snapshot_test.cpp @@ -0,0 +1,730 @@ +#define CATCH_CONFIG_PREFIX_ALL + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "log_instance.h" +#include "log_state_rocksdb_impl.h" + +/** + * Makes the next pthread_create issued by this test thread fail with EAGAIN. + * Thread-local so that brpc and RocksDB threads are unaffected by the injected + * resource exhaustion. + */ +thread_local bool fail_snapshot_pthread = false; +namespace +{ +/** + * Handshake that stalls the snapshot worker inside the fsync of the snapshot + * metadata file until the test releases it. + * + * Matches the metadata inode as well as the worker thread name: static braft + * and RocksDB libraries also call the wrapped fsync, and they must not consume + * this stall or make the shutdown test pass by blocking unrelated I/O. + */ +struct SnapshotSyncGate +{ + /** + * @param path Path of the snapshot metadata file whose fsync is gated. + */ + explicit SnapshotSyncGate(std::string path) : metadata_path(std::move(path)) + { + } + + /** + * @param fd Descriptor passed to fsync. + * @return true when @p fd refers to the gated metadata file. + */ + bool Matches(int fd) const + { + struct stat actual; + struct stat expected; + return fstat(fd, &actual) == 0 && + stat(metadata_path.c_str(), &expected) == 0 && + actual.st_dev == expected.st_dev && + actual.st_ino == expected.st_ino; + } + + /** Blocks the calling worker until Release(), recording its thread kind. */ + void Pause() + { + std::unique_lock lock(mutex); + on_pthread = bthread_self() == 0; + paused = true; + changed.notify_all(); + changed.wait(lock, [&] { return released; }); + } + + /** + * Waits up to ten seconds for a worker to reach Pause(). + * @return true if a worker is paused, false on timeout. + */ + bool WaitUntilPaused() + { + std::unique_lock lock(mutex); + return changed.wait_for( + lock, std::chrono::seconds(10), [&] { return paused; }); + } + + /** Lets a paused worker, or any later one, continue past the gate. */ + void Release() + { + std::lock_guard lock(mutex); + released = true; + changed.notify_all(); + } + + const std::string metadata_path; ///< gated snapshot metadata file + std::mutex mutex; ///< guards the flags below + std::condition_variable changed; ///< signalled on pause and release + bool paused = false; ///< a worker reached Pause() + bool released = false; ///< Release() has been called + bool on_pthread = false; ///< the paused worker was not a bthread + std::atomic sync_completed{false}; ///< the gated fsync returned +}; + +/** Gate consulted by the fsync wrapper; null when no test is gating. */ +std::atomic snapshot_sync_gate{nullptr}; + +/** + * While set, every fsync issued from the snapshot worker thread fails with + * EIO. This makes LogStateRocksDBImpl::WriteSnapshot fail on the metadata + * file after braft has already created the temp snapshot directory, which a + * fault planted on disk beforehand cannot do: braft wipes that directory on + * each attempt. fsyncs from braft's own threads are unaffected. + */ +std::atomic fail_snapshot_fsync{false}; +} // namespace + +/** The real fsync, reached through the linker's --wrap=fsync. */ +extern "C" int __real_fsync(int); + +/** + * Link-time replacement for fsync: on the snapshot worker thread it applies + * the injected failure or the pause gate, otherwise it forwards unchanged. + * @param fd Descriptor being synced. + * @return fsync's result, or -1 with errno EIO while the failure is injected. + */ +extern "C" int __wrap_fsync(int fd) +{ + char name[16]{}; + pthread_getname_np(pthread_self(), name, sizeof(name)); + if (std::string(name) == "log_snapshot") + { + if (fail_snapshot_fsync.load()) + { + errno = EIO; + return -1; + } + auto *gate = snapshot_sync_gate.load(); + if (gate != nullptr && gate->Matches(fd)) + { + gate->Pause(); + const int rc = __real_fsync(fd); + gate->sync_completed.store(true); + return rc; + } + } + return __real_fsync(fd); +} +/** The real pthread_create, reached through the linker's --wrap. */ +extern "C" int __real_pthread_create(pthread_t *, + const pthread_attr_t *, + void *(*) (void *), + void *); + +/** + * Link-time replacement for pthread_create that fails once with EAGAIN when + * fail_snapshot_pthread is set on the calling thread. + * @return EAGAIN for the injected failure, otherwise pthread_create's result. + */ +extern "C" int __wrap_pthread_create(pthread_t *tid, + const pthread_attr_t *attr, + void *(*fn)(void *), + void *arg) +{ + if (fail_snapshot_pthread) + { + fail_snapshot_pthread = false; + return EAGAIN; + } + return __real_pthread_create(tid, attr, fn, arg); +} + +namespace +{ +namespace fs = std::filesystem; +using txlog::LogStateRocksDBImpl; +using namespace std::chrono_literals; +constexpr size_t kSstLimit = 64 * 1024 * 1024; + +/** A fresh directory under /tmp that is removed with the object. */ +struct TempDirectory +{ + TempDirectory() + { + char name[] = "/tmp/eloq-log-snapshot-XXXXXX"; + char *created = mkdtemp(name); + CATCH_REQUIRE(created != nullptr); + path = created; + } + ~TempDirectory() + { + std::error_code ec; + fs::remove_all(path, ec); + } + fs::path path; ///< the created directory +}; + +/** + * A brpc server on an ephemeral loopback port with braft's services added. + * + * braft needs the concrete endpoint before brpc starts listening. The + * constructor asks the kernel for an ephemeral port, then retries if another + * process claims it in the short interval between releasing the reservation + * and starting brpc. + */ +struct LoopbackRaftServer +{ + LoopbackRaftServer() + { + for (int attempt = 0; attempt < 10; ++attempt) + { + butil::fd_guard reservation(socket(AF_INET, SOCK_STREAM, 0)); + CATCH_REQUIRE(reservation >= 0); + sockaddr_in address{}; + address.sin_family = AF_INET; + address.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + CATCH_REQUIRE(bind(reservation, + reinterpret_cast(&address), + sizeof(address)) == 0); + socklen_t size = sizeof(address); + CATCH_REQUIRE(getsockname(reservation, + reinterpret_cast(&address), + &size) == 0); + port = ntohs(address.sin_port); + endpoint = "127.0.0.1:" + std::to_string(port); + server = std::make_unique(); + CATCH_REQUIRE(braft::add_service(server.get(), endpoint.c_str()) == + 0); + reservation.reset(-1); + if (server->Start(endpoint.c_str(), nullptr) == 0) + { + return; + } + server.reset(); + } + CATCH_FAIL( + "Could not start a loopback Raft server on an ephemeral port"); + } + + std::unique_ptr server; ///< the running server + std::string endpoint; ///< "127.0.0.1:" + int port = 0; ///< the bound port +}; + +/** + * Runs a full snapshot cycle on a log state outside braft. + * @param state State to snapshot. + * @param path Directory to write into; created if missing. + * @return The relative file names WriteSnapshot reported, empty on failure. + */ +std::vector Save(LogStateRocksDBImpl &state, const fs::path &path) +{ + fs::create_directories(path); + state.BeginSnapshot(); + auto files = state.WriteSnapshot(path); + state.CleanSnapshotState(); + return files; +} + +/** + * @param files Snapshot file list. + * @param name Relative file name to look for. + * @return true if @p name is in @p files. + */ +bool Has(const std::vector &files, const std::string &name) +{ + return std::find(files.begin(), files.end(), name) != files.end(); +} + +CATCH_TEST_CASE( + "Empty log snapshot preserves metadata and replaces stale records") +{ + TempDirectory dir; + LogStateRocksDBImpl source(dir.path / "source", kSstLimit, 1); + CATCH_REQUIRE(source.Start() == 0); + CATCH_REQUIRE(source.UpdateNgTerm(1, 7, "127.0.0.1", 9000)); + source.UpdateCkptTs(1, 40); + txlog::SchemaOpMessage schema; + schema.set_table_name_str("snapshot-test"); + schema.set_table_type(txlog::CcTableType::Primary); + schema.set_stage(txlog::SchemaOpMessage::PrepareSchema); + source.UpsertSchemaOp(123, 50, schema); + auto files = Save(source, dir.path / "snapshot"); + CATCH_REQUIRE(Has(files, "data")); + CATCH_REQUIRE(Has(files, "rocksdb_ckpt/CURRENT")); + + LogStateRocksDBImpl fresh(dir.path / "fresh", kSstLimit, 1); + CATCH_REQUIRE(fresh.Start() == 0); + CATCH_REQUIRE(fresh.ReadSnapshot(dir.path / "snapshot", files) == 0); + CATCH_REQUIRE(fresh.GetNgLeaderTerm(1) == 7); + + { + LogStateRocksDBImpl restored(dir.path / "restored", kSstLimit, 1); + CATCH_REQUIRE(restored.Start() == 0); + restored.AddLogItem(1, 99, 100, "stale record"); + CATCH_REQUIRE(restored.ReadSnapshot(dir.path / "snapshot", files) == 0); + CATCH_REQUIRE(restored.GetNgLeaderTerm(1) == 7); + CATCH_REQUIRE(restored.LastCkptTimestamp(1) == 40); + CATCH_REQUIRE(restored.SearchTxSchemaLog(123).first); + CATCH_REQUIRE(restored.SearchTxDataLog(99, 1).second == nullptr); + auto second = Save(restored, dir.path / "second"); + CATCH_REQUIRE(Has(second, "rocksdb_ckpt/CURRENT")); + } + // Reopen the rebuilt DB and restore metadata just as Raft restart does. + LogStateRocksDBImpl reopened(dir.path / "restored", kSstLimit, 1); + CATCH_REQUIRE(reopened.Start() == 0); + CATCH_REQUIRE(reopened.ReadSnapshot(dir.path / "snapshot", files) == 0); + CATCH_REQUIRE(reopened.GetNgLeaderTerm(1) == 7); + CATCH_REQUIRE(reopened.SearchTxSchemaLog(123).first); + CATCH_REQUIRE(reopened.SearchTxDataLog(99, 1).second == nullptr); +} + +CATCH_TEST_CASE("Nonempty log snapshots retain the legacy checkpoint format") +{ + TempDirectory dir; + LogStateRocksDBImpl source(dir.path / "source", kSstLimit, 1); + CATCH_REQUIRE(source.Start() == 0); + source.AddLogItemBatch(1, 99, 100, "committed log"); + source.FlushLogItemBatch(); + auto files = Save(source, dir.path / "snapshot"); + CATCH_REQUIRE(Has(files, "rocksdb_ckpt/CURRENT")); + LogStateRocksDBImpl restored(dir.path / "restored", kSstLimit, 1); + CATCH_REQUIRE(restored.Start() == 0); + CATCH_REQUIRE(restored.ReadSnapshot(dir.path / "snapshot", files) == 0); + auto item = restored.SearchTxDataLog(99, 1).second; + CATCH_REQUIRE(item != nullptr); + CATCH_REQUIRE(item->log_message_ == "committed log"); +} + +CATCH_TEST_CASE( + "Snapshot output failures return failure instead of partial success") +{ + TempDirectory dir; + LogStateRocksDBImpl state(dir.path / "db", kSstLimit, 1); + CATCH_REQUIRE(state.Start() == 0); + fs::create_directories(dir.path / "snapshot"); + CATCH_SECTION("Metadata cannot be opened") + { + fs::create_directory(dir.path / "snapshot/data"); + } + CATCH_SECTION("Checkpoint cannot be created, including for an empty DB") + { + // RocksDB uses this staging directory; an existing nonempty staging + // subdirectory makes CreateCheckpoint fail without depending on chmod. + fs::create_directories(dir.path / "snapshot/rocksdb_ckpt.tmp/blocker"); + std::ofstream(dir.path / "snapshot/rocksdb_ckpt.tmp/blocker/child") + << "x"; + } + CATCH_REQUIRE(Save(state, dir.path / "snapshot").empty()); + CATCH_REQUIRE_FALSE(Save(state, dir.path / "retry").empty()); +} + +/** + * A braft snapshot writer that records registered files and the thread that + * registered them, with an optional injected registration failure. + */ +class RecordingWriter : public braft::SnapshotWriter +{ +public: + explicit RecordingWriter(const fs::path &path) : path_(path) + { + fs::create_directories(path_); + } + std::string get_path() override + { + return path_; + } + void list_files(std::vector *files) override + { + *files = files_; + } + int save_meta(const braft::SnapshotMeta &) override + { + return 0; + } + int remove_file(const std::string &) override + { + return 0; + } + int add_file(const std::string &file, + const google::protobuf::Message *) override + { + on_pthread &= bthread_self() == 0; + char name[16]{}; + pthread_getname_np(pthread_self(), name, sizeof(name)); + named_worker &= std::string(name) == "log_snapshot"; + if (fail_registration) + { + return -1; + } + files_.push_back(file); + return 0; + } + bool fail_registration = false; ///< make add_file fail + bool on_pthread = true; ///< no add_file came from a bthread + bool named_worker = true; ///< every add_file came from the worker + std::vector files_; ///< files registered so far + +private: + std::string path_; ///< snapshot directory handed to the state machine +}; + +/** A braft closure that publishes its final status through a promise. */ +class Completion : public braft::Closure +{ +public: + void Run() override + { + result.set_value(status()); + } + std::promise result; ///< fulfilled with the status +}; + +CATCH_TEST_CASE( + "LogInstance completes snapshots on a dedicated pthread and retries " + "failures") +{ + TempDirectory dir; + LoopbackRaftServer server; + const auto &endpoint = server.endpoint; + txlog::LogInstance instance(0, + 0, + "127.0.0.1", + server.port, + endpoint + ":0", + "snapshot-test", + "local://" + (dir.path / "raft").string(), +#ifdef WITH_CLOUD_AZ_INFO + "", + "", +#endif + dir.path / "db", + 1, + 0, + 8, + 12, + 64 * 1024 * 1024, + 0, + false, + 10, + 128 * 1024 * 1024); + CATCH_REQUIRE(instance.Start() == 0); + braft::StateMachine &fsm = instance; + for (int attempt = 0; attempt != 4; ++attempt) + { + RecordingWriter writer(dir.path / std::to_string(attempt)); + Completion done; + auto result = done.result.get_future(); + if (attempt == 0) + { + fail_snapshot_pthread = true; + } + writer.fail_registration = attempt == 1; + if (attempt == 2) + { + fs::create_directory(dir.path / "2/data"); + } + fsm.on_snapshot_save(&writer, &done); + CATCH_REQUIRE(result.wait_for(10s) == std::future_status::ready); + auto status = result.get(); + if (attempt < 3) + { + CATCH_REQUIRE_FALSE(status.ok()); + // Only a pthread start failure reports its errno; write and + // registration failures use EPERM so braft keeps the node alive. + CATCH_REQUIRE(status.error_code() == + (attempt == 0 ? EAGAIN : EPERM)); + } + else + { + CATCH_REQUIRE(status.ok()); + CATCH_REQUIRE(writer.on_pthread); + CATCH_REQUIRE(writer.named_worker); + CATCH_REQUIRE(Has(writer.files_, "rocksdb_ckpt/CURRENT")); + } + } + instance.shutdown(); + instance.join(); +} + +CATCH_TEST_CASE( + "Raft shutdown waits for snapshot I/O and restart loads the empty snapshot") +{ + TempDirectory dir; + LoopbackRaftServer server; + const auto &endpoint = server.endpoint; + auto make_instance = [&] + { + return std::make_unique( + 0, + 0, + "127.0.0.1", + server.port, + endpoint + ":0", + "snapshot-shutdown-test", + "local://" + (dir.path / "raft").string(), +#ifdef WITH_CLOUD_AZ_INFO + "", + "", +#endif + dir.path / "db", + 1, + 0, + 8, + 12, + 64 * 1024 * 1024, + 0, + false, + 10, + 128 * 1024 * 1024); + }; + auto instance = make_instance(); + CATCH_REQUIRE(instance->Start() == 0); + const auto deadline = std::chrono::steady_clock::now() + 10s; + while (!instance->is_leader() && + std::chrono::steady_clock::now() < deadline) + { + std::this_thread::sleep_for(10ms); + } + CATCH_REQUIRE(instance->is_leader()); + + braft::PeerId peer; + CATCH_REQUIRE(peer.parse(endpoint + ":0") == 0); + braft::cli::CliOptions options; + options.timeout_ms = 10000; + options.max_retry = 0; + SnapshotSyncGate gate((dir.path / "raft/snapshot/temp/data").string()); + snapshot_sync_gate.store(&gate); + auto snapshot = std::async(std::launch::async, + [&] { + return braft::cli::snapshot( + "snapshot-shutdown-test", peer, options); + }); + const bool observed_pause = gate.WaitUntilPaused(); + instance->shutdown(); + std::promise join_started; + auto joining = join_started.get_future(); + auto joined = std::async( + std::launch::async, + [&] + { + join_started.set_value(); + instance->join(); + // Check the state at join's return, before waiting for the snapshot + // RPC. Publication and metadata I/O must already be complete then. + bool published = false; + for (const auto &entry : + fs::directory_iterator(dir.path / "raft/snapshot")) + { + if (entry.path().filename() != "temp" && + fs::exists(entry.path() / "rocksdb_ckpt/CURRENT")) + { + published = true; + } + } + return gate.sync_completed.load() && published; + }); + joining.wait(); + // The handshake arranges shutdown with snapshot I/O in flight. Assert + // completion ordering, without requiring join to take a minimum duration. + // Release before assertions or future cleanup, including on a pause + // timeout. + gate.Release(); + const bool completed_before_join_returned = joined.get(); + auto status = snapshot.get(); + snapshot_sync_gate.store(nullptr); + CATCH_REQUIRE(observed_pause); + CATCH_REQUIRE(gate.on_pthread); + CATCH_REQUIRE(completed_before_join_returned); + CATCH_REQUIRE(status.ok()); + instance.reset(); + + bool has_checkpoint_snapshot = false; + for (const auto &entry : fs::directory_iterator(dir.path / "raft/snapshot")) + { + if (fs::exists(entry.path() / "rocksdb_ckpt/CURRENT")) + { + has_checkpoint_snapshot = true; + CATCH_REQUIRE(fs::exists(entry.path() / "data")); + } + } + CATCH_REQUIRE(has_checkpoint_snapshot); + // Start goes through braft's actual on_snapshot_load callback. + instance = make_instance(); + CATCH_REQUIRE(instance->Start() == 0); + instance->shutdown(); + instance->join(); +} + +constexpr uint32_t kNgId = 1; ///< cc node group used by the write tests +constexpr int64_t kNgTerm = 7; ///< term registered for kNgId via raft + +/** + * Proposes a committed data log through the leader's public WriteLog path. + * The write passes the term checks in LogInstance::execute only while the + * node is still the leader and kNgId's term is kNgTerm in the log state. + * @param instance The log group leader. + * @param txn Low bits of the tx number; also used as the commit timestamp. + * @return The response status reported by the raft-applied write. + */ +txlog::LogResponse::ResponseStatus WriteDataLog(txlog::LogInstance &instance, + uint64_t txn) +{ + txlog::LogRequest request; + txlog::WriteLogRequest *write = request.mutable_write_log_request(); + // execute() derives the sending ng id from bits 42.. of the tx number. + write->set_txn_number((uint64_t{kNgId} << 42) | txn); + write->set_commit_timestamp(txn); + write->set_tx_term(kNgTerm); + (*write->mutable_node_terms())[kNgId] = kNgTerm; + (*write->mutable_log_content() + ->mutable_data_log() + ->mutable_node_txn_logs())[kNgId] = + "committed " + std::to_string(txn); + write->set_log_group_id(0); + txlog::LogResponse response; + instance.WriteLog(&request, &response, nullptr); + return response.response_status(); +} + +/** + * Counts the published (non-temp) braft snapshots that carry a RocksDB + * checkpoint. + * @param raft_path The braft storage directory of the instance. + * @return The number of published snapshots with rocksdb_ckpt/CURRENT. + */ +int PublishedCheckpointSnapshots(const fs::path &raft_path) +{ + int count = 0; + std::error_code error; + for (const auto &entry : + fs::directory_iterator(raft_path / "snapshot", error)) + { + if (entry.path().filename() != "temp" && + fs::exists(entry.path() / "rocksdb_ckpt/CURRENT")) + { + ++count; + } + } + return count; +} + +CATCH_TEST_CASE( + "A failed snapshot leaves the raft node serving writes and a later " + "snapshot succeeds without restart") +{ + TempDirectory dir; + LoopbackRaftServer server; + const auto &endpoint = server.endpoint; + const std::string group = "snapshot-retry-test"; + txlog::LogInstance instance(0, + 0, + "127.0.0.1", + server.port, + endpoint + ":0", + group, + "local://" + (dir.path / "raft").string(), +#ifdef WITH_CLOUD_AZ_INFO + "", + "", +#endif + dir.path / "db", + 1, + 0, + 8, + 12, + 64 * 1024 * 1024, + 0, + false, + 10, + 128 * 1024 * 1024); + CATCH_REQUIRE(instance.Start() == 0); + const auto deadline = std::chrono::steady_clock::now() + 10s; + while (!instance.is_leader() && std::chrono::steady_clock::now() < deadline) + { + std::this_thread::sleep_for(10ms); + } + CATCH_REQUIRE(instance.is_leader()); + + // Register kNgId's term through raft so that data-log writes pass the + // term checks; no_replay skips shipping logs to a cc node. + { + txlog::LogRequest request; + txlog::ReplayLogRequest *replay = request.mutable_replay_log_request(); + replay->set_cc_node_group_id(kNgId); + replay->set_term(kNgTerm); + replay->set_source_ip("127.0.0.1"); + replay->set_source_port(9000); + replay->set_log_group_id(0); + replay->set_no_replay(true); + txlog::LogResponse response; + instance.ReplayLog(&request, &response, nullptr); + CATCH_REQUIRE(response.response_status() == + txlog::LogResponse::Success); + } + CATCH_REQUIRE(WriteDataLog(instance, 1) == txlog::LogResponse::Success); + + braft::PeerId peer; + CATCH_REQUIRE(peer.parse(endpoint + ":0") == 0); + braft::cli::CliOptions options; + options.timeout_ms = 10000; + options.max_retry = 0; + const fs::path raft_path = dir.path / "raft"; + CATCH_REQUIRE(PublishedCheckpointSnapshots(raft_path) == 0); + + // Inject the fault for one attempt. braft runs the real on_snapshot_save, + // on_snapshot_save_done and writer close paths; the CLI returns the + // status the save reported. + fail_snapshot_fsync.store(true); + const butil::Status failed = braft::cli::snapshot(group, peer, options); + fail_snapshot_fsync.store(false); + CATCH_REQUIRE_FALSE(failed.ok()); + CATCH_REQUIRE(failed.error_code() == EPERM); + CATCH_REQUIRE(PublishedCheckpointSnapshots(raft_path) == 0); + + // EIO would have made braft call on_error, step the node down and reject + // this write with NotLeader. EPERM must leave the leader serving. + CATCH_REQUIRE(instance.is_leader()); + CATCH_REQUIRE(WriteDataLog(instance, 2) == txlog::LogResponse::Success); + + // With the fault removed, the next attempt publishes a full snapshot on + // the same node, and writes keep succeeding afterwards. + const butil::Status retried = braft::cli::snapshot(group, peer, options); + CATCH_REQUIRE(retried.ok()); + CATCH_REQUIRE(PublishedCheckpointSnapshots(raft_path) == 1); + CATCH_REQUIRE(WriteDataLog(instance, 3) == txlog::LogResponse::Success); + + instance.shutdown(); + instance.join(); +} +} // namespace diff --git a/eloq_log_service/test/snapshot/CMakeLists.txt b/eloq_log_service/test/snapshot/CMakeLists.txt new file mode 100644 index 000000000..1d2b70983 --- /dev/null +++ b/eloq_log_service/test/snapshot/CMakeLists.txt @@ -0,0 +1,13 @@ +# Shared by the parent WITH_TESTS build and the standalone TEST_LOG_SNAPSHOT +# build so CI runs the same suite as a focused local configure. +find_package(Catch2 CONFIG REQUIRED) +if(WITH_LOG_STATE STREQUAL "ROCKSDB") + add_executable(log_snapshot_tests ../log_snapshot_test.cpp) + set_property(TARGET log_snapshot_tests APPEND_STRING PROPERTY LINK_FLAGS + " -Wl,--wrap=pthread_create -Wl,--wrap=fsync") +else() + add_executable(log_snapshot_tests ../log_snapshot_stream_test.cpp) +endif() +target_link_libraries(log_snapshot_tests PRIVATE logservice Catch2::Catch2WithMain) +add_test(NAME log_snapshot_tests COMMAND log_snapshot_tests) +set_tests_properties(log_snapshot_tests PROPERTIES TIMEOUT 60) diff --git a/tx_service/tests/CMakeLists.txt b/tx_service/tests/CMakeLists.txt index b2d318080..41fed6301 100644 --- a/tx_service/tests/CMakeLists.txt +++ b/tx_service/tests/CMakeLists.txt @@ -5,6 +5,11 @@ find_package(Catch2 REQUIRED) include(Catch) +if(WITH_LOG_SERVICE) + add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/../../eloq_log_service/test/snapshot + ${CMAKE_CURRENT_BINARY_DIR}/log_snapshot) +endif() + set(TESTS_DIR ${CMAKE_CURRENT_SOURCE_DIR}) set(HARNESS_DIR ${CMAKE_CURRENT_SOURCE_DIR}/harness) From 677b2c05783bbc62d6ac3fbcb01cf7ffd8e7fe16 Mon Sep 17 00:00:00 2001 From: Liang Jeff Chen Date: Mon, 19 Oct 2026 17:09:48 +0800 Subject: [PATCH 6/7] ci: build pinned MinIO after binary endpoint removal The MinIO binary endpoint returns HTTP 410 on amd64 and arm64, stopping log-service CI before its cloud test runs. Build the official RELEASE.2025-10-15T17-29-55Z commit with Go 1.24.8 and cache the static binary by OS, architecture, compiler version, and source commit. Validated workflow syntax with actionlint and bash -n, built MinIO for linux/amd64 and linux/arm64, and ran the exact cloud SST-reader test against the native binary: 1 case and 11 assertions passed. ARM64 was cross-compiled; native execution remains covered by the CI matrix. --- .github/workflows/tests.yml | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 386cddac8..94f79e684 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -136,6 +136,10 @@ jobs: AWS_ACCESS_KEY_ID: minioadmin AWS_SECRET_ACCESS_KEY: minioadmin AWS_EC2_METADATA_DISABLED: "true" + # MinIO's community binary download endpoint is gone. Build the pinned + # RELEASE.2025-10-15T17-29-55Z source with its declared Go toolchain. + MINIO_COMMIT: 9e49d5e7a648f00e26f2246f4dc28e6b07f8c84a + MINIO_GO_VERSION: "1.24.8" steps: - name: Checkout (recursive submodules) uses: actions/checkout@v4 @@ -148,6 +152,28 @@ jobs: arch: ${{ matrix.arch }} cache-scope: ubuntu-dev + - name: Restore MinIO binary + id: minio-cache + uses: actions/cache@v4 + with: + path: ${{ runner.temp }}/minio-bin + key: minio-${{ runner.os }}-${{ matrix.arch }}-go${{ env.MINIO_GO_VERSION }}-${{ env.MINIO_COMMIT }} + + - name: Setup Go for MinIO + if: steps.minio-cache.outputs.cache-hit != 'true' + uses: actions/setup-go@v6 + with: + go-version: ${{ env.MINIO_GO_VERSION }} + cache: false + + - name: Build MinIO from source (cache miss) + if: steps.minio-cache.outputs.cache-hit != 'true' + env: + GOBIN: ${{ runner.temp }}/minio-bin + CGO_ENABLED: "0" + GOTOOLCHAIN: local + run: go install "github.com/minio/minio@${MINIO_COMMIT}" + - name: Compile-check (RocksDB, all test targets) run: | cmake -S eloq_log_service -B eloq_log_service/bld \ @@ -169,12 +195,11 @@ jobs: # is guaranteed alive for the test. Uses curl health checks rather than # bash-only /dev/tcp so it is robust regardless of shell. run: | - curl -fsSL "https://dl.min.io/server/minio/release/linux-${{ matrix.arch }}/minio" \ - -o /usr/local/bin/minio - chmod +x /usr/local/bin/minio + minio_bin="${RUNNER_TEMP}/minio-bin/minio" + "$minio_bin" --version mkdir -p /tmp/minio-data MINIO_ROOT_USER=minioadmin MINIO_ROOT_PASSWORD=minioadmin \ - minio server /tmp/minio-data --address ":9000" > /tmp/minio.log 2>&1 & + "$minio_bin" server /tmp/minio-data --address ":9000" > /tmp/minio.log 2>&1 & ok=0 for i in $(seq 1 60); do if curl -sf http://127.0.0.1:9000/minio/health/live >/dev/null 2>&1; then From 02f4a2f7cb5702527ad74ab432fab45eef56da01 Mon Sep 17 00:00:00 2001 From: Liang Jeff Chen Date: Mon, 19 Oct 2026 17:17:42 +0800 Subject: [PATCH 7/7] ci: reuse image-provided RustFS for cloud log tests Follow EloqKV #575 and use the RustFS binary already installed in the shared ubuntu-dev image. Remove the MinIO source build and cache, check for RustFS before compiling, and use its readiness endpoint. Preserve failure status and logs while stopping the server and removing its private temporary data on exit. Verified the published amd64/arm64 image manifests and the image-pinned amd64 RustFS checksum. The exact workflow cloud test passed locally: 1 case and 11 assertions. Missing-binary and injected-exit-7 cleanup checks passed, as did actionlint, YAML, shell syntax, and whitespace. --- .github/workflows/tests.yml | 88 +++++++++++++++++++------------------ 1 file changed, 45 insertions(+), 43 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 94f79e684..1f5515b8f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -131,15 +131,11 @@ jobs: CMAKE_LIBRARY_PATH: /opt/eloq/third_party/lib LD_LIBRARY_PATH: /opt/eloq/third_party/lib # Consumed by eloq_log_service/test/test_utils.h + the cloud test harness: - # routes rocksdb-cloud's S3 client at MinIO with path-style addressing. + # routes rocksdb-cloud's S3 client at RustFS with path-style addressing. TEST_S3_ENDPOINT: http://127.0.0.1:9000 - AWS_ACCESS_KEY_ID: minioadmin - AWS_SECRET_ACCESS_KEY: minioadmin + AWS_ACCESS_KEY_ID: rustfsadmin + AWS_SECRET_ACCESS_KEY: rustfsadmin AWS_EC2_METADATA_DISABLED: "true" - # MinIO's community binary download endpoint is gone. Build the pinned - # RELEASE.2025-10-15T17-29-55Z source with its declared Go toolchain. - MINIO_COMMIT: 9e49d5e7a648f00e26f2246f4dc28e6b07f8c84a - MINIO_GO_VERSION: "1.24.8" steps: - name: Checkout (recursive submodules) uses: actions/checkout@v4 @@ -152,27 +148,14 @@ jobs: arch: ${{ matrix.arch }} cache-scope: ubuntu-dev - - name: Restore MinIO binary - id: minio-cache - uses: actions/cache@v4 - with: - path: ${{ runner.temp }}/minio-bin - key: minio-${{ runner.os }}-${{ matrix.arch }}-go${{ env.MINIO_GO_VERSION }}-${{ env.MINIO_COMMIT }} - - - name: Setup Go for MinIO - if: steps.minio-cache.outputs.cache-hit != 'true' - uses: actions/setup-go@v6 - with: - go-version: ${{ env.MINIO_GO_VERSION }} - cache: false - - - name: Build MinIO from source (cache miss) - if: steps.minio-cache.outputs.cache-hit != 'true' - env: - GOBIN: ${{ runner.temp }}/minio-bin - CGO_ENABLED: "0" - GOTOOLCHAIN: local - run: go install "github.com/minio/minio@${MINIO_COMMIT}" + - name: Check image-provided S3 test server + # The shared ubuntu-dev image pins RustFS for EloqKV and EloqDoc too. + run: | + if ! command -v rustfs >/dev/null 2>&1; then + echo "RustFS is missing; use an ubuntu-dev image with RustFS preinstalled." >&2 + exit 1 + fi + rustfs --version - name: Compile-check (RocksDB, all test targets) run: | @@ -190,32 +173,51 @@ jobs: -DELOQ_THIRD_PARTY_REQUIRED=ON cmake --build eloq_log_service/bld_cloud --parallel "$(nproc)" - - name: Run cloud unit tests against MinIO - # Start MinIO and run the test in one step so the server (backgrounded) - # is guaranteed alive for the test. Uses curl health checks rather than - # bash-only /dev/tcp so it is robust regardless of shell. + - name: Run cloud unit tests against RustFS + # Keep the server in this step and clean it up even when the test fails. run: | - minio_bin="${RUNNER_TEMP}/minio-bin/minio" - "$minio_bin" --version - mkdir -p /tmp/minio-data - MINIO_ROOT_USER=minioadmin MINIO_ROOT_PASSWORD=minioadmin \ - "$minio_bin" server /tmp/minio-data --address ":9000" > /tmp/minio.log 2>&1 & + rustfs_run_dir=$(mktemp -d /tmp/tx-service-rustfs.XXXXXX) + rustfs_pid= + cleanup() { + local result=$? + trap - EXIT + if [ "$result" -ne 0 ]; then + cat "${rustfs_run_dir}/rustfs.log" >&2 || true + fi + if [ -n "$rustfs_pid" ]; then + kill "$rustfs_pid" 2>/dev/null || true + wait "$rustfs_pid" 2>/dev/null || true + fi + rm -rf "$rustfs_run_dir" || true + exit "$result" + } + trap cleanup EXIT + mkdir -p "${rustfs_run_dir}/data" + RUSTFS_ACCESS_KEY="$AWS_ACCESS_KEY_ID" RUSTFS_SECRET_KEY="$AWS_SECRET_ACCESS_KEY" \ + RUSTFS_ADDRESS="${TEST_S3_ENDPOINT#http://}" RUSTFS_CONSOLE_ENABLE=false \ + rustfs server "${rustfs_run_dir}/data" > "${rustfs_run_dir}/rustfs.log" 2>&1 & + rustfs_pid=$! ok=0 for i in $(seq 1 60); do - if curl -sf http://127.0.0.1:9000/minio/health/live >/dev/null 2>&1; then - ok=1; echo "MinIO is up"; break + if ! kill -0 "$rustfs_pid" 2>/dev/null; then + echo "RustFS exited before becoming ready." >&2 + exit 1 + fi + if curl -sf --noproxy '*' --max-time 2 "${TEST_S3_ENDPOINT}/health/ready" >/dev/null 2>&1; then + ok=1; echo "RustFS is ready"; break fi sleep 1 done if [ "$ok" != "1" ]; then - echo "MinIO failed to start:" >&2; cat /tmp/minio.log >&2; exit 1 + echo "RustFS did not become ready at ${TEST_S3_ENDPOINT}." >&2 + exit 1 fi # test_rocksdb_cloud_sst_reader exercises the rocksdb-cloud storage - # provider end-to-end against MinIO. The DBCloud::Open-based cases + # provider end-to-end against RustFS. The DBCloud::Open-based cases # currently crash inside librocksdb-cloud (ABI/vtable mismatch) and are # tracked separately, so they are excluded here. cd eloq_log_service/bld_cloud ./log_server_rocksdb_cloud_tests "test_rocksdb_cloud_sst_reader" \ --region us-east-1 \ - --aws_access_key_id minioadmin \ - --aws_secret_key minioadmin + --aws_access_key_id "$AWS_ACCESS_KEY_ID" \ + --aws_secret_key "$AWS_SECRET_ACCESS_KEY"