history: remove flush()/fsync() and don't write after each appended item

Commit 5c0fddae70 (Refactor history flushing, 2025-03-28) made three changes:
1. call fsync() when we are finished writing the history file.
2. when appending to (as opposed to vacuuming) history, call write(2)
   (followed by flush() and sync()) for each item. Previously, we'd only
   call write(2) if our 64k buffer was full, or after processing the last
   history item.
3. actually check the return value of flush() (which would retry when flushing
   fails -- but std::fs::File::flush() never fails!).

The motivation was to potentially fix #10300 which didn't succeed (see
https://github.com/fish-shell/fish-shell/issues/10300#issuecomment-2876718382).

As for 1 and 2, I don't think the way we use fsync really helps, and flushing
eagerly should not make a difference.

As for 3, there are some explanations in comments, commit message and a [PR
comment](https://github.com/fish-shell/fish-shell/pull/11330#discussion_r2020171339).
To summarize, 5c0fddae70 wants to address the scenario where file.flush()
fails. Prior to that commit we would ostensibly carry on with a corrupted
"first_unwritten_new_item_index" (corrupted because it doesn't match what's
written to disk), which can cause various issues.  However this doesn't
ever happen because std::fs::File::flush() never fails because it doesn't
do anything -- std::fs::File::write() does not buffer writes, it always
delegates to write(2).

There can definitely be scenarios like the one described in
https://github.com/fish-shell/fish-shell/pull/11330#discussion_r2020171339
where the disk is full. In that case, either write(2) fails, which we
already check.  Or close(3p) fails with EIO, which we have never checked. We
should probably check that.

Undo all three changes for now.

Closes #11495

src/history.rs

@@ -126,20 +126,15 @@ pub enum SearchDirection {
 pub const VACUUM_FREQUENCY: usize = 25;
 
 /// Output the contents `buffer` to `file` and clear the `buffer`.
-fn drain_buffer_into_file_no_flush(buffer: &mut Vec<u8>, file: &mut File) -> std::io::Result<()> {
+fn flush_to_file(buffer: &mut Vec<u8>, file: &mut File, min_size: usize) -> std::io::Result<()> {
+    if buffer.is_empty() || buffer.len() < min_size {
+        return Ok(());
+    }
     file.write_all(buffer)?;
     buffer.clear();
     Ok(())
 }
 
-/// Output the contents `buffer` to `file` and clear the `buffer`.
-/// Flush the file and sync it to ensure that the updates actually reach the file system.
-fn drain_buffer_into_file_and_flush(buffer: &mut Vec<u8>, file: &mut File) -> std::io::Result<()> {
-    drain_buffer_into_file_no_flush(buffer, file)?;
-    file.flush()?;
-    file.sync_all()
-}
-
 struct TimeProfiler {
     what: &'static str,
     start: SystemTime,
@@ -597,15 +592,13 @@ fn rewrite_to_temporary_file(
         let mut buffer = Vec::with_capacity(HISTORY_OUTPUT_BUFFER_SIZE + 128);
         for item in items {
             append_history_item_to_buffer(&item, &mut buffer);
-            if buffer.len() >= HISTORY_OUTPUT_BUFFER_SIZE {
-                if let Err(e) = drain_buffer_into_file_no_flush(&mut buffer, dst) {
-                    err = Some(e);
-                    break;
-                }
+            if let Err(e) = flush_to_file(&mut buffer, dst, HISTORY_OUTPUT_BUFFER_SIZE) {
+                err = Some(e);
+                break;
             }
         }
         if err.is_none() {
-            if let Err(e) = drain_buffer_into_file_and_flush(&mut buffer, dst) {
+            if let Err(e) = flush_to_file(&mut buffer, dst, 0) {
                 err = Some(e);
             }
         }
@@ -696,13 +689,11 @@ fn save_internal_via_appending(&mut self, history_path: &wstr) -> std::io::Resul
             let item = &self.new_items[self.first_unwritten_new_item_index];
             if item.should_write_to_disk() {
                 append_history_item_to_buffer(item, &mut buffer);
-                // Ensure that each item is written individually and makes it to storage.
-                // This is done to keep `self.first_unwritten_new_item_index` consistent with
-                // the file system.
-                // Flushing and syncing each iteration adds overhead,
-                // but hopefully there are not that many items to write when appending.
-                res = drain_buffer_into_file_and_flush(&mut buffer, locked_history_file.get_mut());
-
+                res = flush_to_file(
+                    &mut buffer,
+                    locked_history_file.get_mut(),
+                    HISTORY_OUTPUT_BUFFER_SIZE,
+                );
                 if res.is_err() {
                     break;
                 }
@@ -711,6 +702,10 @@ fn save_internal_via_appending(&mut self, history_path: &wstr) -> std::io::Resul
             self.first_unwritten_new_item_index += 1;
         }
 
+        if res.is_ok() {
+            res = flush_to_file(&mut buffer, locked_history_file.get_mut(), 0);
+        }
+
         // Since we just modified the file, update our history_file_id to match its current state
         // Otherwise we'll think the file has been changed by someone else the next time we go to
         // write.