From cc87125e41e5864ddbfae1c4ef7c46f19626b7c0 Mon Sep 17 00:00:00 2001
From: Zygo Blaxell <bees@furryterror.org>
Date: Sat, 26 Nov 2022 22:45:24 -0500
Subject: [PATCH] bees: drop bees_sync, we will not need it

bees_sync() was an exception-trapping wrapper around fsync() which is
not needed in any of the contexts from which it was called:

	1.  dedupe operations implicitly flush the src data, so there is
	no need to call fsync() to do that twice.

	2.  crawl position is written to a temporary file and renamed
	over the original, which always forces a flush when the original
	exists.  On the first write, where there is no original, a
	crash would result in starting over with an empty or hole-filled
	beescrawl file, which is the initial state of bees.  There is also
	a long history of kernel bugs triggered by fsync() in this case.

	3.  we use unreadahead to trigger writeback for flushing the
	hash table to persistent storage.  Here is a space where we might
	use fsync after all, as part of bees_unreadahead's emulation of
	POSIX_FADV_DONTNEED, but we need to get read-once behavior from
	the scanner before we can use this capability.

Signed-off-by: Zygo Blaxell <bees@furryterror.org>
---
 src/bees.cc | 23 -----------------------
 src/bees.h  |  1 -
 2 files changed, 24 deletions(-)

diff --git a/src/bees.cc b/src/bees.cc
index 645c744..ba3fc5b 100644
--- a/src/bees.cc
+++ b/src/bees.cc
@@ -214,17 +214,6 @@ BeesTooLong::operator=(const func_type &f)
 	return *this;
 }
 
-void
-bees_sync(int fd)
-{
-	Timer sync_timer;
-	BEESNOTE("syncing " << name_fd(fd));
-	BEESTOOLONG("syncing " << name_fd(fd));
-	DIE_IF_NON_ZERO(fsync(fd));
-	BEESCOUNT(sync_count);
-	BEESCOUNTADD(sync_ms, sync_timer.age() * 1000);
-}
-
 void
 bees_readahead(int const fd, off_t offset, size_t size)
 {
@@ -481,7 +470,6 @@ BeesTempFile::make_copy(const BeesFileRange &src)
 	auto src_p = src.begin();
 	auto dst_p = begin;
 
-	bool did_block_write = false;
 	while (dst_p < end) {
 		auto len = min(BLOCK_SIZE_CLONE, end - dst_p);
 		BeesBlockData bbd(src.fd(), src_p, len);
@@ -492,7 +480,6 @@ BeesTempFile::make_copy(const BeesFileRange &src)
 			BEESNOTE("copying " << src << " to " << rv << "\n"
 				"\tpwrite " << bbd << " to " << name_fd(m_fd) << " offset " << to_hex(dst_p) << " len " << len);
 			pwrite_or_die(m_fd, bbd.data().data(), len, dst_p);
-			did_block_write = true;
 			BEESCOUNT(tmp_block);
 			BEESCOUNTADD(tmp_bytes, len);
 		}
@@ -501,16 +488,6 @@ BeesTempFile::make_copy(const BeesFileRange &src)
 	}
 	BEESCOUNTADD(tmp_copy_ms, copy_timer.age() * 1000);
 
-	if (did_block_write) {
-#if 0
-		// There were a lot of kernel bugs leading to lockups.
-		// Most of them are fixed now.
-		// Unnecessary sync makes us slow, but maybe it has some robustness utility.
-		// TODO:  make this configurable.
-		bees_sync(m_fd);
-#endif
-	}
-
 	BEESCOUNT(tmp_copy);
 	return rv;
 }
diff --git a/src/bees.h b/src/bees.h
index 00762a6..c5aa654 100644
--- a/src/bees.h
+++ b/src/bees.h
@@ -882,7 +882,6 @@ extern const char *BEES_USAGE;
 extern const char *BEES_VERSION;
 extern thread_local default_random_engine bees_generator;
 string pretty(double d);
-void bees_sync(int fd);
 void bees_readahead(int fd, off_t offset, size_t size);
 void bees_unreadahead(int fd, off_t offset, size_t size);
 string format_time(time_t t);