Make failure to load UDI when opening an SST a soft failure #13921
Conversation
| RecordTick(rep_->ioptions.statistics.get(), | ||
| SST_USER_DEFINED_INDEX_LOAD_FAIL_COUNT); | ||
| if (table_options.fail_if_no_udi_on_open) { | ||
| s = Status::Corruption("Failed to load UDI: " + udi_name, s.ToString()); |
There was a problem hiding this comment.
Minor: this overwrites the failure where the UDI block is available but creating a reader on it failed. Similarly for below we overwrite the status to ok.
There was a problem hiding this comment.
The original status is captured in the string message. This way, we can provide some additional info such as the index name and file name.
There was a problem hiding this comment.
Ah I missed the s.ToString() part. Do we want to overwrite status to ok when UDI is present but fail to create index reader?
There was a problem hiding this comment.
If fail_if_no_udi_on_open is false, I'm overwriting status to ok no matter why it failed. That's a good question though. Let me double check if ignoring failure only if the UDI is not present in the file is the desired behavior.
In the read path, we continue to return error since the user explicitly requested the UDI. The caller can retry without the UDI if appropriate. |
Sounds good. As long as we have test to cover this. |
|
@anand1976 has imported this pull request. If you are a Meta employee, you can view this in D81826054. |
|
@anand1976 has imported this pull request. If you are a Meta employee, you can view this in D81826054. |
|
@anand1976 merged this pull request in 0044a76. |
If user_defined_index_factory in BlockBasedTableOptions is configured and we try to open an SST file without the corresponding UDI (either during DB open or file ingestion), ignore a failure to load the UDI by default. If fail_if_no_udi_on_open in BlockBasedTableOptions is true, then treat it as a fatal error.
Test plan:
Update unit tests