order transactions announcements in the order they are received #1435
Conversation
There was a problem hiding this comment.
OK for a quick fix. In general we don't want to use a buffered channel as a queue. A channel buffer greater than 1 is meant for performance rather than a functional requirement.
Out of curiosity, we should also have a look at the heap profile (go tool pprof http://localhost:6060/debug/pprof/heap) to see just what kind of memory just make(chan orderedTxn, txQueueSize) will allocate on construction. It won't be a lot, but it might be surprisingly larger than expected.
Thoughts about what happens if the channel buffer fills? I think it'll hang CE, right? Perhaps it's better to make this a non-blocking channel send (select with a default case).
EDIT: I'm also wondering about, say, 10,000 x 100KB transactions. That's 1GB. When we get a mature solution, I bet we'll want this configurable.
node/nogossip.go
Outdated
| case <-ctx.Done(): | ||
| return | ||
| case txn := <-n.txQueue: | ||
| n.announceRawTx(context.Background(), txn.txID, txn.rawtx, txn.from) |
There was a problem hiding this comment.
This should use ctx from caller I believe since it is now a system level context from Start instead of some other request context with a deadline.
There was a problem hiding this comment.
Ah, I probably don't need to store rawTx in this and pull it from a mempool instead
There was a problem hiding this comment.
Maybe. I think this is alright as is. But if you can do that without wasteful reserialization, I guess so.
charithabandi
force-pushed
the
ordertxns
branch
4 times, most recently
from
11b9312 to
c0ed7aa
Compare
This PR ensures the FIFO consistency to the outgoing transaction announcements to maintain the nonce ordering. So all the transaction announcements are routed through a TxQueue to enforce this order.