Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buffer Pool Manager (Project 1) Refactor #734

Merged
merged 25 commits into from
Sep 11, 2024
Merged

Conversation

connortsui20
Copy link
Member

@connortsui20 connortsui20 commented Sep 1, 2024

This PR will be the main PR for refactoring the buffer pool manager, which is the entirety of Project 1.

The overarching goal of this refactoring is to make the buffer pool manager API both thread-safe and type-safe. This means that use of raw pointers both internally and externally should be removed, and that thread-safe page guards should be the only way to interact with page data.

By doing this, we heavily limit the opportunities for misuse of the buffer pool manager API, which will in turn make the implementation and debugging of future projects (which will be much harder) easier.

Additionally, there will be implementation changes for the reference solution that will prefer indexing into arrays over raw pointers, limiting the opportunities for segfaults and memory corruption.

Here are the steps required for the refactor:

  • Make any modifications to the LRU-K replacer and disk scheduler (change LRU-K Evict() signature #731)
  • Change the BPM API in the public repo (d1fc14f)
    • Propagate the BPM API changes in the public repo (change occurrences of old methods to new ones)
    • Update tests that require OOM error / require old raw pointer API
  • Update the writeup [in progress]
  • Make all private repo changes (see private PR for more information [in progress])
  • Update all test cases
  • Update the starter code documentation
  • [Potentially more steps necessary + review]

src/buffer/buffer_pool_manager.cpp Outdated Show resolved Hide resolved
src/buffer/buffer_pool_manager.cpp Outdated Show resolved Hide resolved
src/buffer/buffer_pool_manager.cpp Outdated Show resolved Hide resolved
src/include/common/config.h Outdated Show resolved Hide resolved
@connortsui20 connortsui20 marked this pull request as ready for review September 10, 2024 13:30
@connortsui20
Copy link
Member Author

We need to also decide if we want to put the detailed implementation hints in the writeup (website) or keep them in the doc comments. Personally, I feel that documentation should always go right next to where the function implementation is, but I also understand that I made the implementation hints really long and detailed.

src/storage/disk/disk_manager.cpp Show resolved Hide resolved
src/include/storage/page/page.h Show resolved Hide resolved
test/storage/page_guard_test.cpp Show resolved Hide resolved
@connortsui20 connortsui20 merged commit a8081ff into master Sep 11, 2024
4 checks passed
@connortsui20 connortsui20 deleted the bpm-refactor-public branch September 11, 2024 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants