Repair the refcount database while checking accounts for errors.

Normally this will be done by housekeeping anyway, so it's not that useful,
but it's good to check for it in tests, especially when testing
BackupStoreCheck's ability to repair bad things that happened to the store
when they involve references.

6 files changed, 161 insertions, 87 deletions

diff --git a/lib/backupstore/BackupStoreCheck.cpp b/lib/backupstore/BackupStoreCheck.cpp
index 1b314eec..33660ffc 100644
--- a/lib/backupstore/BackupStoreCheck.cpp
+++ b/lib/backupstore/BackupStoreCheck.cpp
@@ -17,11 +17,13 @@
 #endif
 
 #include "autogen_BackupStoreException.h"
+#include "BackupStoreAccountDatabase.h"
 #include "BackupStoreCheck.h"
 #include "BackupStoreConstants.h"
 #include "BackupStoreDirectory.h"
 #include "BackupStoreFile.h"
 #include "BackupStoreObjectMagic.h"
+#include "BackupStoreRefCountDatabase.h"
 #include "RaidFileController.h"
 #include "RaidFileException.h"
 #include "RaidFileRead.h"
@@ -78,6 +80,13 @@ BackupStoreCheck::~BackupStoreCheck()
 {
 	// Clean up
 	FreeInfo();
+
+	// Avoid an exception if we forget to discard mapNewRefs
+	if (mapNewRefs.get())
+	{
+		mapNewRefs->Discard();
+		mapNewRefs.reset();
+	}
 }
 
 
@@ -104,6 +113,9 @@ void BackupStoreCheck::Check()
 		BOX_INFO("Will fix errors encountered during checking.");
 	}
 
+	BackupStoreAccountDatabase::Entry account(mAccountID, mDiscSetNumber);
+	mapNewRefs = BackupStoreRefCountDatabase::Create(account);
+
 	// Phase 1, check objects
 	if(!mQuiet)
 	{
@@ -148,9 +160,31 @@ void BackupStoreCheck::Check()
 		BOX_INFO("Phase 6, regenerate store info...");
 	}
 	WriteNewStoreInfo();
-	
-//	DUMP_OBJECT_INFO
-	
+
+	try
+	{
+		std::auto_ptr<BackupStoreRefCountDatabase> apOldRefs =
+			BackupStoreRefCountDatabase::Load(account, false);
+		mNumberErrorsFound += mapNewRefs->ReportChangesTo(*apOldRefs);
+	}
+	catch(BoxException &e)
+	{
+		BOX_WARNING("Reference count database was missing or "
+			"corrupted, cannot check it for errors.");
+		mNumberErrorsFound++;
+	}
+
+	// force file to be saved and closed before releasing the lock below
+	if(mFixErrors)
+	{
+		mapNewRefs->Commit();
+	}
+	else
+	{
+		mapNewRefs->Discard();
+	}
+	mapNewRefs.reset();
+
 	if(mNumberErrorsFound > 0)
 	{
 		BOX_WARNING("Finished checking store account ID " <<
@@ -408,7 +442,8 @@ void BackupStoreCheck::CheckObjectsDir(int64_t StartID)
 			fileOK = false;
 		}
 		// info and refcount databases are OK in the root directory
-		else if(*i == "info" || *i == "refcount.db" || *i == "refcount.rdb")
+		else if(*i == "info" || *i == "refcount.db" ||
+			*i == "refcount.rdb" || *i == "refcount.rdbX")
 		{
 			fileOK = true;
 		}
@@ -717,61 +752,7 @@ void BackupStoreCheck::CheckDirectories()
 					fixed.Commit(true /* convert to raid representation now */);
 				}
 
-				// Count valid entries
-				BackupStoreDirectory::Iterator i(dir);
-				BackupStoreDirectory::Entry *en = 0;
-				while((en = i.Next()) != 0)
-				{
-					int32_t iIndex;
-					IDBlock *piBlock = LookupID(en->GetObjectID(), iIndex);
-
-					ASSERT(piBlock != 0 || 
-						mDirsWhichContainLostDirs.find(en->GetObjectID())
-						!= mDirsWhichContainLostDirs.end());
-					if (piBlock)
-					{
-						// Normally it would exist and this
-						// check would not be necessary, but
-						// we might have missing directories
-						// that we will recreate later.
-						// cf mDirsWhichContainLostDirs.
-						uint8_t iflags = GetFlags(piBlock, iIndex);
-						SetFlags(piBlock, iIndex, iflags | Flags_IsContained);
-					}
-
-					if(en->IsDir())
-					{
-						mNumDirectories++;
-					}
-					else if(!en->IsFile())
-					{
-						BOX_TRACE("Not counting object " << 
-							BOX_FORMAT_OBJECTID(en->GetObjectID()) <<
-							" with flags " << en->GetFlags());
-					}
-					else // it's a good file, add to sizes
-					{
-						// It can be both old and deleted.
-						// If neither, then it's current.
-						if(en->IsDeleted())
-						{
-							mNumDeletedFiles++;
-							mBlocksInDeletedFiles += en->GetSizeInBlocks();
-						}
-
-						if(en->IsOld())
-						{
-							mNumOldFiles++;
-							mBlocksInOldFiles += en->GetSizeInBlocks();
-						}
-
-						if(!en->IsDeleted() && !en->IsOld())
-						{
-							mNumCurrentFiles++;
-							mBlocksInCurrentFiles += en->GetSizeInBlocks();
-						}
-					}
-				}
+				CountDirectoryEntries(dir);
 			}
 		}
 	}
@@ -798,8 +779,8 @@ bool BackupStoreCheck::CheckDirectory(BackupStoreDirectory& dir)
 			++mNumberErrorsFound;
 			isModified = true;
 		}
-		
-		// Go through, and check that everything in that directory exists and is valid
+
+		// Go through, and check that every entry exists and is valid
 		BackupStoreDirectory::Iterator i(dir);
 		BackupStoreDirectory::Entry *en = 0;
 		while((en = i.Next()) != 0)
@@ -863,6 +844,84 @@ bool BackupStoreCheck::CheckDirectory(BackupStoreDirectory& dir)
 	return isModified;
 }
 
+// Count valid remaining entries and the number of blocks in them.
+void BackupStoreCheck::CountDirectoryEntries(BackupStoreDirectory& dir)
+{
+	BackupStoreDirectory::Iterator i(dir);
+	BackupStoreDirectory::Entry *en = 0;
+	while((en = i.Next()) != 0)
+	{
+		int32_t iIndex;
+		IDBlock *piBlock = LookupID(en->GetObjectID(), iIndex);
+		bool badEntry = false;
+		bool wasAlreadyContained = false;
+
+		ASSERT(piBlock != 0 ||
+			mDirsWhichContainLostDirs.find(en->GetObjectID())
+			!= mDirsWhichContainLostDirs.end());
+
+		if (piBlock)
+		{
+			// Normally it would exist and this
+			// check would not be necessary, but
+			// we might have missing directories
+			// that we will recreate later.
+			// cf mDirsWhichContainLostDirs.
+			uint8_t iflags = GetFlags(piBlock, iIndex);
+			wasAlreadyContained = (iflags & Flags_IsContained);
+			SetFlags(piBlock, iIndex, iflags | Flags_IsContained);
+		}
+
+		if(wasAlreadyContained)
+		{
+			// don't double-count objects that are
+			// contained by another directory as well.
+		}
+		else if(en->IsDir())
+		{
+			mNumDirectories++;
+		}
+		else if(!en->IsFile())
+		{
+			BOX_TRACE("Not counting object " <<
+				BOX_FORMAT_OBJECTID(en->GetObjectID()) <<
+				" with flags " << en->GetFlags());
+		}
+		else // it's a file
+		{
+			// Add to sizes?
+			// If piBlock was zero, then wasAlreadyContained
+			// might be uninitialized; but we only process
+			// files here, and if a file's piBlock was zero
+			// then badEntry would be set above, so we
+			// wouldn't be here.
+			ASSERT(!badEntry)
+
+			// It can be both old and deleted.
+			// If neither, then it's current.
+			if(en->IsDeleted())
+			{
+				mNumDeletedFiles++;
+				mBlocksInDeletedFiles += en->GetSizeInBlocks();
+			}
+
+			if(en->IsOld())
+			{
+				mNumOldFiles++;
+				mBlocksInOldFiles += en->GetSizeInBlocks();
+			}
+
+			if(!en->IsDeleted() && !en->IsOld())
+			{
+				mNumCurrentFiles++;
+				mBlocksInCurrentFiles += en->GetSizeInBlocks();
+			}
+		}
+
+		mapNewRefs->AddReference(en->GetObjectID());
+	}
+}
+
 bool BackupStoreCheck::CheckDirectoryEntry(BackupStoreDirectory::Entry& rEntry,
 	int64_t DirectoryID, bool& rIsModified)
 {
diff --git a/lib/backupstore/BackupStoreCheck.h b/lib/backupstore/BackupStoreCheck.h
index 22473169..5353c968 100644
--- a/lib/backupstore/BackupStoreCheck.h
+++ b/lib/backupstore/BackupStoreCheck.h
@@ -20,6 +20,7 @@
 
 class IOStream;
 class BackupStoreFilename;
+class BackupStoreRefCountDatabase;
 
 /*
 
@@ -130,6 +131,7 @@ private:
 	bool CheckDirectory(BackupStoreDirectory& dir);
 	bool CheckDirectoryEntry(BackupStoreDirectory::Entry& rEntry,
 		int64_t DirectoryID, bool& rIsModified);
+	void CountDirectoryEntries(BackupStoreDirectory& dir);
 	int64_t CheckFile(int64_t ObjectID, IOStream &rStream);
 	int64_t CheckDirInitial(int64_t ObjectID, IOStream &rStream);
 
@@ -195,6 +197,9 @@ private:
 	
 	// Set of extra directories added
 	std::set<BackupStoreCheck_ID_t> mDirsAdded;
+
+	// The refcount database, being reconstructed as the check/fix progresses
+	std::auto_ptr<BackupStoreRefCountDatabase> mapNewRefs;
 	
 	// Misc stuff
 	int32_t mLostDirNameSerial;
diff --git a/lib/backupstore/BackupStoreCheck2.cpp b/lib/backupstore/BackupStoreCheck2.cpp
index 1743d420..e5728fa9 100644
--- a/lib/backupstore/BackupStoreCheck2.cpp
+++ b/lib/backupstore/BackupStoreCheck2.cpp
@@ -20,6 +20,7 @@
 #include "BackupStoreFileWire.h"
 #include "BackupStoreInfo.h"
 #include "BackupStoreObjectMagic.h"
+#include "BackupStoreRefCountDatabase.h"
 #include "MemBlockStream.h"
 #include "RaidFileRead.h"
 #include "RaidFileWrite.h"
@@ -259,6 +260,7 @@ void BackupStoreCheck::CheckUnattachedObjects()
 				pFixer->InsertObject(ObjectID,
 					((flags & Flags_IsDir) == Flags_IsDir),
 					lostDirNameSerial);
+				mapNewRefs->AddReference(ObjectID);
 			}
 		}
 	}
diff --git a/lib/backupstore/BackupStoreRefCountDatabase.cpp b/lib/backupstore/BackupStoreRefCountDatabase.cpp
index 3f3eb94b..e561f52b 100644
--- a/lib/backupstore/BackupStoreRefCountDatabase.cpp
+++ b/lib/backupstore/BackupStoreRefCountDatabase.cpp
@@ -336,3 +336,31 @@ bool BackupStoreRefCountDatabase::RemoveReference(int64_t ObjectID)
 	return (refcount > 0);
 }
 
+int BackupStoreRefCountDatabase::ReportChangesTo(BackupStoreRefCountDatabase& rOldRefs)
+{
+	int ErrorCount = 0;
+	int64_t MaxOldObjectId = rOldRefs.GetLastObjectIDUsed();
+	int64_t MaxNewObjectId = GetLastObjectIDUsed();
+
+	for (int64_t ObjectID = BACKUPSTORE_ROOT_DIRECTORY_ID;
+		ObjectID < std::max(MaxOldObjectId, MaxNewObjectId);
+		ObjectID++)
+	{
+		typedef BackupStoreRefCountDatabase::refcount_t refcount_t;
+		refcount_t OldRefs = (ObjectID <= MaxOldObjectId) ?
+			rOldRefs.GetRefCount(ObjectID) : 0;
+		refcount_t NewRefs = (ObjectID <= MaxNewObjectId) ?
+			this->GetRefCount(ObjectID) : 0;
+
+		if (OldRefs != NewRefs)
+		{
+			BOX_WARNING("Reference count of object " <<
+				BOX_FORMAT_OBJECTID(ObjectID) <<
+				" changed from " << OldRefs <<
+				" to " << NewRefs);
+			ErrorCount++;
+		}
+	}
+
+	return ErrorCount;
+}
diff --git a/lib/backupstore/BackupStoreRefCountDatabase.h b/lib/backupstore/BackupStoreRefCountDatabase.h
index 797f05dc..915653a4 100644
--- a/lib/backupstore/BackupStoreRefCountDatabase.h
+++ b/lib/backupstore/BackupStoreRefCountDatabase.h
@@ -87,6 +87,7 @@ public:
 	void AddReference(int64_t ObjectID);
 	// RemoveReference returns false if refcount drops to zero
 	bool RemoveReference(int64_t ObjectID);
+	int ReportChangesTo(BackupStoreRefCountDatabase& rOldRefs);
 
 private:
 	static std::string GetFilename(const BackupStoreAccountDatabase::Entry&
diff --git a/lib/backupstore/HousekeepStoreAccount.cpp b/lib/backupstore/HousekeepStoreAccount.cpp
index 8defcab1..0259461e 100644
--- a/lib/backupstore/HousekeepStoreAccount.cpp
+++ b/lib/backupstore/HousekeepStoreAccount.cpp
@@ -201,37 +201,16 @@ bool HousekeepStoreAccount::DoHousekeeping(bool KeepTryingForever)
 	mErrorCount += info->ReportChangesTo(*pOldInfo);
 	info->Save();
 
-	// We want to compare the mapNewRefs to apOldRefs before we delete any
-	// files, because that will also change the reference count in a way that's not an error.
+	// Try to load the old reference count database and check whether
+	// any counts have changed. We want to compare the mapNewRefs to
+	// apOldRefs before we delete any files, because that will also change
+	// the reference count in a way that's not an error.
 
-	// try to load the reference count database
 	try
 	{
 		std::auto_ptr<BackupStoreRefCountDatabase> apOldRefs =
 			BackupStoreRefCountDatabase::Load(account, false);
-
-		int64_t MaxOldObjectId = apOldRefs->GetLastObjectIDUsed();
-		int64_t MaxNewObjectId = mapNewRefs->GetLastObjectIDUsed();
-
-		for (int64_t ObjectID = BACKUPSTORE_ROOT_DIRECTORY_ID;
-			ObjectID < std::max(MaxOldObjectId, MaxNewObjectId);
-			ObjectID++)
-		{
-			typedef BackupStoreRefCountDatabase::refcount_t refcount_t;
-			refcount_t OldRefs = (ObjectID <= MaxOldObjectId) ?
-				apOldRefs->GetRefCount(ObjectID) : 0;
-			refcount_t NewRefs = (ObjectID <= MaxNewObjectId) ?
-				mapNewRefs->GetRefCount(ObjectID) : 0;
-
-			if (OldRefs != NewRefs)
-			{
-				BOX_WARNING("Reference count of object " <<
-					BOX_FORMAT_OBJECTID(ObjectID) <<
-					" changed from " << OldRefs <<
-					" to " << NewRefs);
-				mErrorCount++;
-			}
-		}
+		mErrorCount += mapNewRefs->ReportChangesTo(*apOldRefs);
 	}
 	catch(BoxException &e)
 	{