Open aravind-wdc opened 2 years ago
This is very similar to #9299. With the change for #9333, the code will no longer return NotSupported if there is a factory but instead return InvalidArgument. This will fix the errors for "ignore_unsupported_options=false" reported by this issue, but not if "ignore_unknown_options=true" (which still will fail).
For the places that call CreateFromUri in the code currently (db_bench_tool, ldb_cmd, sst_dump_tool, testutil), it probably makes sense for them to set ignore_unsupported_options=false in general, making option 3 a reasonable change. I do not know what happens if those calls also set ignore_unknown_options=false, but that might be a reasonable thing to do as well.
Regardless, there should not be a crash. I will attempt to reproduce the crash and see what I can figure out.
Thanks @mrambacher. Looks like #9333 will solve this issue. If it is going to be pulled in shortly, I think we are ok. Else, I can send a PR based on option 3, as discussed above.
Hi,
I tried to send an email to https://groups.google.com/forum/#!forum/rocksdb, as this is more of a discussion/question but my email client could not resolve the recipient, so posting here.
Issue:
Rootcause: The factory function called by
NewObject()
do not allow for returning error codes from underlying filesystems and environments, it always returns a object pointer, if the pointer is null then it is returned asStatus::NotSupported
Since theConfigOption.ignore_unsupported_options
is true by default, underlying layers returns Status::OK instead of the error.In case of ZenFS plugin, it identifies the error in the string, updates the errormsg and returns a null pointer. But it gets translated to Status::NotSupported("errmsg"), and since the "ignore_unsupported_options" in ConfigOptions is true by default, the error is is ignored and it gets converted to Status::OK in customizable_util.h L68, and results in a crash when the apps(db-bench) subsequently try to make the filesystem calls.
Ideal Solution: Change the signature of the factory function which is called from NewObject, so that it can return error codes (not just a null or a valid pointer) and these error codes could be propagated to upper layers. But this may break things for other users. Would such a change PR be acceptable ? So what are your thoughts on this ? Is there any other way to fix it?
Other Probable Quick Fixes and side-effects:
I have a patch ready for option 3.
Thanks Aravind