Allow servers to send and receive messages directly #819
Conversation
Signed-off-by: Derek Collison <derek@nats.io>
Signed-off-by: Derek Collison <derek@nats.io>
server/events.go
Outdated
| acc := s.sys.account | ||
|
|
||
| // Prep internl structures needed to send message. | ||
| c.pa.subject = []byte(subj) |
There was a problem hiding this comment.
It looks to be that sendInternalMsg() can be called concurrently, but it looks here that we use a single client object stored in server struct. So there should be theoretically several go routines changing the c.pa.subject/size/etc..?
Did you mean to have this function be protected by some lock?
There was a problem hiding this comment.
Let me look into it. c.pa not setup to be shared as you know but you raise a good point.
There was a problem hiding this comment.
Yes, but in this case, we use client (which is s.sys.client) from various go routines, each time we want to produce a sys event. So this will be shared. Even the rest (calling processMsgResults, etc..) I think should be protected with a mutex.
There was a problem hiding this comment.
I understand. I had to do the pa.(..) stuff at the end to make it work, I think I have a comment somewhere in there that say something about coordination around outbound seq, etc. I could create a new mutex or spin up a Go routine that does the actual sending. I think I prefer the second option since we don't know all that is getting locked below us in process msg results.
There was a problem hiding this comment.
I personally would go with a dedicated lock, say in the internal structure. Spinning go routing to parse result, etc.. will cause out of order deliveries.
There was a problem hiding this comment.
(maybe you meant a single go-routine). Also were you referring to the situation of when dealing with an event we could end-up generating another kind of event in reaction of the first?
There was a problem hiding this comment.
Would just simulate readLoop like we have for normal clients.
There was a problem hiding this comment.
Yes just single Go routine.
Signed-off-by: Derek Collison <derek@nats.io>
Signed-off-by: Derek Collison <derek@nats.io>
server/events.go
Outdated
| if s.sys == nil { | ||
| return | ||
| } | ||
| s.sys.sendq <- &pubMsg{r, sub, msg} |
There was a problem hiding this comment.
Should we use a select? Are we ok if we lose those messages?
As long as we know for a fact that there won't be any locking causing the "sendLoop" to be blocked by the code calling sendInternalMsg() when chan buffer is full, I guess we are ok, otherwise, use combination of buffered channel and select, or move to list.
There was a problem hiding this comment.
Its a buffered channel now.
There was a problem hiding this comment.
yes, I saw it was a buffered of 128, but say that there is rapid generation of events that fill the buffered channel, then the sendloop is accessing a lock that another routine is holding trying to send to the channel. that's what I meant. If we know for sure that this situation will never occur, then we are fine.
There was a problem hiding this comment.
We don't hold any locks when placing on the sendq or pulling off.
There was a problem hiding this comment.
Did a change though to make more consistent.
Signed-off-by: Derek Collison <derek@nats.io>
Added ability for servers to send and receive messages as their own entity.
Added account scoped events for connection created and connection closed.
Signed-off-by: Derek Collison derek@nats.io
/cc @nats-io/core