THSS-DB / TDB

Educational Database Management System for Software School of Tsinghua University
Mulan Permissive Software License, Version 2
13 stars 17 forks source link

Potential issue in invocation of FileBufferPool::evict_all_pages() in BplusTreeHandler::sync() #17

Closed hotwords123 closed 2 months ago

hotwords123 commented 2 months ago

背景:Lab1 实现了 FileBufferPoolevict_all_pages 方法,实验文档的描述为:

evict_all_pages函数是在关闭文件时被调用的,因此需要将该文件对应的Frame都依次驱逐并刷盘

由于 FileBufferPool 在文件打开时,成员变量 hdr_frame_file_header_ 始终持有一份 page 0 的引用(用于记录文件元信息和页面分配情况),因此在引用释放前理论上不应该调用 evict_all_pages,否则将因为无法释放 page 0 而产生错误。FileBufferPoolclose_file 方法在调用 evict_all_pages() 前先执行了 hdr_frame_->unpin(),也印证了这一点。

问题:在 BplusTreeHandlersync 方法中(创建 index 时会在写入 FIRST_INDEX_PAGE 后调用它),也调用了 file_buffer_pool_->evict_all_pages(),目的应该是把文件更改刷盘。但这里文件并没有被关闭,因此这里的调用不符合 evict_all_pages 的先决条件,会导致错误。

可能的修复方案(不完全):从语义上看,这里只需要将内存中的页面刷盘即可,不需要驱逐页面,可以考虑添加一个 flush_all_pages 方法。或者在 evict_all_pages 中特判 page 0 的情况,但这样会使函数功能不明确(不符合实验文档的描述),增加维护复杂度。

RkGrit commented 2 months ago

BplusTreeHandler 的 sync 方法是在 BplusTreeHandler 的 create 方法中调用的,在调用前,会先执行bp->unpin_page(header_frame)。

hotwords123 commented 2 months ago

BplusTreeHandler 的 sync 方法是在 BplusTreeHandler 的 create 方法中调用的,在调用前,会先执行bp->unpin_page(header_frame)。

这里 unpin 的是 B+ 树索引的 header page,记录的是索引的元信息,它的 page_numFIRST_INDEX_PAGE,也就是 1。

https://github.com/THSS-DB/TDB/blob/a4537c07cd8cb37c599f34b8c74e7e3fb2657eb4/src/server/storage_engine/index/bplus_tree.cpp#L749-L762

但是 hdr_frame_ 持有的是整个文件的 header page,记录文件的元信息,它的 page_num 是 0,不是同一个 page。前面提到的是这个 header page。

https://github.com/THSS-DB/TDB/blob/a4537c07cd8cb37c599f34b8c74e7e3fb2657eb4/src/server/storage_engine/buffer/buffer_pool.cpp#L46-L57

RkGrit commented 2 months ago

是的,hdrframe 持有的是整个文件的元信息,但它没有必要在每次sync的时候刷盘的,只需要在关闭索引文件的时候被刷盘也能保证正确性。

hotwords123 commented 2 months ago

是的,hdrframe 持有的是整个文件的元信息,但它没有必要在每次sync的时候刷盘的,只需要在关闭索引文件的时候被刷盘也能保证正确性。

嗯,但是如果按原先的代码,在 sync 中调用 evict_all_pages 根据文档又应该驱逐 page 0,就产生了前面的问题,所以您觉得怎么改比较好呢?

RkGrit commented 2 months ago

那可以像你说的,增加一个flush_all_pages 方法

hotwords123 commented 2 months ago

那可以像你说的,增加一个flush_all_pages 方法

已经在 #19 中添加了。