aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authormoneromooo-monero <moneromooo-monero@users.noreply.github.com>2019-05-24 08:33:19 +0000
committermoneromooo-monero <moneromooo-monero@users.noreply.github.com>2019-05-25 16:25:10 +0000
commit8f2a99d8ab1d3ea23c1968d6b3f1b43fd7faff89 (patch)
treee28a01cfd2201b7a2c5dd63af2afd1ac861ef21b
parentMerge pull request #5548 (diff)
downloadmonero-8f2a99d8ab1d3ea23c1968d6b3f1b43fd7faff89.tar.xz
core: do not commit half constructed batch db txn
-rw-r--r--src/blockchain_db/blockchain_db.h15
-rw-r--r--src/blockchain_db/testdb.h1
-rw-r--r--src/cryptonote_core/blockchain.cpp28
-rw-r--r--src/cryptonote_core/blockchain.h3
-rw-r--r--src/cryptonote_core/tx_pool.cpp18
5 files changed, 51 insertions, 14 deletions
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
@@ -754,6 +754,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
*
* If the subclass implements batching, this function tells it to begin
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 39c9f8695..760d1ee81 100644
--- a/src/cryptonote_core/blockchain.cpp
+++ b/src/cryptonote_core/blockchain.cpp
@@ -182,7 +182,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__);
}
@@ -619,14 +620,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
{
@@ -637,10 +631,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
@@ -3844,6 +3842,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;
@@ -3852,6 +3851,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;
@@ -4161,7 +4161,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)
@@ -4385,6 +4388,7 @@ bool Blockchain::prepare_handle_incoming_blocks(const std::vector<block_complete
m_tx_pool.lock();
m_blockchain_lock.lock();
}
+ m_batch_success = true;
const uint64_t height = m_db->height();
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<tools::Notify> m_block_notify;
std::shared_ptr<tools::Notify> m_reorg_notify;
diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp
index c1cbe2acd..49d5a8ccc 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<double, std::time_t>(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<double, std::time_t>(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;