From 2368177ab0c8f6f3c82ef4e2279721422dfe4282 Mon Sep 17 00:00:00 2001 From: Benjamin Winger Date: Fri, 19 Apr 2024 12:35:04 -0400 Subject: [PATCH] Fix updating the diskArray nextPipPageIdx when multiple new PIPs are added --- .../storage/storage_structure/disk_array.h | 2 +- src/storage/storage_structure/disk_array.cpp | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/include/storage/storage_structure/disk_array.h b/src/include/storage/storage_structure/disk_array.h index e87158a41b..0cc13b3f2d 100644 --- a/src/include/storage/storage_structure/disk_array.h +++ b/src/include/storage/storage_structure/disk_array.h @@ -48,7 +48,7 @@ struct DiskArrayHeader { }; struct PIP { - PIP() : nextPipPageIdx{DBFileUtils::NULL_PAGE_IDX} {} + PIP() : nextPipPageIdx{DBFileUtils::NULL_PAGE_IDX}, pageIdxs{} {} common::page_idx_t nextPipPageIdx; common::page_idx_t pageIdxs[NUM_PAGE_IDXS_PER_PIP]; diff --git a/src/storage/storage_structure/disk_array.cpp b/src/storage/storage_structure/disk_array.cpp index 0657466bf0..6b06d7a82c 100644 --- a/src/storage/storage_structure/disk_array.cpp +++ b/src/storage/storage_structure/disk_array.cpp @@ -188,20 +188,25 @@ void BaseDiskArrayInternal::setNextPIPPageIDxOfPIPNoLock(DiskArrayHeader* update if (!pipUpdates.updatedLastPIP.has_value()) { pipUpdates.updatedLastPIP = std::make_optional(pips[pipIdxOfPreviousPIP]); } - pipUpdates.updatedLastPIP->pipContents.nextPipPageIdx = nextPIPPageIdx; + if (pipIdxOfPreviousPIP == pips.size() - 1) { + pipUpdates.updatedLastPIP->pipContents.nextPipPageIdx = nextPIPPageIdx; + } else { + KU_ASSERT(pipIdxOfPreviousPIP >= pips.size() && + pipUpdates.newPIPs.size() > pipIdxOfPreviousPIP - pips.size()); + pipUpdates.newPIPs[pipIdxOfPreviousPIP - pips.size()].pipContents.nextPipPageIdx = + nextPIPPageIdx; + } } } page_idx_t BaseDiskArrayInternal::getAPPageIdxNoLock(page_idx_t apIdx, TransactionType trxType) { - auto pipIdxAndOffset = StorageUtils::getQuotientRemainder(apIdx, NUM_PAGE_IDXS_PER_PIP); - uint64_t pipIdx = pipIdxAndOffset.first; - uint64_t offsetInPIP = pipIdxAndOffset.second; + auto [pipIdx, offsetInPIP] = StorageUtils::getQuotientRemainder(apIdx, NUM_PAGE_IDXS_PER_PIP); if ((trxType == TransactionType::READ_ONLY) || !hasPIPUpdatesNoLock(pipIdx)) { return pips[pipIdx].pipContents.pageIdxs[offsetInPIP]; } else if (pipIdx == pips.size() - 1 && pipUpdates.updatedLastPIP) { return pipUpdates.updatedLastPIP->pipContents.pageIdxs[offsetInPIP]; } else { - KU_ASSERT(pipIdx >= pips.size()); + KU_ASSERT(pipIdx >= pips.size() && pipIdx - pips.size() < pipUpdates.newPIPs.size()); return pipUpdates.newPIPs[pipIdx - pips.size()].pipContents.pageIdxs[offsetInPIP]; } }