Merge pull request #2195 from MonsieurNicolas/fixTxQueue

Do not clear tx queue for good accounts Reviewed-by: MonsieurNicolas
stellar · Jul 13, 2019 · 5f7821d · 5f7821d
2 parents 05941b9 + 93bb58a
commit 5f7821d
Show file tree

Hide file tree

Showing 7 changed files with 103 additions and 14 deletions.
diff --git a/Builds/VisualStudio/stellar-core.vcxproj b/Builds/VisualStudio/stellar-core.vcxproj
@@ -257,17 +257,22 @@ exit /b 0
     <ClCompile Include="..\..\src\bucket\BucketInputIterator.cpp" />
     <ClCompile Include="..\..\src\bucket\BucketList.cpp" />
     <ClCompile Include="..\..\src\bucket\BucketManagerImpl.cpp" />
+    <ClCompile Include="..\..\src\bucket\BucketMergeMap.cpp" />
     <ClCompile Include="..\..\src\bucket\BucketOutputIterator.cpp" />
     <ClCompile Include="..\..\src\bucket\FutureBucket.cpp" />
+    <ClCompile Include="..\..\src\bucket\MergeKey.cpp" />
     <ClCompile Include="..\..\src\bucket\PublishQueueBuckets.cpp" />
     <ClCompile Include="..\..\src\bucket\test\BucketListTests.cpp" />
     <ClCompile Include="..\..\src\bucket\test\BucketManagerTests.cpp" />
+    <ClCompile Include="..\..\src\bucket\test\BucketMergeMapTests.cpp" />
     <ClCompile Include="..\..\src\bucket\test\BucketTests.cpp" />
     <ClCompile Include="..\..\src\catchup\ApplyBucketsWork.cpp" />
     <ClCompile Include="..\..\src\catchup\ApplyLedgerChainWork.cpp" />
     <ClCompile Include="..\..\src\catchup\CatchupConfiguration.cpp" />
     <ClCompile Include="..\..\src\catchup\CatchupManagerImpl.cpp" />
     <ClCompile Include="..\..\src\catchup\CatchupWork.cpp" />
+    <ClCompile Include="..\..\src\catchup\simulation\ApplyTransactionsWork.cpp" />
+    <ClCompile Include="..\..\src\catchup\simulation\HistoryArchiveStream.cpp" />
     <ClCompile Include="..\..\src\catchup\test\CatchupWorkTests.cpp" />
     <ClCompile Include="..\..\src\catchup\VerifyLedgerChainWork.cpp" />
     <ClCompile Include="..\..\src\crypto\ByteSliceHasher.cpp" />
@@ -293,6 +298,7 @@ exit /b 0
     <ClCompile Include="..\..\src\herder\HerderUtils.cpp" />
     <ClCompile Include="..\..\src\herder\LedgerCloseData.cpp" />
     <ClCompile Include="..\..\src\herder\PendingEnvelopes.cpp" />
+    <ClCompile Include="..\..\src\herder\simulation\SimulationTxSetFrame.cpp" />
     <ClCompile Include="..\..\src\herder\TransactionQueue.cpp" />
     <ClCompile Include="..\..\src\herder\QuorumTracker.cpp" />
     <ClCompile Include="..\..\src\herder\test\HerderTests.cpp" />
@@ -305,6 +311,8 @@ exit /b 0
     <ClCompile Include="..\..\src\historywork\BatchDownloadWork.cpp" />
     <ClCompile Include="..\..\src\herder\QuorumIntersectionCheckerImpl.cpp" />
     <ClCompile Include="..\..\src\herder\test\QuorumIntersectionTests.cpp" />
+    <ClCompile Include="..\..\src\transactions\simulation\SimulationMergeOpFrame.cpp" />
+    <ClCompile Include="..\..\src\transactions\simulation\SimulationTransactionFrame.cpp" />
     <ClCompile Include="..\..\src\work\BatchWork.cpp" />
     <ClCompile Include="..\..\src\historywork\DownloadBucketsWork.cpp" />
     <ClCompile Include="..\..\src\historywork\FetchRecentQsetsWork.cpp" />
@@ -560,16 +568,21 @@ exit /b 0
     <ClInclude Include="..\..\src\bucket\BucketList.h" />
     <ClInclude Include="..\..\src\bucket\BucketManager.h" />
     <ClInclude Include="..\..\src\bucket\BucketManagerImpl.h" />
+    <ClInclude Include="..\..\src\bucket\BucketMergeMap.h" />
     <ClInclude Include="..\..\src\bucket\BucketOutputIterator.h" />
+    <ClInclude Include="..\..\src\bucket\BucketTests.h" />
     <ClInclude Include="..\..\src\bucket\FutureBucket.h" />
     <ClInclude Include="..\..\src\bucket\LedgerCmp.h" />
+    <ClInclude Include="..\..\src\bucket\MergeKey.h" />
     <ClInclude Include="..\..\src\bucket\PublishQueueBuckets.h" />
     <ClInclude Include="..\..\src\catchup\ApplyBucketsWork.h" />
     <ClInclude Include="..\..\src\catchup\ApplyLedgerChainWork.h" />
     <ClInclude Include="..\..\src\catchup\CatchupConfiguration.h" />
     <ClInclude Include="..\..\src\catchup\CatchupManager.h" />
     <ClInclude Include="..\..\src\catchup\CatchupManagerImpl.h" />
     <ClInclude Include="..\..\src\catchup\CatchupWork.h" />
+    <ClInclude Include="..\..\src\catchup\simulation\ApplyTransactionsWork.h" />
+    <ClInclude Include="..\..\src\catchup\simulation\HistoryArchiveStream.h" />
     <ClInclude Include="..\..\src\catchup\test\CatchupWorkTests.h" />
     <ClInclude Include="..\..\src\catchup\VerifyLedgerChainWork.h" />
     <ClInclude Include="..\..\src\crypto\ByteSlice.h" />
@@ -595,13 +608,16 @@ exit /b 0
     <ClInclude Include="..\..\src\herder\HerderUtils.h" />
     <ClInclude Include="..\..\src\herder\LedgerCloseData.h" />
     <ClInclude Include="..\..\src\herder\PendingEnvelopes.h" />
+    <ClInclude Include="..\..\src\herder\simulation\SimulationTxSetFrame.h" />
     <ClInclude Include="..\..\src\herder\TransactionQueue.h" />
     <ClInclude Include="..\..\src\herder\QuorumTracker.h" />
     <ClInclude Include="..\..\src\herder\TxSetFrame.h" />
     <ClInclude Include="..\..\src\herder\Upgrades.h" />
     <ClInclude Include="..\..\src\historywork\BatchDownloadWork.h" />
     <ClInclude Include="..\..\src\herder\QuorumIntersectionChecker.h" />
     <ClInclude Include="..\..\src\herder\QuorumIntersectionCheckerImpl.h" />
+    <ClInclude Include="..\..\src\transactions\simulation\SimulationMergeOpFrame.h" />
+    <ClInclude Include="..\..\src\transactions\simulation\SimulationTransactionFrame.h" />
     <ClInclude Include="..\..\src\work\BatchWork.h" />
     <ClInclude Include="..\..\src\historywork\DownloadBucketsWork.h" />
     <ClInclude Include="..\..\src\historywork\FetchRecentQsetsWork.h" />
@@ -1003,4 +1019,4 @@ exit /b 0
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
   <ImportGroup Label="ExtensionTargets">
   </ImportGroup>
-</Project>
+</Project>
diff --git a/Builds/VisualStudio/stellar-core.vcxproj.filters b/Builds/VisualStudio/stellar-core.vcxproj.filters
@@ -142,6 +142,15 @@
     <Filter Include="github">
       <UniqueIdentifier>{2f5780ba-6cc8-45ba-b70b-aa50af3af3ce}</UniqueIdentifier>
     </Filter>
+    <Filter Include="herder\simulation">
+      <UniqueIdentifier>{d3bc5d86-b218-4775-a47d-8db1a1ec26a0}</UniqueIdentifier>
+    </Filter>
+    <Filter Include="catchup\simulation">
+      <UniqueIdentifier>{03aee051-1ba2-40f7-9551-8dbeaf8c4232}</UniqueIdentifier>
+    </Filter>
+    <Filter Include="transactions\simulation">
+      <UniqueIdentifier>{b342c551-848c-427d-8d3b-df060afe49aa}</UniqueIdentifier>
+    </Filter>
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="..\..\lib\util\getopt_long.c">
@@ -942,6 +951,30 @@
     <ClCompile Include="..\..\src\herder\QuorumIntersectionCheckerImpl.cpp">
       <Filter>herder</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\src\bucket\test\BucketMergeMapTests.cpp">
+      <Filter>bucket\tests</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\src\bucket\BucketMergeMap.cpp">
+      <Filter>bucket</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\src\bucket\MergeKey.cpp">
+      <Filter>bucket</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\src\herder\simulation\SimulationTxSetFrame.cpp">
+      <Filter>herder\simulation</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\src\catchup\simulation\ApplyTransactionsWork.cpp">
+      <Filter>catchup\simulation</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\src\catchup\simulation\HistoryArchiveStream.cpp">
+      <Filter>catchup\simulation</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\src\transactions\simulation\SimulationMergeOpFrame.cpp">
+      <Filter>transactions\simulation</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\src\transactions\simulation\SimulationTransactionFrame.cpp">
+      <Filter>transactions\simulation</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="..\..\lib\util\easylogging++.h">
@@ -1628,6 +1661,30 @@
     <ClInclude Include="..\..\src\herder\QuorumIntersectionCheckerImpl.h">
       <Filter>herder</Filter>
     </ClInclude>
+    <ClInclude Include="..\..\src\bucket\BucketMergeMap.h">
+      <Filter>bucket</Filter>
+    </ClInclude>
+    <ClInclude Include="..\..\src\bucket\BucketTests.h">
+      <Filter>bucket</Filter>
+    </ClInclude>
+    <ClInclude Include="..\..\src\bucket\MergeKey.h">
+      <Filter>bucket</Filter>
+    </ClInclude>
+    <ClInclude Include="..\..\src\herder\simulation\SimulationTxSetFrame.h">
+      <Filter>herder\simulation</Filter>
+    </ClInclude>
+    <ClInclude Include="..\..\src\catchup\simulation\ApplyTransactionsWork.h">
+      <Filter>catchup\simulation</Filter>
+    </ClInclude>
+    <ClInclude Include="..\..\src\catchup\simulation\HistoryArchiveStream.h">
+      <Filter>catchup\simulation</Filter>
+    </ClInclude>
+    <ClInclude Include="..\..\src\transactions\simulation\SimulationMergeOpFrame.h">
+      <Filter>transactions\simulation</Filter>
+    </ClInclude>
+    <ClInclude Include="..\..\src\transactions\simulation\SimulationTransactionFrame.h">
+      <Filter>transactions\simulation</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <None Include="..\..\AUTHORS" />
@@ -1785,4 +1842,4 @@
     <Text Include="..\..\INSTALL-Windows.md" />
     <Text Include="..\..\LICENSE-APACHE.txt" />
   </ItemGroup>
-</Project>
+</Project>
diff --git a/src/catchup/simulation/ApplyTransactionsWork.cpp b/src/catchup/simulation/ApplyTransactionsWork.cpp
@@ -63,7 +63,7 @@ ApplyTransactionsWork::getNextLedger(
         }
     }
 
-    uint32_t nOps = 0;
+    size_t nOps = 0;
     while (true)
     {
         while (mTransactionIter != mTransactionHistory.txSet.txs.cend() &&

diff --git a/src/herder/TransactionQueue.cpp b/src/herder/TransactionQueue.cpp
@@ -71,7 +71,7 @@ TransactionQueue::removeAndReset(
 {
     for (auto const& tx : dropTxs)
     {
-        auto extracted = extract(tx);
+        auto extracted = extract(tx, true);
         if (extracted.first != std::end(mPendingTransactions))
         {
             extracted.first->second.mAge = 0;
@@ -85,7 +85,7 @@ TransactionQueue::ban(std::vector<TransactionFramePtr> const& dropTxs)
     auto& bannedFront = mBannedTransactions.front();
     for (auto const& tx : dropTxs)
     {
-        for (auto const& extracted : extract(tx).second)
+        for (auto const& extracted : extract(tx, false).second)
         {
             bannedFront.insert(extracted->getFullHash());
         }
@@ -127,7 +127,7 @@ TransactionQueue::find(TransactionFramePtr const& tx)
 }
 
 TransactionQueue::ExtractResult
-TransactionQueue::extract(TransactionFramePtr const& tx)
+TransactionQueue::extract(TransactionFramePtr const& tx, bool keepBacklog)
 {
     auto it = find(tx);
     auto accIt = it.first;
@@ -138,16 +138,23 @@ TransactionQueue::extract(TransactionFramePtr const& tx)
 
     auto& txs = accIt->second.mTransactions;
     auto txIt = it.second;
-    auto feeBid =
-        std::accumulate(txIt, std::end(txs), int64_t{0},
+
+    auto txRemoveEnd = txIt + 1;
+    if (!keepBacklog)
+    {
+        // remove everything passed tx
+        txRemoveEnd = std::end(txs);
+    }
+    auto removedFeeBid =
+        std::accumulate(txIt, txRemoveEnd, int64_t{0},
                         [](int64_t fee, TransactionFramePtr const& tx) {
                             return fee + tx->getFeeBid();
                         });
-    accIt->second.mTotalFees -= feeBid;
+    accIt->second.mTotalFees -= removedFeeBid;
 
     auto movedTxs = std::vector<TransactionFramePtr>{};
-    std::move(txIt, std::end(txs), std::back_inserter(movedTxs));
-    txs.erase(txIt, std::end(txs));
+    std::move(txIt, txRemoveEnd, std::back_inserter(movedTxs));
+    txs.erase(txIt, txRemoveEnd);
 
     if (accIt->second.mTransactions.empty())
     {

diff --git a/src/herder/TransactionQueue.h b/src/herder/TransactionQueue.h
@@ -39,10 +39,11 @@ class Application;
  * by tryAdd operation. If that succeds, it can be later removed from it in one
  * of three ways:
  * * removeAndReset() should be called after transaction is successully
- *   included into some leger
+ *   included into some leger. It preserves the other pending transactions for
+ *   accounts and resets the TTL for banning
  * * ban() should be called after transaction became invalid for some reason
  *   (i.e. its source account cannot afford it anymore)
- * * shift() should be called after each ledger close, it bans transcations
+ * * shift() should be called after each ledger close, it bans transactions
  *   that have associated age greater or equal to pendingDepth and removes
  *   transactions that were banned for more than banDepth ledgers
  *
@@ -140,7 +141,8 @@ class TransactionQueue
     FindResult find(TransactionFramePtr const& tx);
     using ExtractResult = std::pair<PendingTransactions::iterator,
                                     std::vector<TransactionFramePtr>>;
-    ExtractResult extract(TransactionFramePtr const& tx);
+    // keepBacklog: keeps transactions succeding tx in the account's backlog
+    ExtractResult extract(TransactionFramePtr const& tx, bool keepBacklog);
 };
 
 static const char* TX_STATUS_STRING[static_cast<int>(

diff --git a/src/herder/test/TransactionQueueTests.cpp b/src/herder/test/TransactionQueueTests.cpp
@@ -528,13 +528,17 @@ TEST_CASE("TransactionQueue", "[herder][TransactionQueue]")
         test.check({{{account1, 1, {txSeqA1T1, txSeqA1T2}},
                      {account2, 1, {txSeqA2T1, txSeqA2T2}}}});
         test.removeAndReset({txSeqA1T1, txSeqA2T2});
+        test.check({{{account1, 0, {txSeqA1T2}}, {account2, 0, {txSeqA2T1}}}});
+        test.removeAndReset({txSeqA1T2});
         test.check({{{account1}, {account2, 0, {txSeqA2T1}}}});
         test.add(txSeqA1T1, TransactionQueue::AddResult::ADD_STATUS_PENDING);
         test.check({{{account1, 0, {txSeqA1T1}}, {account2, 0, {txSeqA2T1}}}});
         test.add(txSeqA2T2, TransactionQueue::AddResult::ADD_STATUS_PENDING);
         test.check({{{account1, 0, {txSeqA1T1}},
                      {account2, 0, {txSeqA2T1, txSeqA2T2}}}});
         test.removeAndReset({txSeqA2T1});
+        test.check({{{account1, 0, {txSeqA1T1}}, {account2, 0, {txSeqA2T2}}}});
+        test.removeAndReset({txSeqA2T2});
         test.check({{{account1, 0, {txSeqA1T1}}, {account2}}});
         test.removeAndReset({txSeqA1T1});
         test.check({{{account1}, {account2}}});

diff --git a/src/test/test.cpp b/src/test/test.cpp
@@ -185,6 +185,9 @@ getTestConfig(int instanceNumber, Config::TestDbMode mode)
         thisConfig.REPORT_METRICS = gTestMetrics;
         // disable maintenance
         thisConfig.AUTOMATIC_MAINTENANCE_COUNT = 0;
+        // only spin up a small number of worker threads
+        thisConfig.WORKER_THREADS = 2;
+        thisConfig.QUORUM_INTERSECTION_CHECKER = false;
     }
     return *cfgs[instanceNumber];
 }