Change Channel ID not known log message level#314
Change Channel ID not known log message level#314glebkin wants to merge 1 commit intoFDio:masterfrom
Conversation
…iour and not an error. Signed-off-by: Gleb Kogtev <gleb.kogtev@gmail.com>
|
Hello @glebkin, GoVPP dump (i.e., multirequest) tells the VPP to stream the given items and send the control ping at the end. On the GoVPP side, messages are received by Now, GoVPP watches all received messages and does "message callback", where the appropriate channel for the reply is looked for. If the stream is closed before the dump loop is finished, it won't prevent receiving the rest of the messages, but it removes the assigned channel. The callback then cannot find it, and you see the error you described. In most cases, there is no performance gain from breaking the dump loop. You can do is to check the api you are using if any filters are available (for example, To your PR, hiding the error message like that is not desired, and it won't solve the underlying problem. If you have any questions, feel free to ask. |
| l.Errorf("Channel ID not known, ignoring the message: %T %+v", msg, msg) | ||
| // This can happen when a stream was closed explicitly before the last message received. | ||
| // It's expected and not an error. | ||
| if log.Level >= logrus.DebugLevel { |
There was a problem hiding this comment.
You should have included the part above that does decoding as that is actually what can have performance hit.
|
@VladoLavor is correct on some the reasoning. However, I've seen this too many times as well and I believe there are many uses that only need single value (no way to filter) and then skip reading rest. In these cases if we look at it, it extra boilerplate for the users to write reading the rest of data, which can easily be handled discreetly by GoVPP. This definitely should not be an error level thing in such cases. Shooting a bit here, but perhaps we could differentiate between cases where these "pre-maturely closed streams" and those cases where such stream (channel id) is totally unknown. We would have to keep those channels in memory and not reuse their ID, but for how long is another problem. Perhaps a middle ground could be changing the log level to warning and also adding some opt-in variable that could control the log level - allowing users to set debug level at start of their application (in init or main, etc). Let me know what you think @glebkin and thanks for your contribution. |
We're using GoVPP and have a lot of cases dumping from VPP using streams. Sometimes we need to find only one single item and close stream after that without reading it till the end. It leads to tonns of errors in logs like: Channel ID not known, ignoring the message: *memclnt.ControlPingReply
So I would like to change "Channel ID not known" log message level, as it looks like expected behaviour and not an error.