Improved notification for direct-io logging #2519
Conversation
| @@ -47,6 +48,13 @@ impl RocksError { | |||
| { | |||
| Self::DbLocked(err) | |||
| } else { | |||
| if err_message.contains("Direct I/O is not supported") { | |||
There was a problem hiding this comment.
I'd be inclined to have a check at the top level (ie else if) and then return a new named error Self::DirectIOUnsupported(err) as we do in the case of it being locked - then the instructions for what to do can be part of the error message. But i defer to @AhmedSoliman for opinions
There was a problem hiding this comment.
That sounds a more robust approach, I guess I wanted to be as little intrusive as possible 🤣
There was a problem hiding this comment.
No question that what @jackkleeman suggested is a nicer approach. I'm okay with either approaches considering that this is quite a niche case. Perhaps we go with what @edmondop proposed and when this list of error expands we can bubble them up.
The reason I don't feel strongly about it is because it wouldn't impact the higher layer in this case.
|
Agreed @AhmedSoliman , after thinking about it, here the point is not so much to have a separate error type to match it upstream and take more precise behavior, but simply provide a more verbose explanation of this error |
|
Thanks @edmondop for making Restate better. |
See #2505