From 3b233d271d57934b6740fe62d0291ec9d35b0c5b Mon Sep 17 00:00:00 2001 From: Jacob Brinlee <66123601+jrbrinlee1@users.noreply.github.com> Date: Fri, 6 Jan 2023 14:04:17 -0600 Subject: [PATCH] adding more error handling for rbf (#2395) * adding more error handling for rbf Co-authored-by: Jacob Brinlee (cherry picked from commit c9ce26ce968a7ef82d8c7823feb8696bdfa72612) --- fragment.go | 9 +++++---- rbf.go | 6 ++++-- rbf/cursorx.go | 11 +++++++---- rbf/tx.go | 15 ++++++++++++--- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/fragment.go b/fragment.go index 1609374d1..801a9d2a7 100644 --- a/fragment.go +++ b/fragment.go @@ -2089,7 +2089,7 @@ func (f *fragment) ImportRoaringClearAndSet(ctx context.Context, tx Tx, clear, s err = tx.ApplyRewriter(f.index(), f.field(), f.view(), f.shard, 0, rewriter) if err != nil { - errors.Wrap(err, "applying rewriter") + return fmt.Errorf("pilosa.ImportRoaringClearAndSet: %s", err) } if f.CacheType != CacheTypeNone { // TODO this may be quite a bit slower than the way @@ -2132,7 +2132,7 @@ func (f *fragment) ImportRoaringBSI(ctx context.Context, tx Tx, clear, set []byt } err = tx.ApplyRewriter(f.index(), f.field(), f.view(), f.shard, 0, rewriter) - return errors.Wrap(err, "applying rewriter") + return errors.Wrap(err, "pilosa.ImportRoaringBSI: ") } // ImportRoaringSingleValued treats "clear" as a single row and clears @@ -2156,7 +2156,7 @@ func (f *fragment) ImportRoaringSingleValued(ctx context.Context, tx Tx, clear, } err = tx.ApplyRewriter(f.index(), f.field(), f.view(), f.shard, 0, rewriter) - return errors.Wrap(err, "applying rewriter") + return errors.Wrap(err, "pilosa.ImportRoaringSingleValued: ") } func (f *fragment) doImportRoaring(ctx context.Context, tx Tx, data []byte, clear bool) (map[uint64]int, bool, error) { @@ -2722,7 +2722,8 @@ func (f *fragment) intRowIterator(tx Tx, wrap bool, filters ...roaring.BitmapFil func (f *fragment) foreachRow(tx Tx, filters []roaring.BitmapFilter, fn func(rid uint64) error) error { filter := roaring.NewBitmapRowFilter(fn, filters...) - return tx.ApplyFilter(f.index(), f.field(), f.view(), f.shard, 0, filter) + err := tx.ApplyFilter(f.index(), f.field(), f.view(), f.shard, 0, filter) + return errors.Wrap(err, "pilosa.foreachRow: ") } func (it *intRowIterator) Seek(rowID uint64) { diff --git a/rbf.go b/rbf.go index b82f94363..78081f1bc 100644 --- a/rbf.go +++ b/rbf.go @@ -388,11 +388,13 @@ func (tx *RBFTx) ImportRoaringBits(index, field, view string, shard uint64, rit } func (tx *RBFTx) ApplyFilter(index, field, view string, shard uint64, ckey uint64, filter roaring.BitmapFilter) (err error) { - return tx.tx.ApplyFilter(rbfName(index, field, view, shard), ckey, filter) + err = tx.tx.ApplyFilter(rbfName(index, field, view, shard), ckey, filter) + return errors.Wrap(err, fmt.Sprintf("applying filter for index %s, field %s, view %s, shard %d", index, field, view, shard)) } func (tx *RBFTx) ApplyRewriter(index, field, view string, shard uint64, ckey uint64, filter roaring.BitmapRewriter) (err error) { - return tx.tx.ApplyRewriter(rbfName(index, field, view, shard), ckey, filter) + err = tx.tx.ApplyRewriter(rbfName(index, field, view, shard), ckey, filter) + return errors.Wrap(err, fmt.Sprintf("applying rewriter for index %s, field %s, view %s, shard %d", index, field, view, shard)) } func (tx *RBFTx) GetSortedFieldViewList(idx *Index, shard uint64) (fvs []txkey.FieldView, err error) { diff --git a/rbf/cursorx.go b/rbf/cursorx.go index 9a4b98003..cbd56d928 100644 --- a/rbf/cursorx.go +++ b/rbf/cursorx.go @@ -196,9 +196,9 @@ func intoContainer(l leafCell, tx *Tx, replacing *roaring.Container, target []by // intoWritableContainer always uses the provided target for a copy of // the container's contents, so the container can be modified safely. -func intoWritableContainer(l leafCell, tx *Tx, replacing *roaring.Container, target []byte) (c *roaring.Container) { +func intoWritableContainer(l leafCell, tx *Tx, replacing *roaring.Container, target []byte) (c *roaring.Container, err error) { if len(l.Data) == 0 { - return nil + return nil, nil } orig := l.Data target = target[:len(orig)] @@ -209,7 +209,10 @@ func intoWritableContainer(l leafCell, tx *Tx, replacing *roaring.Container, tar case ContainerTypeBitmapPtr: pgno := toPgno(target) target = target[:PageSize] // reslice back to full size - _, bm, _ := tx.leafCellBitmapInto(pgno, target) + _, bm, err := tx.leafCellBitmapInto(pgno, target) + if err != nil { + return nil, fmt.Errorf("intoContainer: %s", err) + } c = roaring.RemakeContainerBitmapN(replacing, bm, int32(l.BitN)) case ContainerTypeBitmap: c = roaring.RemakeContainerBitmapN(replacing, toArray64(target), int32(l.BitN)) @@ -221,7 +224,7 @@ func intoWritableContainer(l leafCell, tx *Tx, replacing *roaring.Container, tar // expensive. c.CheckN() c.SetMapped(false) - return c + return c, nil } func toContainer(l leafCell, tx *Tx) (c *roaring.Container) { diff --git a/rbf/tx.go b/rbf/tx.go index 04d8f9488..0c0df1272 100644 --- a/rbf/tx.go +++ b/rbf/tx.go @@ -1668,7 +1668,10 @@ func (s *containerFilter) ApplyFilter() (err error) { } for err := s.cursor.Next(); err == nil; err = s.cursor.Next() { elem := &s.cursor.stack.elems[s.cursor.stack.top] - leafPage, _, _ := s.cursor.tx.readPage(elem.pgno) + leafPage, _, err := s.cursor.tx.readPage(elem.pgno) + if err != nil { + return fmt.Errorf("reading from pgno %d applying filter: %s", elem.pgno, err) + } readLeafCellInto(&cell, leafPage, elem.index) key := roaring.FilterKey(cell.Key) if key < minKey { @@ -1730,7 +1733,10 @@ func (s *containerFilter) ApplyRewriter() (err error) { } for err := s.cursor.Next(); err == nil; err = s.cursor.Next() { elem := &s.cursor.stack.elems[s.cursor.stack.top] - leafPage, _, _ := s.cursor.tx.readPage(elem.pgno) + leafPage, _, err := s.cursor.tx.readPage(elem.pgno) + if err != nil { + return fmt.Errorf("reading from pgno %d applying rewriter: %s", elem.pgno, err) + } readLeafCellInto(&cell, leafPage, elem.index) key = roaring.FilterKey(cell.Key) if key < minKey { @@ -1741,7 +1747,10 @@ func (s *containerFilter) ApplyRewriter() (err error) { return res.Err } if res.YesKey <= key && res.NoKey <= key { - data := intoWritableContainer(cell, s.cursor.tx, &s.header, s.body[:]) + data, err := intoWritableContainer(cell, s.cursor.tx, &s.header, s.body[:]) + if err != nil { + return fmt.Errorf("applying rewriter: %s", err) + } res = s.rewriter.RewriteData(key, data, writeback) if res.Err != nil { return res.Err