From 8722452d059df68fb47f3ee2a7b470ef1c6a434f Mon Sep 17 00:00:00 2001 From: "crypro.zoidberg" Date: Fri, 26 Apr 2019 03:49:38 +0200 Subject: [PATCH] some optimizations over tx pool synchronization --- src/currency_core/blockchain_storage.cpp | 2 +- src/currency_core/currency_core.cpp | 4 +- src/currency_core/tx_pool.cpp | 95 +++++++++++++++--------- src/currency_core/tx_pool.h | 32 ++++---- 4 files changed, 83 insertions(+), 50 deletions(-) diff --git a/src/currency_core/blockchain_storage.cpp b/src/currency_core/blockchain_storage.cpp index a7fab028..a991f3ba 100644 --- a/src/currency_core/blockchain_storage.cpp +++ b/src/currency_core/blockchain_storage.cpp @@ -4654,7 +4654,7 @@ bool blockchain_storage::add_new_block(const block& bl_, block_verification_cont block bl = bl_; crypto::hash id = get_block_hash(bl); - CRITICAL_REGION_LOCAL(m_tx_pool); + //CRITICAL_REGION_LOCAL(m_tx_pool); //CRITICAL_REGION_LOCAL1(m_read_lock); if (have_block(id)) { diff --git a/src/currency_core/currency_core.cpp b/src/currency_core/currency_core.cpp index 0e4e66c4..62cd6934 100644 --- a/src/currency_core/currency_core.cpp +++ b/src/currency_core/currency_core.cpp @@ -386,7 +386,9 @@ namespace currency //----------------------------------------------------------------------------------------------- bool core::add_new_tx(const transaction& tx, const crypto::hash& tx_hash, size_t blob_size, tx_verification_context& tvc, bool kept_by_block) { - if(m_mempool.have_tx(tx_hash)) + //extra check "kept_by_block" to let happen call "m_mempool.add_tx" for transactions that came with block + //which should update "last_touch_time", to avoid removing transaction as "stuck" while it adding with new block(possible when mem_pool is over flooded) + if(!kept_by_block && m_mempool.have_tx(tx_hash)) { LOG_PRINT_L3("add_new_tx: already have tx " << tx_hash << " in the pool"); return true; diff --git a/src/currency_core/tx_pool.cpp b/src/currency_core/tx_pool.cpp index e09f5869..033bf83c 100644 --- a/src/currency_core/tx_pool.cpp +++ b/src/currency_core/tx_pool.cpp @@ -34,6 +34,7 @@ namespace currency tx_memory_pool::tx_memory_pool(blockchain_storage& bchs, i_currency_protocol* pprotocol) : m_blockchain(bchs) , m_pprotocol(pprotocol) + , m_current_processing_tx_id(null_hash) { } @@ -69,8 +70,32 @@ namespace currency return true; } //--------------------------------------------------------------------------------- + bool tx_memory_pool::touch_tx(const crypto::hash &id) + { + CRITICAL_REGION_LOCAL(m_transactions_lock); + auto it = m_transactions.find(id); + CHECK_AND_ASSERT_THROW_MES(it != m_transactions.end(), "Unable to find id: " << id); + it->second.last_touch_time = get_core_time(); + return true; + } + //--------------------------------------------------------------------------------- bool tx_memory_pool::add_tx(const transaction &tx, const crypto::hash &id, uint64_t blob_size, tx_verification_context& tvc, bool kept_by_block, bool from_core) { + CRITICAL_REGION_LOCAL(m_add_tx_lock); + + //using m_transactions_lock here is needed to moderate access with remove_stuck_transactions() + CRITICAL_REGION_BEGIN(m_transactions_lock); + if (have_tx(id)) + { + if (kept_by_block) + { + //prevent pool to get rid of transaction by expiration time + touch_tx(id); + } + } + CRITICAL_REGION_END(); + + TIME_MEASURE_START_PD(tx_processing_time); TIME_MEASURE_START_PD(check_inputs_types_supported_time); if(!check_inputs_types_supported(tx)) @@ -225,7 +250,7 @@ namespace currency td.max_used_block_height = max_used_block_height; td.last_failed_height = 0; td.last_failed_id = null_hash; - td.receive_time = get_core_time(); + td.last_touch_time = td.receive_time = get_core_time(); m_transactions.insert(std::make_pair(id, td)); on_tx_add(tx, kept_by_block); @@ -234,23 +259,23 @@ namespace currency return true; } //--------------------------------------------------------------------------------- - bool tx_memory_pool::check_is_taken(const crypto::hash& id) const - { - CRITICAL_REGION_LOCAL(m_taken_txs_lock); - return m_taken_txs.count(id) ? true : false; - } +// bool tx_memory_pool::check_is_taken(const crypto::hash& id) const +// { +// CRITICAL_REGION_LOCAL(m_taken_txs_lock); +// return m_taken_txs.count(id) ? true : false; +// } //--------------------------------------------------------------------------------- - void tx_memory_pool::set_taken(const crypto::hash& id) - { - CRITICAL_REGION_LOCAL(m_taken_txs_lock); - m_taken_txs.insert(id); - } +// void tx_memory_pool::set_taken(const crypto::hash& id) +// { +// CRITICAL_REGION_LOCAL(m_taken_txs_lock); +// m_taken_txs.insert(id); +// } //--------------------------------------------------------------------------------- - void tx_memory_pool::reset_all_taken() - { - CRITICAL_REGION_LOCAL(m_taken_txs_lock); - m_taken_txs.clear(); - } +// void tx_memory_pool::reset_all_taken() +// { +// CRITICAL_REGION_LOCAL(m_taken_txs_lock); +// m_taken_txs.clear(); +// } //--------------------------------------------------------------------------------- bool tx_memory_pool::process_cancel_offer_rules(const transaction& tx) { @@ -433,7 +458,7 @@ namespace currency } on_tx_remove(tx, kept_by_block); - set_taken(id); + //set_taken(id); return true; } //--------------------------------------------------------------------------------- @@ -444,11 +469,11 @@ namespace currency //--------------------------------------------------------------------------------- bool tx_memory_pool::remove_stuck_transactions() { - if (!CRITICAL_SECTION_TRY_LOCK(m_remove_stuck_txs_lock)) - return true; +// if (!CRITICAL_SECTION_TRY_LOCK(m_remove_stuck_txs_lock)) +// return true; - CRITICAL_REGION_LOCAL(m_remove_stuck_txs_lock); - CRITICAL_SECTION_UNLOCK(m_remove_stuck_txs_lock);// release try_lock iteration +// CRITICAL_REGION_LOCAL(m_remove_stuck_txs_lock); +// CRITICAL_SECTION_UNLOCK(m_remove_stuck_txs_lock);// release try_lock iteration CRITICAL_REGION_LOCAL1(m_transactions_lock); // !!TODO!! review lock scheme @@ -478,7 +503,7 @@ namespace currency return true; // maximum age check - remove too old - uint64_t tx_age = get_core_time() - tx_entry.receive_time; + uint64_t tx_age = get_core_time() - tx_entry.last_touch_time; if ((tx_age > CURRENCY_MEMPOOL_TX_LIVETIME )) { @@ -661,7 +686,7 @@ namespace currency //--------------------------------------------------------------------------------- bool tx_memory_pool::on_finalize_db_transaction() { - reset_all_taken(); + //reset_all_taken(); return true; } //--------------------------------------------------------------------------------- @@ -708,8 +733,8 @@ namespace currency if(m_transactions.count(id)) return true; - if (check_is_taken(id)) - return true; +// if (check_is_taken(id)) +// return true; return false; } @@ -858,15 +883,15 @@ namespace currency return false; } //--------------------------------------------------------------------------------- - void tx_memory_pool::lock() - { - CRITICAL_SECTION_LOCK(m_remove_stuck_txs_lock); - } + //void tx_memory_pool::lock() + //{ + // CRITICAL_SECTION_LOCK(m_remove_stuck_txs_lock); + //} //--------------------------------------------------------------------------------- - void tx_memory_pool::unlock() - { - CRITICAL_SECTION_UNLOCK(m_remove_stuck_txs_lock); - } + //void tx_memory_pool::unlock() + //{ + // CRITICAL_SECTION_UNLOCK(m_remove_stuck_txs_lock); + //} //--------------------------------------------------------------------------------- void tx_memory_pool::purge_transactions() { @@ -874,8 +899,8 @@ namespace currency m_cancel_offer_hashes.clear(); // should m_db_black_tx_list be cleared here? m_key_images_set.clear(); - // TODO : m_alias_names_set ? - // TODO : m_alias_addresses_set ? + m_alias_names_set.clear(); + m_alias_addresses_set.clear(); } //--------------------------------------------------------------------------------- void tx_memory_pool::clear() diff --git a/src/currency_core/tx_pool.h b/src/currency_core/tx_pool.h index 127ac220..509a3c84 100644 --- a/src/currency_core/tx_pool.h +++ b/src/currency_core/tx_pool.h @@ -50,6 +50,7 @@ namespace currency uint64_t last_failed_height; crypto::hash last_failed_id; time_t receive_time; + time_t last_touch_time; BEGIN_SERIALIZE_OBJECT() FIELD(tx) @@ -61,7 +62,8 @@ namespace currency FIELD(last_failed_height) FIELD(last_failed_id) FIELD(receive_time) - END_SERIALIZE() + FIELD(last_touch_time) // last time new block added this tx + END_SERIALIZE() }; @@ -108,8 +110,8 @@ namespace currency void on_idle(); - void lock(); - void unlock(); + //void lock(); + //void unlock(); void purge_transactions(); void clear(); @@ -158,6 +160,7 @@ namespace currency bool insert_alias_info(const transaction& tx); bool remove_alias_info(const transaction& tx); + bool touch_tx(const crypto::hash &id); bool is_valid_contract_finalization_tx(const transaction &tx)const; bool remove_stuck_transactions(); bool is_transaction_ready_to_go(tx_details& txd, const crypto::hash& id)const; @@ -165,9 +168,9 @@ namespace currency bool get_key_images_from_tx_pool(std::unordered_set& key_images)const; bool process_cancel_offer_rules(const transaction& tx); bool unprocess_cancel_offer_rules(const transaction& tx); - bool check_is_taken(const crypto::hash& id) const; - void set_taken(const crypto::hash& id); - void reset_all_taken(); +// bool check_is_taken(const crypto::hash& id) const; +// void set_taken(const crypto::hash& id); +// void reset_all_taken(); typedef std::unordered_map transactions_container; typedef std::unordered_map key_images_container; @@ -175,10 +178,6 @@ namespace currency typedef std::unordered_set aliases_container; typedef std::unordered_set aliases_addresses_container; - //main accessor - epee::shared_recursive_mutex m_dummy_rw_lock; - mutable epee::critical_section m_transactions_lock; - //containers transactions_container m_transactions; hash_container m_cancel_offer_hashes; @@ -195,9 +194,16 @@ namespace currency i_currency_protocol* m_pprotocol; //in memory containers - mutable epee::critical_section m_taken_txs_lock; - std::unordered_set m_taken_txs; - mutable epee::critical_section m_remove_stuck_txs_lock; + //mutable epee::critical_section m_taken_txs_lock; + //std::unordered_set m_taken_txs; + + + mutable epee::critical_section m_add_tx_lock; + std::atomic m_current_processing_tx_id; + //main accessor + mutable epee::critical_section m_transactions_lock; + //other containers locks + //mutable epee::critical_section m_remove_stuck_txs_lock; mutable epee::critical_section m_cancel_offer_hashes_lock; mutable epee::critical_section m_aliases_lock; mutable epee::critical_section m_black_tx_list_lock;