From da17def877bdecbce24b1a83e25a912a61235d04 Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Fri, 17 Jul 2020 17:29:44 -0700 Subject: [PATCH] crimson/os/seastore/journal: return addr and seq from submit_record Reworks journal_seq_t to be a monotonically increasing, but non-dense value from the current segment_seq_t and segment offset. Signed-off-by: Samuel Just --- src/crimson/os/seastore/cache.cc | 3 +- src/crimson/os/seastore/cache.h | 5 +-- src/crimson/os/seastore/journal.cc | 18 +++++------ src/crimson/os/seastore/journal.h | 32 +++++++++---------- src/crimson/os/seastore/seastore_types.cc | 8 +++++ src/crimson/os/seastore/seastore_types.h | 25 +++++++++++++++ .../os/seastore/transaction_manager.cc | 5 +-- .../seastore/test_btree_lba_manager.cc | 5 +-- .../crimson/seastore/test_seastore_cache.cc | 3 +- .../crimson/seastore/test_seastore_journal.cc | 2 +- 10 files changed, 70 insertions(+), 36 deletions(-) diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 8df0478ff8dc2..264a8f24ec919 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -223,7 +223,8 @@ std::optional Cache::try_construct_record(Transaction &t) void Cache::complete_commit( Transaction &t, - paddr_t final_block_start) + paddr_t final_block_start, + journal_seq_t seq) { if (t.root) { remove_extent(root); diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 9ec64611c365a..46ea4f453013e 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -281,8 +281,9 @@ public: * and mutated exents. */ void complete_commit( - Transaction &t, ///< [in, out] current transaction - paddr_t final_block_start ///< [in] offset of initial block + Transaction &t, ///< [in, out] current transaction + paddr_t final_block_start, ///< [in] offset of initial block + journal_seq_t seq ///< [in] journal commit seq ); /** diff --git a/src/crimson/os/seastore/journal.cc b/src/crimson/os/seastore/journal.cc index fe2594c311983..098fb80b2a62e 100644 --- a/src/crimson/os/seastore/journal.cc +++ b/src/crimson/os/seastore/journal.cc @@ -59,7 +59,6 @@ ceph::bufferlist Journal::encode_record( record_header_t header{ rsize.mdlength, rsize.dlength, - current_journal_seq, 0 /* checksum, TODO */, record.deltas.size(), record.extents.size() @@ -85,7 +84,7 @@ ceph::bufferlist Journal::encode_record( return metadatabl; } -Journal::write_record_ertr::future<> Journal::write_record( +Journal::write_record_ret Journal::write_record( record_size_t rsize, record_t &&record) { @@ -99,7 +98,13 @@ Journal::write_record_ertr::future<> Journal::write_record( rsize.dlength); return current_journal_segment->write(target, to_write).handle_error( write_record_ertr::pass_further{}, - crimson::ct_error::assert_all{ "TODO" }); + crimson::ct_error::assert_all{ "TODO" }).safe_then([this, target] { + return write_record_ret( + write_record_ertr::ready_future_marker{}, + paddr_t{ + current_journal_segment->get_segment_id(), + target}); + }); } Journal::record_size_t Journal::get_encoded_record_length( @@ -123,13 +128,6 @@ bool Journal::needs_roll(segment_off_t length) const current_journal_segment->get_write_capacity(); } -paddr_t Journal::next_record_addr() const -{ - paddr_t ret{current_journal_segment->get_segment_id(), written_to}; - logger().debug("next_record_addr: {}", ret); - return ret; -} - Journal::roll_journal_segment_ertr::future<> Journal::roll_journal_segment() { diff --git a/src/crimson/os/seastore/journal.h b/src/crimson/os/seastore/journal.h index ab65f5f29e557..6d3b3cfee3f4e 100644 --- a/src/crimson/os/seastore/journal.h +++ b/src/crimson/os/seastore/journal.h @@ -19,10 +19,6 @@ namespace crimson::os::seastore { -using journal_seq_t = uint64_t; -static constexpr journal_seq_t NO_DELTAS = - std::numeric_limits::max(); - /** * Segment header * @@ -50,7 +46,6 @@ struct record_header_t { // Fixed portion extent_len_t mdlength; // block aligned, length of metadata extent_len_t dlength; // block aligned, length of data - journal_seq_t seq; // current journal seqid checksum_t full_checksum; // checksum for full record (TODO) size_t deltas; // number of deltas size_t extents; // number of extents @@ -59,7 +54,6 @@ struct record_header_t { DENC_START(1, 1, p); denc(v.mdlength, p); denc(v.dlength, p); - denc(v.seq, p); denc(v.full_checksum, p); denc(v.deltas, p); denc(v.extents, p); @@ -126,15 +120,17 @@ public: close_ertr::future<> close() { return close_ertr::now(); } /** - * write_record + * submit_record * - * @param write record and returns offset of first block + * @param write record and returns offset of first block and seq */ using submit_record_ertr = crimson::errorator< crimson::ct_error::erange, crimson::ct_error::input_output_error >; - using submit_record_ret = submit_record_ertr::future; + using submit_record_ret = submit_record_ertr::future< + std::pair + >; submit_record_ret submit_record(record_t &&record) { auto rsize = get_encoded_record_length(record); auto total = rsize.mdlength + rsize.dlength; @@ -146,10 +142,11 @@ public: : roll_journal_segment_ertr::now(); return roll.safe_then( [this, rsize, record=std::move(record)]() mutable { - auto ret = next_record_addr(); return write_record(rsize, std::move(record) - ).safe_then([this, ret] { - return ret.add_offset(block_size); + ).safe_then([this](auto addr) { + return std::make_pair( + addr.add_offset(block_size), + get_journal_seq(addr)); }); }); } @@ -179,7 +176,10 @@ private: segment_off_t written_to = 0; segment_id_t next_journal_segment_seq = NULL_SEG_ID; - journal_seq_t current_journal_seq = 0; + + journal_seq_t get_journal_seq(paddr_t addr) { + return journal_seq_t{current_journal_segment_seq, addr}; + } /// prepare segment for writes, writes out segment header using initialize_segment_ertr = crimson::errorator< @@ -212,7 +212,8 @@ private: /// do record write using write_record_ertr = crimson::errorator< crimson::ct_error::input_output_error>; - write_record_ertr::future<> write_record( + using write_record_ret = write_record_ertr::future; + write_record_ret write_record( record_size_t rsize, record_t &&record); @@ -224,9 +225,6 @@ private: /// returns true iff current segment has insufficient space bool needs_roll(segment_off_t length) const; - /// returns next record addr - paddr_t next_record_addr() const; - /// return ordered vector of segments to replay using find_replay_segments_ertr = crimson::errorator< crimson::ct_error::input_output_error diff --git a/src/crimson/os/seastore/seastore_types.cc b/src/crimson/os/seastore/seastore_types.cc index 5e65c8d827a1c..9c20fc3f0e8d6 100644 --- a/src/crimson/os/seastore/seastore_types.cc +++ b/src/crimson/os/seastore/seastore_types.cc @@ -36,6 +36,14 @@ std::ostream &operator<<(std::ostream &out, const paddr_t &rhs) return out << ">"; } +std::ostream &operator<<(std::ostream &out, const journal_seq_t &seq) +{ + return out << "journal_seq_t(segment_seq=" + << seq.segment_seq << ", offset=" + << seq.offset + << ")"; +} + std::ostream &operator<<(std::ostream &out, extent_types_t t) { switch (t) { diff --git a/src/crimson/os/seastore/seastore_types.h b/src/crimson/os/seastore/seastore_types.h index 2bb5a4939e1cd..a17e47aac5ab8 100644 --- a/src/crimson/os/seastore/seastore_types.h +++ b/src/crimson/os/seastore/seastore_types.h @@ -181,6 +181,30 @@ std::ostream &operator<<(std::ostream &out, const paddr_t &rhs); using objaddr_t = uint32_t; constexpr objaddr_t OBJ_ADDR_MIN = std::numeric_limits::min(); +/* Monotonically increasing identifier for the location of a + * journal_record. + */ +struct journal_seq_t { + segment_seq_t segment_seq = 0; + paddr_t offset; + + DENC(journal_seq_t, v, p) { + DENC_START(1, 1, p); + denc(v.segment_seq, p); + denc(v.offset, p); + DENC_FINISH(p); + } +}; +WRITE_CMP_OPERATORS_2(journal_seq_t, segment_seq, offset) +WRITE_EQ_OPERATORS_2(journal_seq_t, segment_seq, offset) + +std::ostream &operator<<(std::ostream &out, const journal_seq_t &seq); + +static constexpr journal_seq_t NO_DELTAS = journal_seq_t{ + NULL_SEG_SEQ, + P_ADDR_NULL +}; + // logical addr, see LBAManager, TransactionManager using laddr_t = uint64_t; constexpr laddr_t L_ADDR_MIN = std::numeric_limits::min(); @@ -314,4 +338,5 @@ struct record_t { } WRITE_CLASS_DENC_BOUNDED(crimson::os::seastore::paddr_t) +WRITE_CLASS_DENC_BOUNDED(crimson::os::seastore::journal_seq_t) WRITE_CLASS_DENC_BOUNDED(crimson::os::seastore::delta_info_t) diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index 8b035768106fb..cf56154f35f05 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -164,8 +164,9 @@ TransactionManager::submit_transaction( logger().debug("TransactionManager::submit_transaction"); return journal.submit_record(std::move(*record)).safe_then( - [this, t=std::move(t)](paddr_t addr) mutable { - cache.complete_commit(*t, addr); + [this, t=std::move(t)](auto p) mutable { + auto [addr, journal_seq] = p; + cache.complete_commit(*t, addr, journal_seq); lba_manager.complete_transaction(*t); }, submit_transaction_ertr::pass_further{}, diff --git a/src/test/crimson/seastore/test_btree_lba_manager.cc b/src/test/crimson/seastore/test_btree_lba_manager.cc index 71ddbb29c8754..28059c4c90138 100644 --- a/src/test/crimson/seastore/test_btree_lba_manager.cc +++ b/src/test/crimson/seastore/test_btree_lba_manager.cc @@ -65,8 +65,9 @@ struct btree_lba_manager_test : } return journal.submit_record(std::move(*record)).safe_then( - [this, t=std::move(t)](paddr_t addr) mutable { - cache.complete_commit(*t, addr); + [this, t=std::move(t)](auto p) mutable { + auto [addr, seq] = p; + cache.complete_commit(*t, addr, seq); lba_manager->complete_transaction(*t); }, crimson::ct_error::all_same_way([](auto e) { diff --git a/src/test/crimson/seastore/test_seastore_cache.cc b/src/test/crimson/seastore/test_seastore_cache.cc index f480cd59ef814..e6b0fa7f393f1 100644 --- a/src/test/crimson/seastore/test_seastore_cache.cc +++ b/src/test/crimson/seastore/test_seastore_cache.cc @@ -23,6 +23,7 @@ struct cache_test_t : public seastar_test_suite_t { segment_manager::EphemeralSegmentManager segment_manager; Cache cache; paddr_t current{0, 0}; + journal_seq_t seq; cache_test_t() : segment_manager(segment_manager::DEFAULT_TEST_EPHEMERAL), @@ -55,7 +56,7 @@ struct cache_test_t : public seastar_test_suite_t { true ).safe_then( [this, prev, t=std::move(t)]() mutable { - cache.complete_commit(*t, prev); + cache.complete_commit(*t, prev, seq /* TODO */); return seastar::make_ready_future>(prev); }, crimson::ct_error::all_same_way([](auto e) { diff --git a/src/test/crimson/seastore/test_seastore_journal.cc b/src/test/crimson/seastore/test_seastore_journal.cc index ef5c1efa44b16..2529f3753fea0 100644 --- a/src/test/crimson/seastore/test_seastore_journal.cc +++ b/src/test/crimson/seastore/test_seastore_journal.cc @@ -154,7 +154,7 @@ struct journal_test_t : seastar_test_suite_t, JournalSegmentProvider { auto submit_record(T&&... _record) { auto record{std::forward(_record)...}; records.push_back(record); - auto addr = journal->submit_record(std::move(record)).unsafe_get0(); + auto [addr, _] = journal->submit_record(std::move(record)).unsafe_get0(); records.back().record_final_offset = addr; return addr; } -- 2.39.5