Skip to content

Commit

Permalink
Dedupe Reload/NoSyncReload, prefer empty instead of nil init
Browse files Browse the repository at this point in the history
Reload and NoSyncReload have duplicated code, this unifies both
for later refactoring.

This PR is split from etcd-io#786, where the tests found differences on reloading
and nil/empty initializations. Added some more clarifications in godocs
for certain panic behavior and expected returns on the interface.

Signed-off-by: Thomas Jungblut <[email protected]>
  • Loading branch information
tjungblu committed Aug 5, 2024
1 parent 02e13c1 commit 4061583
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 24 deletions.
7 changes: 4 additions & 3 deletions internal/freelist/freelist.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type ReadWriter interface {
// Write writes the freelist into the given page.
Write(page *common.Page)

// EstimatedWritePageSize returns the size of the freelist after serialization in Write.
// EstimatedWritePageSize returns the size in bytes of the freelist after serialization in Write.
// This should never underestimate the size.
EstimatedWritePageSize() int
}
Expand Down Expand Up @@ -46,13 +46,14 @@ type Interface interface {
ReleasePendingPages()

// Free releases a page and its overflow for a given transaction id.
// If the page is already free then a panic will occur.
// If the page is already free or is one of the meta pages, then a panic will occur.
Free(txId common.Txid, p *common.Page)

// Freed returns whether a given page is in the free list.
Freed(pgId common.Pgid) bool

// Rollback removes the pages from a given pending tx.
// Should always be followed by Reload or NoSyncReload from last freelist state.
Rollback(txId common.Txid)

// Copyall copies a list of all free ids and all pending ids in one sorted list.
Expand All @@ -65,7 +66,7 @@ type Interface interface {
// NoSyncReload reads the freelist from Pgids and filters out pending items.
NoSyncReload(pgIds common.Pgids)

// freePageIds returns the IDs of all free pages.
// freePageIds returns the IDs of all free pages. No free pages returns an empty slice.
freePageIds() common.Pgids

// pendingPageIds returns all pending pages by transaction id.
Expand Down
24 changes: 3 additions & 21 deletions internal/freelist/shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,25 +208,7 @@ func (t *shared) Copyall(dst []common.Pgid) {

func (t *shared) Reload(p *common.Page) {
t.Read(p)

// Build a cache of only pending pages.
pcache := make(map[common.Pgid]bool)
for _, txp := range t.pending {
for _, pendingID := range txp.ids {
pcache[pendingID] = true
}
}

// Check each page in the freelist and build a new available freelist
// with any pages not in the pending lists.
var a []common.Pgid
for _, id := range t.freePageIds() {
if !pcache[id] {
a = append(a, id)
}
}

t.Init(a)
t.NoSyncReload(t.freePageIds())
}

func (t *shared) NoSyncReload(pgIds common.Pgids) {
Expand All @@ -240,7 +222,7 @@ func (t *shared) NoSyncReload(pgIds common.Pgids) {

// Check each page in the freelist and build a new available freelist
// with any pages not in the pending lists.
var a []common.Pgid
a := []common.Pgid{}
for _, id := range pgIds {
if !pcache[id] {
a = append(a, id)
Expand Down Expand Up @@ -274,7 +256,7 @@ func (t *shared) Read(p *common.Page) {

// Copy the list of page ids from the freelist.
if len(ids) == 0 {
t.Init(nil)
t.Init([]common.Pgid{})
} else {
// copy the ids, so we don't modify on the freelist page directly
idsCopy := make([]common.Pgid, len(ids))
Expand Down

0 comments on commit 4061583

Please sign in to comment.