From cd45df502bf91e550692d9930c6aaffc137eaadf Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 9 Apr 2014 22:15:19 +0000 Subject: Directories' entries in parent directory should track current size. When entries are added to a directory by a command (BackupStoreContext), and when entries are removed from a directory (by housekeeping), update the parent directory's entry for us if our size has changed. Make BackupStoreCheck check for, report and fix errors when directory entries are directories and the size is wrong (as well as files). Conflicts: test/backupstore/testbackupstore.cpp Fix directories loaded without size being set, leading to warnings later. We can't check that the old size in the parent entry matched the old real size of the directory, unless we set the old real size in the directory. And we don't need to pass the old directory size to HousekeepStoreAccount::DeleteFile, because we can get it from the directory itself. --- lib/backupstore/BackupStoreCheck.cpp | 93 +++++++++++++-------------- lib/backupstore/BackupStoreContext.cpp | 28 ++++++++- lib/backupstore/HousekeepStoreAccount.cpp | 101 +++++++++++++++++++++++++----- lib/backupstore/HousekeepStoreAccount.h | 4 +- 4 files changed, 156 insertions(+), 70 deletions(-) (limited to 'lib/backupstore') diff --git a/lib/backupstore/BackupStoreCheck.cpp b/lib/backupstore/BackupStoreCheck.cpp index 70fe4a1e..05a901fa 100644 --- a/lib/backupstore/BackupStoreCheck.cpp +++ b/lib/backupstore/BackupStoreCheck.cpp @@ -871,7 +871,6 @@ bool BackupStoreCheck::CheckDirectoryEntry(BackupStoreDirectory::Entry& rEntry, ASSERT(piBlock != 0); uint8_t iflags = GetFlags(piBlock, IndexInDirBlock); - bool badEntry = false; // Is the type the same? if(((iflags & Flags_IsDir) == Flags_IsDir) != rEntry.IsDir()) @@ -882,73 +881,67 @@ bool BackupStoreCheck::CheckDirectoryEntry(BackupStoreDirectory::Entry& rEntry, " references object " << BOX_FORMAT_OBJECTID(rEntry.GetObjectID()) << " which has a different type than expected."); - badEntry = true; ++mNumberErrorsFound; + return false; // remove this entry } + // Check that the entry is not already contained. - else if(iflags & Flags_IsContained) + if(iflags & Flags_IsContained) { BOX_ERROR("Directory ID " << BOX_FORMAT_OBJECTID(DirectoryID) << " references object " << BOX_FORMAT_OBJECTID(rEntry.GetObjectID()) << " which is already contained."); - badEntry = true; ++mNumberErrorsFound; + return false; // remove this entry } - else + + // Not already contained by another directory. + // Don't set the flag until later, after we finish repairing + // the directory and removing all bad entries. + + // Check that the container ID of the object is correct + if(piBlock->mContainer[IndexInDirBlock] != DirectoryID) { - // Not already contained by another directory. - // Don't set the flag until later, after we finish repairing - // the directory and removing all bad entries. - - // Check that the container ID of the object is correct - if(piBlock->mContainer[IndexInDirBlock] != DirectoryID) + // Needs fixing... + if(iflags & Flags_IsDir) { - // Needs fixing... - if(iflags & Flags_IsDir) - { - // Add to will fix later list - BOX_ERROR("Directory ID " << - BOX_FORMAT_OBJECTID(rEntry.GetObjectID()) - << " has wrong container ID."); - mDirsWithWrongContainerID.push_back(rEntry.GetObjectID()); - ++mNumberErrorsFound; - } - else - { - // This is OK for files, they might move - BOX_INFO("File ID " << - BOX_FORMAT_OBJECTID(rEntry.GetObjectID()) - << " has different container ID, " - "probably moved"); - } - - // Fix entry for now - piBlock->mContainer[IndexInDirBlock] = DirectoryID; + // Add to will fix later list + BOX_ERROR("Directory ID " << + BOX_FORMAT_OBJECTID(rEntry.GetObjectID()) + << " has wrong container ID."); + mDirsWithWrongContainerID.push_back(rEntry.GetObjectID()); + ++mNumberErrorsFound; + } + else + { + // This is OK for files, they might move + BOX_INFO("File ID " << + BOX_FORMAT_OBJECTID(rEntry.GetObjectID()) + << " has different container ID, " + "probably moved"); } + + // Fix entry for now + piBlock->mContainer[IndexInDirBlock] = DirectoryID; } - - // Check the object size, if it's OK and a file - if(!badEntry && !rEntry.IsDir()) + + // Check the object size + if(rEntry.GetSizeInBlocks() != piBlock->mObjectSizeInBlocks[IndexInDirBlock]) { - if(rEntry.GetSizeInBlocks() != piBlock->mObjectSizeInBlocks[IndexInDirBlock]) - { - // Wrong size, correct it. - rEntry.SetSizeInBlocks(piBlock->mObjectSizeInBlocks[IndexInDirBlock]); + // Wrong size, correct it. + BOX_ERROR("Directory " << BOX_FORMAT_OBJECTID(DirectoryID) << + " entry for " << BOX_FORMAT_OBJECTID(rEntry.GetObjectID()) << + " has wrong size " << rEntry.GetSizeInBlocks() << + ", should be " << piBlock->mObjectSizeInBlocks[IndexInDirBlock]); - // Mark as changed - rIsModified = true; - ++mNumberErrorsFound; + rEntry.SetSizeInBlocks(piBlock->mObjectSizeInBlocks[IndexInDirBlock]); - // Tell user - BOX_ERROR("Directory ID " << - BOX_FORMAT_OBJECTID(DirectoryID) << - " has wrong size for object " << - BOX_FORMAT_OBJECTID(rEntry.GetObjectID())); - } + // Mark as changed + rIsModified = true; + ++mNumberErrorsFound; } - return !badEntry; + return true; // don't delete this entry } - diff --git a/lib/backupstore/BackupStoreContext.cpp b/lib/backupstore/BackupStoreContext.cpp index 33c2a660..974c4e3e 100644 --- a/lib/backupstore/BackupStoreContext.cpp +++ b/lib/backupstore/BackupStoreContext.cpp @@ -972,6 +972,8 @@ void BackupStoreContext::SaveDirectory(BackupStoreDirectory &rDir) // Write to disc, adjust size in store info std::string dirfn; MakeObjectFilename(ObjectID, dirfn); + int64_t old_dir_size = rDir.GetUserInfo1_SizeInBlocks(); + { RaidFileWrite writeDir(mStoreDiscSet, dirfn); writeDir.Open(true /* allow overwriting */); @@ -994,6 +996,7 @@ void BackupStoreContext::SaveDirectory(BackupStoreDirectory &rDir) // Update size stored in directory rDir.SetUserInfo1_SizeInBlocks(dirSize); } + // Refresh revision ID in cache { int64_t revid = 0; @@ -1003,6 +1006,22 @@ void BackupStoreContext::SaveDirectory(BackupStoreDirectory &rDir) } rDir.SetRevisionID(revid); } + + // Update the directory entry in the grandparent, to ensure + // that it reflects the current size of the parent directory. + int64_t new_dir_size = rDir.GetUserInfo1_SizeInBlocks(); + if(new_dir_size != old_dir_size && + rDir.GetObjectID() != BACKUPSTORE_ROOT_DIRECTORY_ID) + { + BackupStoreDirectory& parent( + GetDirectoryInternal(rDir.GetContainerID())); + BackupStoreDirectory::Entry* en = + parent.FindEntryByID(rDir.GetObjectID()); + ASSERT(en); + ASSERT(en->GetSizeInBlocks() == old_dir_size); + en->SetSizeInBlocks(new_dir_size); + SaveDirectory(parent); + } } catch(...) { @@ -1608,7 +1627,9 @@ void BackupStoreContext::SetClientStoreMarker(int64_t ClientStoreMarker) // Created: 12/11/03 // // -------------------------------------------------------------------------- -void BackupStoreContext::MoveObject(int64_t ObjectID, int64_t MoveFromDirectory, int64_t MoveToDirectory, const BackupStoreFilename &rNewFilename, bool MoveAllWithSameName, bool AllowMoveOverDeletedObject) +void BackupStoreContext::MoveObject(int64_t ObjectID, int64_t MoveFromDirectory, + int64_t MoveToDirectory, const BackupStoreFilename &rNewFilename, + bool MoveAllWithSameName, bool AllowMoveOverDeletedObject) { if(mReadOnly) { @@ -1683,8 +1704,9 @@ void BackupStoreContext::MoveObject(int64_t ObjectID, int64_t MoveFromDirectory, return; } - // Got to be careful how this is written, as we can't guarentte that if we have two - // directories open, the first won't be deleted as the second is opened. (cache) + // Got to be careful how this is written, as we can't guarantee that + // if we have two directories open, the first won't be deleted as the + // second is opened. (cache) // List of entries to move std::vector moving; diff --git a/lib/backupstore/HousekeepStoreAccount.cpp b/lib/backupstore/HousekeepStoreAccount.cpp index 0bcc6a44..2bcc59ae 100644 --- a/lib/backupstore/HousekeepStoreAccount.cpp +++ b/lib/backupstore/HousekeepStoreAccount.cpp @@ -368,6 +368,7 @@ bool HousekeepStoreAccount::ScanDirectory(int64_t ObjectID, BackupStoreDirectory dir; BufferedStream buf(*dirStream); dir.ReadFromStream(buf, IOStream::TimeOutInfinite); + dir.SetUserInfo1_SizeInBlocks(originalDirSizeInBlocks); dirStream->Close(); // Is it empty? @@ -409,8 +410,8 @@ bool HousekeepStoreAccount::ScanDirectory(int64_t ObjectID, && (en->IsDeleted() || en->IsOld())) { // Delete this immediately. - DeleteFile(ObjectID, en->GetObjectID(), dir, objectFilename, - originalDirSizeInBlocks, rBackupStoreInfo); + DeleteFile(ObjectID, en->GetObjectID(), dir, + objectFilename, rBackupStoreInfo); // flag as having done something deletedSomething = true; @@ -643,19 +644,17 @@ bool HousekeepStoreAccount::DeleteFiles(BackupStoreInfo& rBackupStoreInfo) // Get the filename std::string dirFilename; BackupStoreDirectory dir; - int64_t dirSizeInBlocksOrig = 0; { MakeObjectFilename(i->mInDirectory, dirFilename); std::auto_ptr dirStream(RaidFileRead::Open(mStoreDiscSet, dirFilename)); - dirSizeInBlocksOrig = dirStream->GetDiscUsageInBlocks(); dir.ReadFromStream(*dirStream, IOStream::TimeOutInfinite); + dir.SetUserInfo1_SizeInBlocks(dirStream->GetDiscUsageInBlocks()); } // Delete the file BackupStoreRefCountDatabase::refcount_t refs = DeleteFile(i->mInDirectory, i->mObjectID, dir, - dirFilename, dirSizeInBlocksOrig, - rBackupStoreInfo); + dirFilename, rBackupStoreInfo); if(refs == 0) { BOX_INFO("Housekeeping removed " << @@ -702,7 +701,7 @@ bool HousekeepStoreAccount::DeleteFiles(BackupStoreInfo& rBackupStoreInfo) BackupStoreRefCountDatabase::refcount_t HousekeepStoreAccount::DeleteFile( int64_t InDirectory, int64_t ObjectID, BackupStoreDirectory &rDirectory, - const std::string &rDirectoryFilename, int64_t OriginalDirSizeInBlocks, + const std::string &rDirectoryFilename, BackupStoreInfo& rBackupStoreInfo) { // Find the entry inside the directory @@ -727,6 +726,7 @@ BackupStoreRefCountDatabase::refcount_t HousekeepStoreAccount::DeleteFile( BOX_FORMAT_OBJECTID(InDirectory) << ", " "indicates logic error/corruption? Run " "bbstoreaccounts check fix"); + mErrorCount++; return refs; } @@ -859,7 +859,6 @@ BackupStoreRefCountDatabase::refcount_t HousekeepStoreAccount::DeleteFile( // Save directory back to disc // BLOCK - int64_t dirRevisedSize = 0; { RaidFileWrite writeDir(mStoreDiscSet, rDirectoryFilename, mapNewRefs->GetRefCount(InDirectory)); @@ -867,18 +866,18 @@ BackupStoreRefCountDatabase::refcount_t HousekeepStoreAccount::DeleteFile( rDirectory.WriteToStream(writeDir); // Get the disc usage (must do this before commiting it) - dirRevisedSize = writeDir.GetDiscUsageInBlocks(); + int64_t new_size = writeDir.GetDiscUsageInBlocks(); // Commit directory writeDir.Commit(BACKUP_STORE_CONVERT_TO_RAID_IMMEDIATELY); // Adjust block counts if the directory itself changed in size - if(dirRevisedSize > 0) - { - int64_t adjust = dirRevisedSize - OriginalDirSizeInBlocks; - mBlocksUsedDelta += adjust; - mBlocksInDirectoriesDelta += adjust; - } + int64_t original_size = rDirectory.GetUserInfo1_SizeInBlocks(); + int64_t adjust = new_size - original_size; + mBlocksUsedDelta += adjust; + mBlocksInDirectoriesDelta += adjust; + + UpdateDirectorySize(rDirectory, new_size); } // Commit any new adjusted entry @@ -926,6 +925,75 @@ BackupStoreRefCountDatabase::refcount_t HousekeepStoreAccount::DeleteFile( return 0; } +// -------------------------------------------------------------------------- +// +// Function +// Name: HousekeepStoreAccount::UpdateDirectorySize( +// BackupStoreDirectory& rDirectory, +// IOStream::pos_type new_size_in_blocks) +// Purpose: Update the directory size, modifying the parent +// directory's entry for this directory if necessary. +// Created: 05/03/14 +// +// -------------------------------------------------------------------------- + +void HousekeepStoreAccount::UpdateDirectorySize( + BackupStoreDirectory& rDirectory, + IOStream::pos_type new_size_in_blocks) +{ +#ifndef NDEBUG + { + std::string dirFilename; + MakeObjectFilename(rDirectory.GetObjectID(), dirFilename); + std::auto_ptr dirStream( + RaidFileRead::Open(mStoreDiscSet, dirFilename)); + ASSERT(new_size_in_blocks == dirStream->GetDiscUsageInBlocks()); + } +#endif + + IOStream::pos_type old_size_in_blocks = + rDirectory.GetUserInfo1_SizeInBlocks(); + + if(new_size_in_blocks == old_size_in_blocks) + { + return; + } + + rDirectory.SetUserInfo1_SizeInBlocks(new_size_in_blocks); + + if (rDirectory.GetObjectID() == BACKUPSTORE_ROOT_DIRECTORY_ID) + { + return; + } + + std::string parentFilename; + MakeObjectFilename(rDirectory.GetContainerID(), parentFilename); + std::auto_ptr parentStream( + RaidFileRead::Open(mStoreDiscSet, parentFilename)); + BackupStoreDirectory parent(*parentStream); + BackupStoreDirectory::Entry* en = + parent.FindEntryByID(rDirectory.GetObjectID()); + ASSERT(en); + + if (en->GetSizeInBlocks() != old_size_in_blocks) + { + BOX_WARNING("Directory " << + BOX_FORMAT_OBJECTID(rDirectory.GetObjectID()) << + " entry in directory " << + BOX_FORMAT_OBJECTID(rDirectory.GetContainerID()) << + " had incorrect size " << en->GetSizeInBlocks() << + ", should have been " << old_size_in_blocks); + mErrorCount++; + } + + en->SetSizeInBlocks(new_size_in_blocks); + + RaidFileWrite writeDir(mStoreDiscSet, parentFilename, + mapNewRefs->GetRefCount(rDirectory.GetContainerID())); + writeDir.Open(true /* allow overwriting */); + parent.WriteToStream(writeDir); + writeDir.Commit(BACKUP_STORE_CONVERT_TO_RAID_IMMEDIATELY); +} // -------------------------------------------------------------------------- // @@ -1022,11 +1090,13 @@ void HousekeepStoreAccount::DeleteEmptyDirectory(int64_t dirId, containingDirStream->GetDiscUsageInBlocks(); containingDir.ReadFromStream(*containingDirStream, IOStream::TimeOutInfinite); + containingDir.SetUserInfo1_SizeInBlocks(containingDirSizeInBlocksOrig); } // Find the entry BackupStoreDirectory::Entry *pdirentry = containingDir.FindEntryByID(dir.GetObjectID()); + // TODO FIXME invert test and reduce indentation if((pdirentry != 0) && pdirentry->IsDeleted()) { // Should be deleted @@ -1049,6 +1119,7 @@ void HousekeepStoreAccount::DeleteEmptyDirectory(int64_t dirId, // Commit directory writeDir.Commit(BACKUP_STORE_CONVERT_TO_RAID_IMMEDIATELY); + UpdateDirectorySize(containingDir, dirSize); // adjust usage counts for this directory if(dirSize > 0) diff --git a/lib/backupstore/HousekeepStoreAccount.h b/lib/backupstore/HousekeepStoreAccount.h index ff57d1a1..ff9e9ffe 100644 --- a/lib/backupstore/HousekeepStoreAccount.h +++ b/lib/backupstore/HousekeepStoreAccount.h @@ -56,10 +56,10 @@ private: int64_t ObjectID, BackupStoreDirectory &rDirectory, const std::string &rDirectoryFilename, - int64_t OriginalDirSizeInBlocks, BackupStoreInfo& rBackupStoreInfo); + void UpdateDirectorySize(BackupStoreDirectory &rDirectory, + IOStream::pos_type new_size_in_blocks); -private: typedef struct { int64_t mObjectID; -- cgit v1.2.3