[ox/fs] Fix FileStoreTemplate::compact to skip first item and correctly find parent

This commit is contained in:
Gary Talent 2019-07-19 20:14:09 -05:00
parent 31b75d1e50
commit 8fa5488d77

View File

@ -164,7 +164,7 @@ class FileStoreTemplate {
/** /**
* Finds the parent an inode by its ID. * Finds the parent an inode by its ID.
*/ */
ItemPtr findParent(ItemPtr ptr, size_t id) const; ItemPtr findParent(ItemPtr ptr, size_t id, size_t oldAddr) const;
/** /**
* Finds an inode by its ID. * Finds an inode by its ID.
@ -492,25 +492,32 @@ ValErr<typename FileStoreTemplate<size_t>::InodeId_t> FileStoreTemplate<size_t>:
template<typename size_t> template<typename size_t>
void FileStoreTemplate<size_t>::compact() { void FileStoreTemplate<size_t>::compact() {
m_buffer->compact([this](uint64_t oldAddr, ItemPtr item) { auto isFirstItem = true;
if (item.valid()) { m_buffer->compact([this, &isFirstItem](uint64_t oldAddr, ItemPtr item) {
oxTrace("ox::FileStoreTemplate::compact::moveItem") if (isFirstItem) {
<< "Moving Item:" << item->id isFirstItem = false;
<< "from" << oldAddr return;
<< "to" << item.offset(); }
// update rootInode if this is it if (!item.valid()) {
auto fsData = fileStoreData(); return;
if (fsData && oldAddr == fsData->rootNode) { }
fsData->rootNode = item.offset(); oxTrace("ox::FileStoreTemplate::compact::moveItem")
} << "Moving Item:" << item->id
auto parent = findParent(rootInode(), item); << "from" << oldAddr
oxAssert(parent.valid(), "Parent inode not found."); << "to" << item.offset();
if (parent.valid()) { // update rootInode if this is it
if (parent->left == oldAddr) { auto fsData = fileStoreData();
parent->left = item; if (fsData && oldAddr == fsData->rootNode) {
} else if (parent->right == oldAddr) { fsData->rootNode = item.offset();
parent->right = item; }
} auto parent = findParent(rootInode(), item->id, oldAddr);
oxAssert(parent.valid() || rootInode() == item.offset(),
"Parent inode not found for item that should have parent.");
if (parent.valid()) {
if (parent->left == oldAddr) {
parent->left = item;
} else if (parent->right == oldAddr) {
parent->right = item;
} }
} }
}); });
@ -668,25 +675,25 @@ Error FileStoreTemplate<size_t>::remove(ItemPtr item) {
} }
template<typename size_t> template<typename size_t>
typename FileStoreTemplate<size_t>::ItemPtr FileStoreTemplate<size_t>::findParent(ItemPtr item, size_t id) const { typename FileStoreTemplate<size_t>::ItemPtr FileStoreTemplate<size_t>::findParent(ItemPtr item, size_t id, size_t oldAddr) const {
// This is a little bit confusing. findParent uses the inode ID to find
// where the target ID should be, but the actual address of that item is
// currently invalid, so we check it against what is known to be the old
// address of the item to confirm that we have the right item.
if (item.valid()) { if (item.valid()) {
if (id > item->id) { if (id > item->id) {
auto right = m_buffer->ptr(item->right); if (item->right == oldAddr) {
if (right.valid()) { return item;
if (right->id == id) { } else {
return item; auto right = m_buffer->ptr(item->right);
} else { return findParent(right, id, oldAddr);
return findParent(right, id);
}
} }
} else if (id < item->id) { } else if (id < item->id) {
auto left = m_buffer->ptr(item->left); if (item->left == oldAddr) {
if (left.valid()) { return item;
if (left->id == id) { } else {
return item; auto left = m_buffer->ptr(item->left);
} else { return findParent(left, id, oldAddr);
return findParent(left, id);
}
} }
} }
} }