Skip to content

Commit

Permalink
core/tx_pool: use a ts for each tx in the queue, but only update the …
Browse files Browse the repository at this point in the history 
…heartbeat on promotion or pending replaced
  • Loading branch information
@villanuevawill @holiman
villanuevawill authored and holiman committed Jul 6, 2020
1 parent 6f8a7f6 commit 45334d9
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 16 deletions.
36 changes: 23 additions & 13 deletions core/tx_pool.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,12 @@ type TxPool struct {
locals *accountSet // Set of local transaction to exempt from eviction rules
journal *txJournal // Journal of local transaction to back up to disk

pending map[common.Address]*txList // All currently processable transactions
queue map[common.Address]*txList // Queued but non-processable transactions
beats map[common.Address]time.Time // Last heartbeat from each known account
all *txLookup // All transactions to allow lookups
priced *txPricedList // All transactions sorted by price
pending map[common.Address]*txList // All currently processable transactions
queue map[common.Address]*txList // Queued but non-processable transactions
beats map[common.Address]time.Time // Last heartbeat from each known account
queued_ts map[common.Hash]time.Time // Timestamp for when queued transactions were added
all *txLookup // All transactions to allow lookups
priced *txPricedList // All transactions sorted by price

chainHeadCh chan ChainHeadEvent
chainHeadSub event.Subscription
Expand Down Expand Up @@ -267,6 +268,7 @@ func NewTxPool(config TxPoolConfig, chainconfig *params.ChainConfig, chain block
pending: make(map[common.Address]*txList),
queue: make(map[common.Address]*txList),
beats: make(map[common.Address]time.Time),
queued_ts: make(map[common.Hash]time.Time),
all: newTxLookup(),
chainHeadCh: make(chan ChainHeadEvent, chainHeadChanSize),
reqResetCh: make(chan *txpoolResetRequest),
Expand Down Expand Up @@ -364,8 +366,10 @@ func (pool *TxPool) loop() {
// Any non-locals old enough should be removed
if time.Since(pool.beats[addr]) > pool.config.Lifetime {
for _, tx := range pool.queue[addr].Flatten() {
queuedEvictionMeter.Mark(1)
pool.removeTx(tx.Hash(), true)
if time.Since(pool.queued_ts[tx.Hash()]) > pool.config.Lifetime {
queuedEvictionMeter.Mark(1)
pool.removeTx(tx.Hash(), true)
}
}
}
}
Expand Down Expand Up @@ -661,17 +665,20 @@ func (pool *TxPool) enqueueTx(hash common.Hash, tx *types.Transaction) (bool, er
}
// Discard any previous transaction and mark this
if old != nil {
pool.all.Remove(old.Hash())
old_hash := old.Hash()
pool.all.Remove(old_hash)
pool.priced.Removed(1)
delete(pool.queued_ts, old_hash)
queuedReplaceMeter.Mark(1)
} else {
// Nothing was replaced, bump the queued counter
queuedGauge.Inc(1)
pool.queued_ts[hash] = time.Now()
}
if pool.all.Get(hash) == nil {
pool.all.Add(tx)
pool.priced.Put(tx)
pool.beats[from] = time.Now()
pool.queued_ts[hash] = time.Now()
}
return old != nil, nil
}
Expand Down Expand Up @@ -704,15 +711,14 @@ func (pool *TxPool) promoteTx(addr common.Address, hash common.Hash, tx *types.T
// An older transaction was better, discard this
pool.all.Remove(hash)
pool.priced.Removed(1)

delete(pool.queued_ts, hash)
pendingDiscardMeter.Mark(1)
return false
}
// Otherwise discard any previous transaction and mark this
if old != nil {
pool.all.Remove(old.Hash())
pool.priced.Removed(1)

pendingReplaceMeter.Mark(1)
} else {
// Nothing was replaced, bump the pending counter
Expand All @@ -725,6 +731,7 @@ func (pool *TxPool) promoteTx(addr common.Address, hash common.Hash, tx *types.T
}
// Set the potentially new pending nonce and notify any subsystems of the new tx
pool.beats[addr] = time.Now()
delete(pool.queued_ts, hash)
pool.pendingNonces.set(addr, tx.Nonce()+1)

return true
Expand Down Expand Up @@ -916,10 +923,10 @@ func (pool *TxPool) removeTx(hash common.Hash, outofbound bool) {
if removed, _ := future.Remove(tx); removed {
// Reduce the queued counter
queuedGauge.Dec(1)
delete(pool.queued_ts, hash)
}
if future.Empty() {
delete(pool.queue, addr)
delete(pool.beats, addr)
}
}
}
Expand Down Expand Up @@ -1195,13 +1202,15 @@ func (pool *TxPool) promoteExecutables(accounts []common.Address) []*types.Trans
for _, tx := range forwards {
hash := tx.Hash()
pool.all.Remove(hash)
delete(pool.queued_ts, hash)
}
log.Trace("Removed old queued transactions", "count", len(forwards))
// Drop all transactions that are too costly (low balance or out of gas)
drops, _ := list.Filter(pool.currentState.GetBalance(addr).Uint64(), pool.currentMaxGas)
for _, tx := range drops {
hash := tx.Hash()
pool.all.Remove(hash)
delete(pool.queued_ts, hash)
}
log.Trace("Removed unpayable queued transactions", "count", len(drops))
queuedNofundsMeter.Mark(int64(len(drops)))
Expand All @@ -1224,6 +1233,7 @@ func (pool *TxPool) promoteExecutables(accounts []common.Address) []*types.Trans
for _, tx := range caps {
hash := tx.Hash()
pool.all.Remove(hash)
delete(pool.queued_ts, hash)
log.Trace("Removed cap-exceeding queued transaction", "hash", hash)
}
queuedRateLimitMeter.Mark(int64(len(caps)))
Expand All @@ -1237,7 +1247,6 @@ func (pool *TxPool) promoteExecutables(accounts []common.Address) []*types.Trans
// Delete the entire queue entry if it became empty.
if list.Empty() {
delete(pool.queue, addr)
delete(pool.beats, addr)
}
}
return promoted
Expand Down Expand Up @@ -1422,6 +1431,7 @@ func (pool *TxPool) demoteUnexecutables() {
// Delete the entire pending entry if it became empty.
if list.Empty() {
delete(pool.pending, addr)
delete(pool.beats, addr)
}
}
}
Expand Down
57 changes: 54 additions & 3 deletions core/tx_pool_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ func validateTxPoolInternals(pool *TxPool) error {
if priced := pool.priced.items.Len() - pool.priced.stales; priced != pending+queued {
return fmt.Errorf("total priced transaction count %d != %d pending + %d queued", priced, pending, queued)
}
if queued != len(pool.queued_ts) {
return fmt.Errorf("total queued transaction count %d != %d queued_ts length", queued, len(pool.queued_ts))
}

// Ensure the next nonce to assign is the correct one
for addr, txs := range pool.pending {
// Find the last transaction
Expand Down Expand Up @@ -972,13 +976,20 @@ func testTransactionQueueTimeLimiting(t *testing.T, nolocals bool) {
t.Fatalf("failed to add remote transaction: %v", err)
}

// wait a short amount of time to add an additional future queued item to test proper eviction when
// pending is removed
time.Sleep(2 * evictionInterval)
if err := pool.addRemoteSync(pricedTransaction(5, 100000, big.NewInt(1), remote)); err != nil {
t.Fatalf("failed to add remote transaction: %v", err)
}

// Make sure future queue and pending have transactions
pending, queued = pool.Stats()
if pending != 2 {
t.Fatalf("pending transactions mismatched: have %d, want %d", pending, 2)
}
if queued != 2 {
t.Fatalf("queued transactions mismatched: have %d, want %d", queued, 2)
if queued != 3 {
t.Fatalf("queued transactions mismatched: have %d, want %d", queued, 3)
}
if err := validateTxPoolInternals(pool); err != nil {
t.Fatalf("pool internal state corrupted: %v", err)
Expand All @@ -998,12 +1009,52 @@ func testTransactionQueueTimeLimiting(t *testing.T, nolocals bool) {
if pending != 0 {
t.Fatalf("pending transactions mismatched: have %d, want %d", pending, 0)
}
if queued != 2 {
if queued != 3 {
t.Fatalf("queued transactions mismatched: have %d, want %d", queued, 2)
}
if err := validateTxPoolInternals(pool); err != nil {
t.Fatalf("pool internal state corrupted: %v", err)
}

// Wait for the lifetime to run for all transactions except the one that was added later
time.Sleep(evictionInterval * 7)
pending, queued = pool.Stats()
if pending != 0 {
t.Fatalf("pending transactions mismatched: have %d, want %d", pending, 0)
}
if nolocals {
if queued != 1 {
t.Fatalf("queued transactions mismatched: have %d, want %d", queued, 1)
}
} else {
if queued != 2 {
t.Fatalf("queued transactions mismatched: have %d, want %d", queued, 2)
}
}

if err := validateTxPoolInternals(pool); err != nil {
t.Fatalf("pool internal state corrupted: %v", err)
}

// lifetime should pass for the final transaction
time.Sleep(evictionInterval * 2)

pending, queued = pool.Stats()
if pending != 0 {
t.Fatalf("pending transactions mismatched: have %d, want %d", pending, 0)
}
if nolocals {
if queued != 0 {
t.Fatalf("queued transactions mismatched: have %d, want %d", queued, 2)
}
} else {
if queued != 1 {
t.Fatalf("queued transactions mismatched: have %d, want %d", queued, 0)
}
}
if err := validateTxPoolInternals(pool); err != nil {
t.Fatalf("pool internal state corrupted: %v", err)
}
}

// Tests that even if the transaction count belonging to a single account goes
Expand Down

0 comments on commit 45334d9

Please sign in to comment.