Skip to content

Commit

Permalink
Ranges cache: add ownership check (#6004)
Browse files Browse the repository at this point in the history
* add ownership check. if the value isn't owned, copy it

* stop copying value's bytes since the byte buffer is owned by the calling function
  • Loading branch information
Jonathan-Rosenberg authored Jun 4, 2023
1 parent 39d1444 commit 03068a7
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 4 deletions.
3 changes: 1 addition & 2 deletions pkg/graveler/committed/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ func getBytes(b *[]byte) ([]byte, error) {
if l < 0 {
return nil, fmt.Errorf("impossible negative length %d: %w", l, ErrBadValueBytes)
}
ret := make([]byte, l)
copy(ret, (*b)[:l])
ret := (*b)[:l]
*b = (*b)[l:]
return ret, nil
}
Expand Down
16 changes: 14 additions & 2 deletions pkg/graveler/sstable/range_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,13 @@ func (m *RangeManager) GetValueGE(ctx context.Context, ns committed.Namespace, i
}
return nil, ErrKeyNotFound
}
vBytes, _, err := value.Value(nil)
vBytes, owned, err := value.Value(nil)
if !owned {
b := vBytes
vBytes = make([]byte, len(b))
copy(vBytes, b)
}

if err != nil {
return nil, fmt.Errorf("extract value from sstable id %s (key %s): %w", id, key, err)
}
Expand Down Expand Up @@ -132,7 +138,13 @@ func (m *RangeManager) GetValue(ctx context.Context, ns committed.Namespace, id
// lookup path in range but key not found
return nil, ErrKeyNotFound
}
vBytes, _, err := value.Value(nil)
vBytes, owned, err := value.Value(nil)
if !owned {
b := vBytes
vBytes = make([]byte, len(b))
copy(vBytes, b)
}

if err != nil {
return nil, fmt.Errorf("extract value from sstable id %s (key %s): %w", id, key, err)
}
Expand Down

0 comments on commit 03068a7

Please sign in to comment.