Skip to content

Commit

Permalink
Handle zero chunk size
Browse files Browse the repository at this point in the history
The existing implementation of the method
getChunkSize(...) (uint64, error), in some cases,
can return chunkSize==0 and err==nil.

The onus is on the caller to check for such possibility
and handle it properly.
Callers often use the returned chunkSize as a divisor and
a zero chunkSize lead to panic.
see #209

This PR intends to update the method implementation to
always return an error in case the returned chunkSize
value is 0.
That way caller need to only worry about error being non-nil.

Callers which are ok with 0 chunkSize can check the returned
error against ErrChunkSizeZero
  • Loading branch information
moshaad7 committed Sep 18, 2024
1 parent e148470 commit 3268000
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 4 deletions.
14 changes: 14 additions & 0 deletions chunk.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package zap

import (
"errors"
"fmt"
)

Expand All @@ -26,6 +27,13 @@ var LegacyChunkMode uint32 = 1024
// be used by default.
var DefaultChunkMode uint32 = 1026

var ErrChunkSizeZero = errors.New("chunk size is zero")

// getChunkSize returns the chunk size for the given chunkMode, cardinality, and
// maxDocs.
//
// In error cases, the returned chunk size will be 0. Caller can differentiate
// between a valid chunk size of 0 and an error by checking for ErrChunkSizeZero.
func getChunkSize(chunkMode uint32, cardinality uint64, maxDocs uint64) (uint64, error) {
switch {
// any chunkMode <= 1024 will always chunk with chunkSize=chunkMode
Expand All @@ -46,6 +54,9 @@ func getChunkSize(chunkMode uint32, cardinality uint64, maxDocs uint64) (uint64,
// chunk-size items.
// no attempt is made to tweak any other case
if cardinality <= 1024 {
if maxDocs == 0 {
return 0, ErrChunkSizeZero
}
return maxDocs, nil
}
return 1024, nil
Expand All @@ -61,6 +72,9 @@ func getChunkSize(chunkMode uint32, cardinality uint64, maxDocs uint64) (uint64,
// 2. convert to chunkSize, dividing into maxDocs
numChunks := (cardinality / 1024) + 1
chunkSize := maxDocs / numChunks
if chunkSize == 0 {
return 0, ErrChunkSizeZero
}
return chunkSize, nil
}
return 0, fmt.Errorf("unknown chunk mode %d", chunkMode)
Expand Down
13 changes: 9 additions & 4 deletions posting.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,7 @@ func (rv *PostingsList) read(postingsOffset uint64, d *Dictionary) error {
chunkSize, err := getChunkSize(d.sb.chunkMode,
rv.postings.GetCardinality(), d.sb.numDocs)
if err != nil {
return err
} else if chunkSize == 0 {
return fmt.Errorf("chunk size is zero, chunkMode: %v, numDocs: %v",
d.sb.chunkMode, d.sb.numDocs)
return fmt.Errorf("failed to get chunk size: %v", err)
}

rv.chunkSize = chunkSize
Expand Down Expand Up @@ -632,6 +629,10 @@ func (i *PostingsIterator) nextDocNumAtOrAfter(atOrAfter uint64) (uint64, bool,
return i.nextDocNumAtOrAfterClean(atOrAfter)
}

if i.postings.chunkSize == 0 {
return 0, false, ErrChunkSizeZero
}

i.Actual.AdvanceIfNeeded(uint32(atOrAfter))

if !i.Actual.HasNext() || !i.all.HasNext() {
Expand Down Expand Up @@ -741,6 +742,10 @@ func (i *PostingsIterator) nextDocNumAtOrAfterClean(
return uint64(i.Actual.Next()), true, nil
}

if i.postings != nil && i.postings.chunkSize == 0 {
return 0, false, ErrChunkSizeZero
}

// freq-norm's needed, so maintain freq-norm chunk reader
sameChunkNexts := 0 // # of times we called Next() in the same chunk
n := i.Actual.Next()
Expand Down

0 comments on commit 3268000

Please sign in to comment.