-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Let the thread which fetched a TX add it to the mempool #4984
base: bolt12/coot/tx-submission
Are you sure you want to change the base?
Let the thread which fetched a TX add it to the mempool #4984
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Add the time it took to add TXs to the mempool to the TxInboundAddedToMempool tracer.
DeltaQ metrics is only available for our warm and hot peers that also have us as hot. So a fraction of all downstream clients will have a metric. This change the ranking of peers to use simple scoring system. Deliver a new TX before in time before it gets into the block gives you one point. Delivering a TXs thats already in the mempool, is invalid, or fail because it was included in a recent blocks gives you a penalty.
4a99a6e
to
a3de87b
Compare
let !accepted = length txidsAccepted | ||
|
||
traceWith tracer $ TraceTxSubmissionProcessed ProcessedTxCount { | ||
ptxcAccepted = accepted | ||
, ptxcRejected = collected - accepted | ||
, ptxcScore = 0 -- This implementatin does not track score |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be a TODO
?
cntRejects :: SharedTxState peeraddr txid tx | ||
-> SharedTxState peeraddr txid tx | ||
cntRejects st@SharedTxState { peerTxStates } = | ||
let peerTxStates' = Map.update (\ps -> Just $! ps { rejectedTxs = min 42 (max (-42) (rejectedTxs ps + n)) }) peeraddr peerTxStates in | ||
st {peerTxStates = peerTxStates'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be more efficient to update it for all peers at once, but it might be harder to implement it. Another way would be to store the counts in Map peerAddt (TVar ...)
, so that we don't block on taking the whole state to update the count just for one peer.
let receivedL = [ (txId tx, tx) | tx <- txs ] | ||
fetchedSet <- consumeFetchedTxs (Set.fromList (map fst receivedL)) | ||
|
||
-- Only attempt to add TXs if we actually has fetched some. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-- Only attempt to add TXs if we actually has fetched some. | |
-- Only attempt to add TXs if we actually have fetched some. |
-- Note that checking if the mempool contains a TX before | ||
-- spending several ms attempting to add it to the pool has | ||
-- been judged immoral. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😁
Use hashWithSalt to breaks ties when ranking peers with equal score. Track rejections in a bucket that slowly drains over time.
Bind responder threads to the lower cores, reserving the higest two cores for other tasks (block adoption, draining mempool etc.).
Make demos and framework sim-tests compile.
Description
Previously all peers that had the TXid of a downloaded TX would attempt to add it to the mempool (a rather costly operation).
This patch moves the addition to the mempool to the Server's CollectTxs state. Which means that the downloaded TXs is only added by the thread that actually downloaded it.
The legacy TX submission protocol has been left as is.
Checklist
Quality
Maintenance
ouroboros-network
project.