Skip to content

Commit

Permalink
[BP] Handle zero chunk size
Browse files Browse the repository at this point in the history
This is a backport of #265.

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 authored and abhinavdangeti committed Oct 15, 2024
1 parent a72794c commit 3148db2
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 3148db2

Please sign in to comment.