Improve error handling and logging for Retryable IO Errors about kvrocks HOT 13 CLOSED

@VasuDevrani Looks good, you can go ahead to improve this.

from kvrocks.

do we have ability to recovery from error if Resume() failed for one-times?

There might be some cases (disk no space?) where it needs to be done externally for the Resume() to succeed.

from kvrocks.

@VasuDevrani Cool, don't hesitate to raise if you have any ideas.

from kvrocks.

@VasuDevrani Looks good, you can go ahead to improve this.

See, the code proposed as solution just prints ERROR in else block.
How about we add a set number of retry for resuming DB operation in case of failure, and if it still fails after retries, then we log a CRITICAL error finally.

from kvrocks.

I'm wondering if it's a good idea to do that. I prefer letting users determine whether to terminate themself instead of by default N times retry. @PragmaTwice @caipengbo @torwig What do you think?

from kvrocks.

Yes, I also don't think it's necessary to set a number of retries. Alternatively, we could provide a RESUME command, which would give administrators more options.

from kvrocks.

Out-of curiousity, do we have ability to recovery from error if Resume() failed for one-times? Seems we can only recover from merely case like Compaction output error or flush error?

from kvrocks.

Thats a good point, so we should let user decide what to do during this case...

from kvrocks.

@VasuDevrani As discussed above, improving the logging message is good, but don't expect to escalate the FATAL error after N time tries. What do you think?

from kvrocks.

@VasuDevrani As discussed above, improving the logging message is good, but don't expect to escalate the FATAL error after N time tries. What do you think?

Yeah I've considered above conversation. I'm learning a bit more about Kvrocks, its internals. will update here with my thoughts after a while. Thanks for following up.

from kvrocks.

@VasuDevrani As discussed above, improving the logging message is good, but don't expect to escalate the FATAL error after N time tries. What do you think?

I agree with just improving the logging message for now. (should i make continue with a PR?)

@VasuDevrani Cool, don't hesitate to raise if you have any ideas.

Later, if need arise we can go with idea of having RESUME command for admins as suggested by @caipengbo
or, we can add a configuration option for max_number_of_retry for this operation

from kvrocks.

I agree with just improving the logging message for now. (should i make continue with a PR?)

Sure, you can continue improving the logging message.

from kvrocks.

I agree with just improving the logging message for now. (should i make continue with a PR?)

Yes, RESUME is not a high priority.

from kvrocks.