Fix DeleteFile error handling in SstFileWriter::Finish (facebook#13776)

anand1976 · facebook-github-bot · commit f6841d1e68eb · 2025-07-14T18:34:56.000-07:00
Summary: In SstFileWriter::Finish, the call to DeleteFile to delete the output file in case of an error may fail. The current behavior is to ignore the error. In stress tests, there may be expected failures due to error injection. Not acting on the return status will cause the ASSERT_STATUS_CHECKED test to fail, so silence it. Pull Request resolved: facebook#13776 Reviewed By: mszeszko-meta Differential Revision: D78307124 Pulled By: anand1976 fbshipit-source-id: d27d9397c15cac5cb33b27094c9123a3fde7fa24
diff --git a/table/sst_file_writer.cc b/table/sst_file_writer.cc
@@ -497,9 +497,9 @@ Status SstFileWriter::Finish(ExternalSstFileInfo* file_info) {
   }
   if (!s.ok()) {
     Status status = r->ioptions.env->DeleteFile(r->file_info.file_path);
-    // Silence ASSERT_STATUS_CHECKED warning
-    assert(status.ok());
-    ;
+    // Silence ASSERT_STATUS_CHECKED warning, since DeleteFile may fail under
+    // some error injection, and we can just ignore the failure
+    status.PermitUncheckedError();
   }
 
   if (file_info != nullptr) {

Original file line number	Diff line number	Diff line change
`@@ -497,9 +497,9 @@ Status SstFileWriter::Finish(ExternalSstFileInfo* file_info) {`
`497`	`497`	`}`
`498`	`498`	`if (!s.ok()) {`
`499`	`499`	`Status status = r->ioptions.env->DeleteFile(r->file_info.file_path);`
`500`		`- // Silence ASSERT_STATUS_CHECKED warning`
`501`		`- assert(status.ok());`
`502`		`- ;`
	`500`	`+ // Silence ASSERT_STATUS_CHECKED warning, since DeleteFile may fail under`
	`501`	`+ // some error injection, and we can just ignore the failure`
	`502`	`+ status.PermitUncheckedError();`
`503`	`503`	`}`
`504`	`504`
`505`	`505`	`if (file_info != nullptr) {`