MINOR: improve Streams error message#5975
Conversation
|
Found this by chance. Error message seems to be miss leading. Call for review @guozhangwang @bbejeck @vvcephei |
guozhangwang
left a comment
guozhangwang
left a comment
There was a problem hiding this comment.
One nit. Feel free to merge afterwards.
There was a problem hiding this comment.
nit: how about the underlying RocksDB store for this iterator has closed?
There was a problem hiding this comment.
open does not indicate if the underlying store is closed or not -- it only indicates if the iterator was closed.
(Of course, if the store gets closed, the iterator will be closed, too, but if open==false we don't know if the store is closed or not.)
|
@guozhangwang Added a new log statement. |
There was a problem hiding this comment.
Why do we want to log it as WARN?
There was a problem hiding this comment.
For our own code base, this should never happen. For IQ, it's a different story. I though it might be helpful to expose potential bugs in our code base? We can also log at debug level if you prefer.
There was a problem hiding this comment.
Maybe we could put this to INFO but change the message slightly to say "This could be an issue for IQ" or something along those lines. Just a thought.
There was a problem hiding this comment.
I think @guozhangwang would prefer less verbose log level like DEBUG? Not sure.
Like the idea to mention IQ explicitly.
There was a problem hiding this comment.
I don't have a strong opinion on the log level DEBUG is fine with me.
There was a problem hiding this comment.
@mjsax I think I'm sold on your arguments, let's keep them as WARN then :)
There was a problem hiding this comment.
Maybe we could put this to INFO but change the message slightly to say "This could be an issue for IQ" or something along those lines. Just a thought.
guozhangwang
left a comment
guozhangwang
left a comment
There was a problem hiding this comment.
Please feel free to merge.
mjsax
force-pushed
the
fix-rockdb-iterator-error-msg
branch
from
14ed5ab to
ce07a4c
Compare
|
Thanks @guozhangwang and @bbejeck Rebased on |
|
Retest this please |
1 similar comment
|
Retest this please |
mjsax
force-pushed
the
fix-rockdb-iterator-error-msg
branch
from
ce07a4c to
b1e7a87
Compare
Reviewers: Bill Bejeck <bill@confluent.io>, Guozhang Wang <guozhang@confluent.io>
3 participants
No description provided.