diff --git a/src/blockchain_db/blockchain_db.h b/src/blockchain_db/blockchain_db.h index 567be6a65..b6b8c6c3e 100644 --- a/src/blockchain_db/blockchain_db.h +++ b/src/blockchain_db/blockchain_db.h @@ -753,6 +753,21 @@ public: */ virtual void batch_stop() = 0; + /** + * @brief aborts a batch transaction + * + * If the subclass implements batching, this function should abort the + * batch it is currently on. + * + * If no batch is in-progress, this function should throw a DB_ERROR. + * This exception may change in the future if it is deemed necessary to + * have a more granular exception type for this scenario. + * + * If any of this cannot be done, the subclass should throw the corresponding + * subclass of DB_EXCEPTION + */ + virtual void batch_abort() = 0; + /** * @brief sets whether or not to batch transactions * diff --git a/src/blockchain_db/testdb.h b/src/blockchain_db/testdb.h index 6c97713d5..34e635899 100644 --- a/src/blockchain_db/testdb.h +++ b/src/blockchain_db/testdb.h @@ -54,6 +54,7 @@ public: virtual void unlock() override { } virtual bool batch_start(uint64_t batch_num_blocks=0, uint64_t batch_bytes=0) override { return true; } virtual void batch_stop() override {} + virtual void batch_abort() override {} virtual void set_batch_transactions(bool) override {} virtual void block_wtxn_start() override {} virtual void block_wtxn_stop() override {} diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 9a970e915..b143751af 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -137,7 +137,8 @@ Blockchain::Blockchain(tx_memory_pool& tx_pool) : m_long_term_block_weights_cache_rolling_median(CRYPTONOTE_LONG_TERM_BLOCK_WEIGHT_WINDOW_SIZE), m_difficulty_for_next_block_top_hash(crypto::null_hash), m_difficulty_for_next_block(1), - m_btc_valid(false) + m_btc_valid(false), + m_batch_success(true) { LOG_PRINT_L3("Blockchain::" << __func__); } @@ -574,14 +575,7 @@ void Blockchain::pop_blocks(uint64_t nblocks) CRITICAL_REGION_LOCAL(m_tx_pool); CRITICAL_REGION_LOCAL1(m_blockchain_lock); - while (!m_db->batch_start()) - { - m_blockchain_lock.unlock(); - m_tx_pool.unlock(); - epee::misc_utils::sleep_no_w(1000); - m_tx_pool.lock(); - m_blockchain_lock.lock(); - } + bool stop_batch = m_db->batch_start(); try { @@ -592,10 +586,14 @@ void Blockchain::pop_blocks(uint64_t nblocks) } catch (const std::exception& e) { - LOG_ERROR("Error when popping blocks, only " << i << " blocks are popped: " << e.what()); + LOG_ERROR("Error when popping blocks after processing " << i << " blocks: " << e.what()); + if (stop_batch) + m_db->batch_abort(); + return; } - m_db->batch_stop(); + if (stop_batch) + m_db->batch_stop(); } //------------------------------------------------------------------ // This function tells BlockchainDB to remove the top block from the @@ -3862,6 +3860,7 @@ leave: catch (const KEY_IMAGE_EXISTS& e) { LOG_ERROR("Error adding block with hash: " << id << " to blockchain, what = " << e.what()); + m_batch_success = false; bvc.m_verifivation_failed = true; return_tx_to_pool(txs); return false; @@ -3870,6 +3869,7 @@ leave: { //TODO: figure out the best way to deal with this failure LOG_ERROR("Error adding block with hash: " << id << " to blockchain, what = " << e.what()); + m_batch_success = false; bvc.m_verifivation_failed = true; return_tx_to_pool(txs); return false; @@ -4179,7 +4179,10 @@ bool Blockchain::cleanup_handle_incoming_blocks(bool force_sync) try { - m_db->batch_stop(); + if (m_batch_success) + m_db->batch_stop(); + else + m_db->batch_abort(); success = true; } catch (const std::exception &e) @@ -4403,6 +4406,7 @@ bool Blockchain::prepare_handle_incoming_blocks(const std::vectorheight(); if ((height + blocks_entry.size()) < m_blocks_hash_check.size()) diff --git a/src/cryptonote_core/blockchain.h b/src/cryptonote_core/blockchain.h index 6200ec87e..32ed96b5b 100644 --- a/src/cryptonote_core/blockchain.h +++ b/src/cryptonote_core/blockchain.h @@ -1102,6 +1102,9 @@ namespace cryptonote uint64_t m_btc_expected_reward; bool m_btc_valid; + + bool m_batch_success; + std::shared_ptr m_block_notify; std::shared_ptr m_reorg_notify; diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp index bb3520873..1b286ae89 100644 --- a/src/cryptonote_core/tx_pool.cpp +++ b/src/cryptonote_core/tx_pool.cpp @@ -95,13 +95,17 @@ namespace cryptonote // the whole prepare/handle/cleanup incoming block sequence. class LockedTXN { public: - LockedTXN(Blockchain &b): m_blockchain(b), m_batch(false) { + LockedTXN(Blockchain &b): m_blockchain(b), m_batch(false), m_active(false) { m_batch = m_blockchain.get_db().batch_start(); + m_active = true; } - ~LockedTXN() { try { if (m_batch) { m_blockchain.get_db().batch_stop(); } } catch (const std::exception &e) { MWARNING("LockedTXN dtor filtering exception: " << e.what()); } } + void commit() { try { if (m_batch && m_active) { m_blockchain.get_db().batch_stop(); m_active = false; } } catch (const std::exception &e) { MWARNING("LockedTXN::commit filtering exception: " << e.what()); } } + void abort() { try { if (m_batch && m_active) { m_blockchain.get_db().batch_abort(); m_active = false; } } catch (const std::exception &e) { MWARNING("LockedTXN::abort filtering exception: " << e.what()); } } + ~LockedTXN() { abort(); } private: Blockchain &m_blockchain; bool m_batch; + bool m_active; }; } //--------------------------------------------------------------------------------- @@ -255,6 +259,7 @@ namespace cryptonote if (!insert_key_images(tx, id, kept_by_block)) return false; m_txs_by_fee_and_receive_time.emplace(std::pair(fee / (double)tx_weight, receive_time), id); + lock.commit(); } catch (const std::exception &e) { @@ -299,6 +304,7 @@ namespace cryptonote if (!insert_key_images(tx, id, kept_by_block)) return false; m_txs_by_fee_and_receive_time.emplace(std::pair(fee / (double)tx_weight, receive_time), id); + lock.commit(); } catch (const std::exception &e) { @@ -398,6 +404,7 @@ namespace cryptonote return; } } + lock.commit(); if (changed) ++m_cookie; if (m_txpool_weight > bytes) @@ -494,6 +501,7 @@ namespace cryptonote m_blockchain.remove_txpool_tx(id); m_txpool_weight -= tx_weight; remove_transaction_keyimages(tx, id); + lock.commit(); } catch (const std::exception &e) { @@ -578,6 +586,7 @@ namespace cryptonote // ignore error } } + lock.commit(); ++m_cookie; } return true; @@ -641,6 +650,7 @@ namespace cryptonote // continue } } + lock.commit(); } //--------------------------------------------------------------------------------- size_t tx_memory_pool::get_transactions_count(bool include_unrelayed_txes) const @@ -1119,6 +1129,7 @@ namespace cryptonote } } } + lock.commit(); if (changed) ++m_cookie; } @@ -1271,6 +1282,7 @@ namespace cryptonote append_key_images(k_images, tx); LOG_PRINT_L2(" added, new block weight " << total_weight << "/" << max_total_weight << ", coinbase " << print_money(best_coinbase)); } + lock.commit(); expected_reward = best_coinbase; LOG_PRINT_L2("Block template filled with " << bl.tx_hashes.size() << " txes, weight " @@ -1336,6 +1348,7 @@ namespace cryptonote // continue } } + lock.commit(); } if (n_removed > 0) ++m_cookie; @@ -1395,6 +1408,7 @@ namespace cryptonote // ignore error } } + lock.commit(); } m_cookie = 0;