-
Notifications
You must be signed in to change notification settings - Fork 474
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
txHandler: add more metric #4786
Changes from 3 commits
f79a445
6d1f0ad
f234c96
a12fe42
5fd63cb
96d46e5
e5bcf91
b9a1d9b
17a100b
9188690
937818e
2c29d72
ab035ca
d998917
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1440,11 +1440,13 @@ func (wn *WebsocketNetwork) peerSnapshot(dest []*wsPeer) ([]*wsPeer, int32) { | |
|
||
// preparePeerData prepares batches of data for sending. | ||
// It performs optional zstd compression for proposal massages | ||
func (wn *WebsocketNetwork) preparePeerData(request broadcastRequest, prio bool, peers []*wsPeer) ([][]byte, [][]byte, []crypto.Digest) { | ||
func (wn *WebsocketNetwork) preparePeerData(request broadcastRequest, prio bool, peers []*wsPeer) ([][]byte, [][]byte, []crypto.Digest, map[protocol.Tag]struct{}) { | ||
// determine if there is a payload proposal and peers supporting compressed payloads | ||
wantCompression := false | ||
var messageTags map[protocol.Tag]struct{} | ||
if prio { | ||
wantCompression = checkCanCompress(request, peers) | ||
messageTags = make(map[protocol.Tag]struct{}, 1) | ||
} | ||
|
||
digests := make([]crypto.Digest, len(request.data)) | ||
|
@@ -1463,8 +1465,11 @@ func (wn *WebsocketNetwork) preparePeerData(request broadcastRequest, prio bool, | |
digests[i] = crypto.Hash(mbytes) | ||
} | ||
|
||
if prio && request.tags[i] == protocol.ProposalPayloadTag { | ||
networkPrioPPNonCompressedSize.AddUint64(uint64(len(d)), nil) | ||
if prio { | ||
if request.tags[i] == protocol.ProposalPayloadTag { | ||
Comment on lines
+1467
to
+1468
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it necessary to make two levels of if? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, we want containsPrioPPTag |
||
networkPrioPPNonCompressedSize.AddUint64(uint64(len(d)), nil) | ||
} | ||
messageTags[request.tags[i]] = struct{}{} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't get it, what is this for? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is for correcting the PP metric. without it it counts all prio messages, not only PP |
||
} | ||
|
||
if wantCompression { | ||
|
@@ -1482,7 +1487,7 @@ func (wn *WebsocketNetwork) preparePeerData(request broadcastRequest, prio bool, | |
} | ||
} | ||
} | ||
return data, dataCompressed, digests | ||
return data, dataCompressed, digests, messageTags | ||
} | ||
|
||
// prio is set if the broadcast is a high-priority broadcast. | ||
|
@@ -1499,7 +1504,7 @@ func (wn *WebsocketNetwork) innerBroadcast(request broadcastRequest, prio bool, | |
} | ||
|
||
start := time.Now() | ||
data, dataWithCompression, digests := wn.preparePeerData(request, prio, peers) | ||
data, dataWithCompression, digests, seenPrioTags := wn.preparePeerData(request, prio, peers) | ||
|
||
// first send to all the easy outbound peers who don't block, get them started. | ||
sentMessageCount := 0 | ||
|
@@ -1515,12 +1520,16 @@ func (wn *WebsocketNetwork) innerBroadcast(request broadcastRequest, prio bool, | |
// if this peer supports compressed proposals and compressed data batch is filled out, use it | ||
ok = peer.writeNonBlockMsgs(request.ctx, dataWithCompression, prio, digests, request.enqueueTime) | ||
if prio { | ||
networkPrioBatchesPPWithCompression.Inc(nil) | ||
if _, ok := seenPrioTags[protocol.ProposalPayloadTag]; ok { | ||
algonautshant marked this conversation as resolved.
Show resolved
Hide resolved
|
||
networkPrioBatchesPPWithCompression.Inc(nil) | ||
} | ||
} | ||
} else { | ||
ok = peer.writeNonBlockMsgs(request.ctx, data, prio, digests, request.enqueueTime) | ||
if prio { | ||
networkPrioBatchesPPWithoutCompression.Inc(nil) | ||
if _, ok := seenPrioTags[protocol.ProposalPayloadTag]; ok { | ||
networkPrioBatchesPPWithoutCompression.Inc(nil) | ||
} | ||
} | ||
} | ||
if ok { | ||
|
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.
I think txnBatchPrep is already returning ErrTxGroupError, maybe you would just want to keep the error it created (not make a new one), which preserves .Reason, and set .err on it to have this wrapped fmt?