bustub
bustub copied to clipboard
Replacer API leading to student confusion
The buffer pool manager has Pin() and Unpin() functions. This is standard terminology, and we should probably keep it that way.
In fall 2019, we renamed the replacer API to have Pin() and Unpin() functions. This led to some student confusion; they were mixing up pinning and unpinning of replacer frames and buffer pool pages.
We should think about better names or a better API for the replacer.
The names pin() and unpin() are actually good.
What really leads to the confusion is the documentation of the contracts of these APIs. It's unclear from the docs of a Replacer interface should the pin() track a pinned page. Class homework provides relatively clear API spec yet it does not feel very natural that pin() is a remove operation by semantics and unpin() as an idempotent addition.
There is also a typo in a doc on victim method. It can't return nullptr:
- @param[out] frame_id id of frame that was removed, nullptr if no victim was found
Also as this is C++ and not C, it could return a tuple (frame_id_t, bool) or (Option<frame_id_t>) instead of using [out] *ptr as in pure C.
In my opinion, Unpin and Pin APIs should be complementary. So, Unpin should not be idempotent.
Otherwise, it causes source of bugs where we are unpinning a page which was never pinned or we forgot to pin a page.
In current requirements, i can't put assert in unpin page according to this test line
2022 fall's LRU-K should be less confusing.