Add NextReader to BlockReader #603
Conversation
Kubuxu
requested review from
rvagg,
willscott,
masih and
hannahhoward
as code owners
BlockReader.NextReader facilities reading larger blocks from the CAR file. Signed-off-by: Jakub Sztandera <[email protected]>
Kubuxu
force-pushed
the
feat/next-reader
branch
from
97a3941 to
a1db313
Compare
v2/internal/carv1/util/util.go
Outdated
| if size > math.MaxInt64 { | ||
| return cid.Cid{}, 0, ErrHeaderTooLarge | ||
| } |
There was a problem hiding this comment.
| if size > math.MaxInt64 { | |
| return cid.Cid{}, 0, ErrHeaderTooLarge | |
| } |
this is unnecessary, it's what LdReadSize does with its third arg for you to generate a uniform error pattern
There was a problem hiding this comment.
also if this really is necessary, ErrSectionTooLarge cause this isn't actually a header.
There was a problem hiding this comment.
Added this because the int64 cast for LimitReader (don't want to cast to negatives)
v2/block_reader.go
Outdated
| // on the BlockReader. | ||
| // The returned length might be larger than MaxAllowedSectionSize, it is up to the user to check before loading the data into memory. | ||
| func (br *BlockReader) NextReader() (cid.Cid, io.Reader, uint64, error) { | ||
| c, length, err := util.ReadNodeHeader(br.r, br.opts.ZeroLengthSectionAsEOF, math.MaxUint64) |
There was a problem hiding this comment.
| c, length, err := util.ReadNodeHeader(br.r, br.opts.ZeroLengthSectionAsEOF, math.MaxUint64) | |
| c, length, err := util.ReadNodeHeader(br.r, br.opts.ZeroLengthSectionAsEOF, math.MaxInt64) |
Or is there a reason you're using MaxUint64 but then capping it at MaxInt64 inside ReadNodeHeader? Why not just use MaxInt64 all the way through.
There was a problem hiding this comment.
Because the maxsize param is uint64 everywhere, so from an external API viewpoint, I'm passing unlimited (maximum allowed value). Then the function internally caps to what it is able to handle.
Both perspectives make sense though
Signed-off-by: Jakub Sztandera <[email protected]>
|
@rvagg : will you be ok to merge and cut a release at your convenience as filecoin-project/lotus#13129 will be depending on it (being worked on next week). |
BlockReader.NextReaderfacilities reading larger blocks from the CAR file.This is necessary for the support of the Filecoin snapshot format extension (FRC-108, lotus issue), where a large block is used to transfer data about F3.