[ADDED] Add account-level connection stats#6967
Conversation
|
I will fix the test.. it is an existing one that is doing a contains string match for the existing accstatz payload. |
bruth
force-pushed
the
byron/account-conn-stats
branch
from
e0bc201 to
d89f4e1
Compare
bruth
force-pushed
the
byron/account-conn-stats
branch
from
e72e3b4 to
a8d80b1
Compare
bruth
force-pushed
the
byron/account-conn-stats
branch
2 times, most recently
from
79bc06d to
64f9af2
Compare
server/client.go
Outdated
| if acc != nil { | ||
| atomic.AddInt64(&acc.inMsgs, int64(c.in.msgs)) | ||
| atomic.AddInt64(&acc.inBytes, int64(c.in.bytes)) | ||
| acc.smu.Lock() |
There was a problem hiding this comment.
Even though for larger more complex structs that could have multiple locks we use mu, for small simple structs you can leave it unamed and just do a lock direct on the struct. So if stats is always non-nil in acc, then you could do acc.stats.Lock()
There was a problem hiding this comment.
There are four stats structs on the acc now. The existing one is embedded and the new ones, ln, gw, and rt. The choice for the smu was for a single lock when any/all of them need to be updated in one shot vs. individual locks.
The alternative is to refactor this to encapsulate the stats together as one unit (new struct) with a lock.
There was a problem hiding this comment.
I think we should do this.
There was a problem hiding this comment.
Have refactored the struct shape a bit.
server/gateway.go
Outdated
| if acc != nil { | ||
| atomic.AddInt64(&acc.outMsgs, dlvMsgs) | ||
| atomic.AddInt64(&acc.outBytes, totalBytes) | ||
| acc.smu.Lock() |
There was a problem hiding this comment.
Maybe acc needs updateStats(msgs, bytes, rmsgs, rbytes, gwmsgs, gwbytes, lnmsgs, lnbytes)?
There was a problem hiding this comment.
I had a go at doing this, but it needs to be split over two functions for inbound vs outbound, and I couldn't come up with a shape that felt good at the callsites. For now I think the current approach is clearer.
This adds tracking of msgs and bytes for route, gateway, and leaf connections. Signed-off-by: Byron Ruth <byron@nats.io> Signed-off-by: Neil Twigg <neil@nats.io>
neilalexander
force-pushed
the
byron/account-conn-stats
branch
from
64f9af2 to
a27cf43
Compare
wallyqs
changed the title
This was introduced in #6967 to support gateway, route, and leaf stats. However the DataStats type is used for client connection stats (e.g. disconnect event) for which the additional fields are not relevant. Having a pointer ensures the omitempty JSON tag is respected and those nil fields are not serialized. Signed-off-by: Byron Ruth <byron@nats.io>
This was introduced in #6967 to support gateway, route, and leaf stats. However the DataStats type is used for client connection stats (e.g. disconnect event) for which the additional fields are not relevant. Having a pointer ensures the omitempty JSON tag is respected and those nil fields are not serialized. Signed-off-by: Byron Ruth <byron@nats.io>
This was introduced in #6967 to support gateway, route, and leaf stats. However the DataStats type is used for client connection stats (e.g. disconnect event) for which the additional fields are not relevant. Having a pointer ensures the omitempty JSON tag is respected and those nil fields are not serialized. Signed-off-by: Byron Ruth <byron@nats.io>
This was introduced in #6967 to support gateway, route, and leaf stats. However the DataStats type is used for client connection stats (e.g. disconnect event) for which the additional fields are not relevant. Having a pointer ensures the omitempty JSON tag is respected and those nil fields are not serialized. Signed-off-by: Byron Ruth <byron@nats.io>
3 participants
This adds tracking of msgs and bytes for route, gateway, and leaf connections.
More tests need to be added, but I wanted to get a proposal of the idea prepared for initial review in case there are any glaring issues.
Signed-off-by: Byron Ruth byron@nats.io