Conversation
| } | ||
|
|
||
| // Append bounding box bytes, shape bytes and the document score | ||
| g.boundingBoxes = append(g.boundingBoxes, f.EncodedBoundingBox()) |
There was a problem hiding this comment.
since these structures are called once for every doc, can you prealloc the slice capacity to numDocs?
| } | ||
|
|
||
| func (g *geoShapeV2IndexSectionOpaque) persist(w *FileWriter) error { | ||
|
|
There was a problem hiding this comment.
nit: can you drop these kind of empty lines?
There was a problem hiding this comment.
i think there are some funcs that have these kind of empty new lines. so can you please keep them consistent in all funcs?
| func (g *geoShapeV2IndexSection) Merge(opaque map[int]resetable, segments []*SegmentBase, | ||
| drops []*roaring.Bitmap, fieldsInv []string, newDocNumsIn [][]uint64, w *FileWriter, | ||
| closeCh chan struct{}) error { | ||
|
|
| // Assign merged geo docIDs and build, per segment, a direct | ||
| // oldGeoDocID -> mergedGeoDocID slice | ||
| segRemaps, numDocs := buildGeoDocRemaps(indexInfos, mergedContent) | ||
|
|
There was a problem hiding this comment.
nit: maybe you can remove these kind of new lines?
|
|
||
| // Each segment's inner and cross cells are already stored sorted | ||
| // So instead of concatenating every segment's cells and sorting the whole | ||
| // set, we k-way merge the pre-sorted per-segment runs while. |
There was a problem hiding this comment.
does the comment need to be updated?
| shapes: make([][]byte, 0, totalDocs), | ||
| } | ||
|
|
||
| // Assign merged geo docIDs and build, per segment, a direct |
There was a problem hiding this comment.
just to clarify - geoDocIDs are just the docIDs of the docs having geo_v2 field content right?
| continue | ||
| } | ||
| remap[geoDocID] = uint32(numDocs) | ||
| numDocs++ |
There was a problem hiding this comment.
since numDocs corresponds to the geoDocIDs, can you rename this to numGeoDocs?
| crossCells := f.CrossCells() | ||
|
|
||
| docID := uint32(len(g.docNums)) | ||
| g.docNums = append(g.docNums, docNum) |
There was a problem hiding this comment.
i'm guessing this is a mapping of geoDocIDs -> localDocNum (of the one having this geoshape field) right? can you rename docID to geoDocID? its a little confusing otherwise
and also going by that assumption, if one of the local docs doesn't have the geo shape field in it, then the mapping and the geoDocID generation would be such that we don't faulty mappings right? and you've verified in your functional testing?
| // merged geo docID. | ||
| func buildGeoDocRemaps(indexInfos []*geoIndexInfo, | ||
| mergedContent *geoIndexContent) (segRemaps [][]uint32, numDocs uint64) { | ||
|
|
| } | ||
|
|
||
| func (g *geoShapeV2IndexSectionOpaque) persist(w *FileWriter) error { | ||
|
|
There was a problem hiding this comment.
i think there are some funcs that have these kind of empty new lines. so can you please keep them consistent in all funcs?
| return nil, nil | ||
| } | ||
|
|
||
| entry, ok := gc.cache[field] |
There was a problem hiding this comment.
Is there a race case here?
we release the read lock before calling entry.load(). In that window cleanup() can take the write lock, see refs == 0, and evict + Close() this entry, after which we incRef() an orphaned entry. It's harmless today since Close() only decrements a refcount, but it becomes a use(only it gets FREE) the moment Close() frees real resources (the case the faiss vector cache guards against).
this might help:
if entry, ok := gc.cache[field]; ok {
defer gc.m.RUnlock()
return entry.load(except), nil
}
4 participants
Uh oh!
There was an error while loading. Please reload this page.