[ADDED] Ability to retrieve client ID from server currently connected to #395
Conversation
Server 1.2.0 returns the client's ID (cid) through the INFO protocol. An API is added to retrieve this value. This can be useful for debugging and monitoring. Resolves #394 Signed-off-by: Ivan Kozlovic <[email protected]>
|
@derekcollison Not sure of the naming |
nats.go
Outdated
| // client reconnects. Also, returned value may be 0 if connecting | ||
| // to a server pre 1.2.0, which did not send the CID back to the | ||
| // client. | ||
| func (nc *Conn) GetCID() (uint64, error) { |
nats.go
Outdated
|
|
||
| // GetCID returns the client ID assigned by the server the client | ||
| // is currently connected to. Note that the value would change if | ||
| // client reconnects. Also, returned value may be 0 if connecting |
There was a problem hiding this comment.
Should return an error if not provided.
There was a problem hiding this comment.
Done, please comment on error variable name and content
| func (nc *Conn) GetCID() (uint64, error) { | ||
| nc.mu.Lock() | ||
| defer nc.mu.Unlock() | ||
| if nc.isClosed() { |
There was a problem hiding this comment.
If 0 return an error that it was not provided.
| } | ||
| defer nc2.Close() | ||
|
|
||
| // Stop server A, nc1 will reconnect to B, and should have different CID |
There was a problem hiding this comment.
Could be the same since CID is scoped to the server.
There was a problem hiding this comment.
No since I made sure that there was another client connected to server B prior to killing A and having nc1 reconnect to server B.
kozlovic
changed the title
Signed-off-by: Ivan Kozlovic <[email protected]>
|
@derekcollison Made some changes and also added a test with fake server returning no CID and making sure GetClientID() returns proper error. |
nats.go
Outdated
| @@ -93,6 +93,7 @@ var ( | |||
| ErrInvalidArg = errors.New("nats: invalid argument") | |||
| ErrInvalidContext = errors.New("nats: invalid context") | |||
| ErrNoEchoNotSupported = errors.New("nats: no echo option not supported by this server") | |||
| ErrNoClientIDReturned = errors.New("nats: client ID not returned by this server") | |||
There was a problem hiding this comment.
I would say not supported, similar to the no echo option above.
| nc.mu.Lock() | ||
| defer nc.mu.Unlock() | ||
| if nc.isClosed() { | ||
| return 0, ErrConnectionClosed | ||
| } | ||
| if nc.info.CID == 0 { |
There was a problem hiding this comment.
Any better way to make sure this was not included in the INFO json? I doubt we would wrap an uint64 but if we did I think it would show up as a zero.
There was a problem hiding this comment.
Then the server should make sure that if it wraps, then it start again to 1.
I guess we could search "client_id" in the info string we get before doing unmarshal and if not present have a flag in connection that says cid not set?
There was a problem hiding this comment.
Maybe post an issue against the server.
There was a problem hiding this comment.
Ok, is it really worth it? I mean at 10M new connections a second, it would take 58,494 years without restarting the server before it wraps (if my math is correct).
There was a problem hiding this comment.
Ok, so will change the error name/content and update
Signed-off-by: Ivan Kozlovic <[email protected]>
Server 1.2.0 returns the client's ID (cid) through the INFO protocol.
An API is added to retrieve this value. This can be useful for
debugging and monitoring.
Resolves #394
Signed-off-by: Ivan Kozlovic [email protected]