Fix some minor hash index issues (kuzudb#3471)

* Fix uninitialized values in hash index * Fix BaseDiskArray::getAlignedElementSize
ted-wq-x · May 10, 2024 · 7251ee6 · 7251ee6
1 parent 62b0590
commit 7251ee6
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 6 deletions.
diff --git a/src/include/storage/index/hash_index_slot.h b/src/include/storage/index/hash_index_slot.h
@@ -22,7 +22,7 @@ class SlotHeader {
     // instead restrict the capacity to 20
     static constexpr uint8_t FINGERPRINT_CAPACITY = 20;
 
-    SlotHeader() : validityMask{0}, nextOvfSlotId{0} {}
+    SlotHeader() : fingerprints{}, validityMask{0}, nextOvfSlotId{0} {}
 
     void reset() {
         validityMask = 0;
@@ -70,6 +70,7 @@ static constexpr uint8_t getSlotCapacity() {
 
 template<typename T>
 struct Slot {
+    Slot() : header{}, entries{} {}
     SlotHeader header;
     SlotEntry<T> entries[getSlotCapacity<T>()];
 };

diff --git a/src/include/storage/index/in_mem_hash_index.h b/src/include/storage/index/in_mem_hash_index.h
@@ -53,6 +53,11 @@ class InMemHashIndex final {
     // Size of the validity mask
     static_assert(getSlotCapacity<T>() <= sizeof(SlotHeader().validityMask) * 8);
     static_assert(getSlotCapacity<T>() <= std::numeric_limits<entry_pos_t>::max() + 1);
+    static_assert(BaseDiskArray<Slot<T>>::getAlignedElementSize() <=
+                  common::HashIndexConstants::SLOT_CAPACITY_BYTES);
+    static_assert(BaseDiskArray<Slot<T>>::getAlignedElementSize() >= sizeof(Slot<T>));
+    static_assert(BaseDiskArray<Slot<T>>::getAlignedElementSize() >
+                  common::HashIndexConstants::SLOT_CAPACITY_BYTES / 2);
 
 public:
     explicit InMemHashIndex(OverflowFileHandle* overflowFileHandle);

diff --git a/src/include/storage/storage_structure/disk_array.h b/src/include/storage/storage_structure/disk_array.h
@@ -283,8 +283,8 @@ class BaseDiskArray {
 
 public:
     // Used by copiers.
-    BaseDiskArray(FileHandle& fileHandle, common::page_idx_t headerPageIdx, uint64_t elementSize)
-        : diskArray(fileHandle, headerPageIdx, elementSize) {}
+    BaseDiskArray(FileHandle& fileHandle, common::page_idx_t headerPageIdx)
+        : diskArray(fileHandle, headerPageIdx, getAlignedElementSize()) {}
     // Used when loading from file
     // If bypassWAL is set, the buffer manager is used to pages new to this transaction to the
     // original file, but does not handle flushing them. BufferManager::flushAllDirtyPagesInFrames
@@ -356,9 +356,7 @@ class BaseDiskArray {
 
     inline WriteIterator iter_mut() { return WriteIterator{diskArray.iter_mut(sizeof(U))}; }
     inline uint64_t getAPIdx(uint64_t idx) const { return diskArray.getAPIdx(idx); }
-    static constexpr uint32_t getAlignedElementSize() {
-        return (1 << (std::bit_width(sizeof(U)) - 1));
-    }
+    static constexpr uint32_t getAlignedElementSize() { return std::bit_ceil(sizeof(U)); }
 
 private:
     BaseDiskArrayInternal diskArray;

diff --git a/src/storage/storage_structure/overflow_file.cpp b/src/storage/storage_structure/overflow_file.cpp
@@ -182,6 +182,8 @@ void OverflowFile::createEmptyFiles(const std::string& fName, common::VirtualFil
     uint8_t page[common::BufferPoolConstants::PAGE_4KB_SIZE];
     StringOverflowFileHeader header;
     memcpy(page, &header, sizeof(StringOverflowFileHeader));
+    // Zero free space at the end of the header page
+    std::fill(page + sizeof(header), page + BufferPoolConstants::PAGE_4KB_SIZE, 0);
     fileHandle.addNewPage();
     fileHandle.writePage(page, HEADER_PAGE_IDX);
 }
@@ -218,6 +220,8 @@ void OverflowFile::prepareCommit() {
         uint8_t page[BufferPoolConstants::PAGE_4KB_SIZE];
         header.pages = pageCounter;
         memcpy(page, &header, sizeof(header));
+        // Zero free space at the end of the header page
+        std::fill(page + sizeof(header), page + BufferPoolConstants::PAGE_4KB_SIZE, 0);
         writePageToDisk(HEADER_PAGE_IDX, page);
     }
 }