[ADDED] Support for route permissions config reload #753
Conversation
Signed-off-by: Ivan Kozlovic <[email protected]>
server/reload.go
Outdated
|
|
||
| // Go through all local subscriptions | ||
| for _, sub := range localSubs { | ||
| sub.client.mu.Lock() |
There was a problem hiding this comment.
I used to have a flag in the sub to know if we already sent a SUB to routes, to know if we should or not send it again. But that wasn't too good, but explains why I had the lock here. Will remove (although we access sub.subject, but this is immutable).
server/reload.go
Outdated
| route.sendInfo(infoJSON) | ||
| // Now send SUB and UNSUB protocols as needed. | ||
| for _, sub := range subsNeedSUB { | ||
| route.queueOutbound([]byte(fmt.Sprintf(subProto, sub.subject, sub.queue, routeSid(sub)))) |
There was a problem hiding this comment.
These fmt can be expensive if we have alot of sub/unsubs. Might be good to do some high sub benchmarking to see if we want to change out how these loops work.
There was a problem hiding this comment.
What would be the alternative, though?
There was a problem hiding this comment.
Format by hand, avoid fmt.Sprintf, use a large staged buffer or something and only send occasionally to the socket etc.
There was a problem hiding this comment.
I thought queueOutbound would do that (not sending right away), hence presence of signal. But passed buffer may not be copied, so we can't "reuse" a stack buffer to build the protocol, right?
There was a problem hiding this comment.
It will reference (with sketchy ownership) if data passed in is larger than maxBufSize. Otherwise it copies it. We should just measure it and if there is a concern put a TODO comment in the code.
There was a problem hiding this comment.
Will do, but understand that this is no different than route sending local subs interest on route connect.
There was a problem hiding this comment.
Need to optimize that one too IMO, and I will most likely during rewrite.. :)
- Removed un-needed lock/unlock - Buffer SUBs/UNSUBs protocols and ensure flushing when buffer gets to a certain size (otherwise route would get disconnected with high number of subscriptions) Signed-off-by: Ivan Kozlovic <[email protected]>
- Use stack buffers - Ensure that buffer size is no greater than 90% of max_pending - Added test with low max_pending Signed-off-by: Ivan Kozlovic <[email protected]>
server/route.go
Outdated
| _buf [staticBufSize]byte // array on stack | ||
| buf = _buf[:0] // our buffer will initially point to the stack buffer | ||
| mbs = staticBufSize // max size of the buffer | ||
| mpMax = int(c.out.mp * 90 / 100) // 90% of max_pending |
Signed-off-by: Ivan Kozlovic <[email protected]>
|
@derekcollison Made the change. Let me know if ok to merge as-is or want me to squash. |
|
Merge as is I think. |
Signed-off-by: Ivan Kozlovic [email protected]