search

etcd-io / bbolt

An embedded key/value database for Go.
https://go.etcd.io/bbolt
MIT License
8.16k stars 634 forks source link

No need to handle freelist as a specical case when freeing a page #788

Closed ahrtr closed 2 months ago

ahrtr commented 2 months ago

Read https://github.com/etcd-io/bbolt/pull/67#discussion_r1657120866

When the page ID to be freed isn't included in f.allocs, then it means it must be allocated before opening the db this time, and the write transaction is the very first write TXN after opening the db. So it (the page ID to be freed) must be visible to all readonly transactions. We just need to use the default value 0 for such case, in the same way as we did for other page types.

Will rebase this PR once https://github.com/etcd-io/bbolt/pull/775 gets merged.

The existing test cases already cover the sanity test. We will add more dedicated test cases for freelist management later.

cc @tjungblu @fuweid @ivanvc

k8s-ci-robot commented 2 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahrtr, fuweid, ivanvc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/etcd-io/bbolt/blob/main/OWNERS)~~ [ahrtr] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
tjungblu commented 2 months ago

/lgtm

Thanks @ahrtr - I'll adjust the testcases accordingly.