From c3b664fea54cfd8ac25411cbdb9536e4f24b008e Mon Sep 17 00:00:00 2001
From: Zygo Blaxell <bees@furryterror.org>
Date: Thu, 22 Dec 2022 00:07:49 -0500
Subject: [PATCH] context: don't forget to retry locked extents

The caller of scan_forward has to stop advancing the BeesFileCrawl
position when an extent lock blocks a scan, so that it will resume
from the same position when the Task is scheduled again; otherwise,
bees simply skips over the extent and leave it incompletely deduped.

Signed-off-by: Zygo Blaxell <bees@furryterror.org>
---
 src/bees-context.cc | 11 +++++------
 src/bees-roots.cc   | 11 +++++++++--
 src/bees.h          |  2 +-
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/bees-context.cc b/src/bees-context.cc
index 796bfc6..ae2075a 100644
--- a/src/bees-context.cc
+++ b/src/bees-context.cc
@@ -678,7 +678,7 @@ BeesContext::get_inode_mutex(const uint64_t inode)
 	return m_inode_locks(inode);
 }
 
-void
+bool
 BeesContext::scan_forward(const BeesFileRange &bfr_in)
 {
 	BEESTRACE("scan_forward " << bfr_in);
@@ -689,7 +689,7 @@ BeesContext::scan_forward(const BeesFileRange &bfr_in)
 	// Silently filter out blacklisted files
 	if (is_blacklisted(bfr_in.fid())) {
 		BEESCOUNT(scan_blacklisted);
-		return;
+		return false;
 	}
 
 	// Reconstitute FD
@@ -703,14 +703,14 @@ BeesContext::scan_forward(const BeesFileRange &bfr_in)
 	if (!bfr.fd()) {
 		// BEESLOGINFO("No FD in " << root_path() << " for " << bfr);
 		BEESCOUNT(scan_no_fd);
-		return;
+		return false;
 	}
 
 	// Sanity check
 	if (bfr.begin() >= bfr.file_size()) {
 		BEESLOGWARN("past EOF: " << bfr);
 		BEESCOUNT(scan_eof);
-		return;
+		return false;
 	}
 
 	BtrfsExtentWalker ew(bfr.fd(), bfr.begin(), root_fd());
@@ -729,7 +729,6 @@ BeesContext::scan_forward(const BeesFileRange &bfr_in)
 					// BEESLOGDEBUG("Deferring extent bytenr " << to_hex(extent_bytenr) << " from " << bfr);
 					BEESCOUNT(scanf_deferred_extent);
 					start_over = true;
-					return;
 				}
 				Timer one_extent_timer;
 				scan_one_extent(bfr, e);
@@ -750,7 +749,7 @@ BeesContext::scan_forward(const BeesFileRange &bfr_in)
 	BEESCOUNTADD(scanf_total_ms, scan_timer.age() * 1000);
 	BEESCOUNT(scanf_total);
 
-	return;
+	return start_over;
 }
 
 BeesResolveAddrResult::BeesResolveAddrResult()
diff --git a/src/bees-roots.cc b/src/bees-roots.cc
index 9ed8732..a048968 100644
--- a/src/bees-roots.cc
+++ b/src/bees-roots.cc
@@ -626,13 +626,20 @@ BeesFileCrawl::crawl_one_extent()
 					// It might be corrupted data, the file might have been deleted or truncated,
 					// or we might hit some other recoverable error.  We'll try again with
 					// the next extent.
+					bool scanned_ok = false;
 					catch_all([&]() {
 						BEESNOTE("scan_forward " << bfr);
 						// BEESLOGDEBUG("scan_forward #" << Task::current_task().id() << " " << bfr);
-						m_ctx->scan_forward(bfr);
+						scanned_ok = m_ctx->scan_forward(bfr);
 						// BEESLOGDEBUG("done_forward #" << Task::current_task().id() << " " << bfr);
 					} );
-					m_hold = new_holder;
+					if (scanned_ok) {
+						m_hold = new_holder;
+					} else {
+						BEESLOGDEBUG("retrying lock for extent at " << bfr);
+						BEESCOUNT(crawl_restart);
+						return true;
+					}
 				}
 			} else {
 				BEESCOUNT(crawl_hole);
diff --git a/src/bees.h b/src/bees.h
index 55b2925..41d8814 100644
--- a/src/bees.h
+++ b/src/bees.h
@@ -751,7 +751,7 @@ public:
 	Fd home_fd();
 	string root_path() const { return m_root_path; }
 
-	void scan_forward(const BeesFileRange &bfr);
+	bool scan_forward(const BeesFileRange &bfr);
 
 	bool is_root_ro(uint64_t root);
 	BeesRangePair dup_extent(const BeesFileRange &src, const shared_ptr<BeesTempFile> &tmpfile);