Repository navigation
replace database data storage with BSUP columns instead of old-bsup - #7424
Conversation
This commit replaces the storage format of the databsae with BSUP columns. We're currently suboptimally writing one big column frame per storage object but this will change in a subsequent PR when we add a row-group-like construct (pruner and vcache just need to understand this). We put merge and the pool scanners in subdirectories of runtime/op to break import loops. We need to revisit interfaces again to come up with a cleaner design but this will come later. The ingest path is now native columns though there is an scode boomerang to sort the input. We'll avoid this and add optional sort keys in a subsequent PR.
| type PullCloser interface { | ||
| Puller | ||
| io.Closer | ||
| } |
There was a problem hiding this comment.
Nit: Fine to merge this but in the long run I don't think we need or want this or ScanCloser since a call to sbuf.File.Pull that returns nil, nil can close the underlying file (after we we address the vector.TypeValue.loader issue, that is).
There was a problem hiding this comment.
Good point, let's fix later!
| w.Respond(http.StatusOK, api.CommitResponse{Commit: branch.Commit}) | ||
| return |
There was a problem hiding this comment.
w.Respond has already been called up above.
| w.Respond(http.StatusOK, api.CommitResponse{Commit: branch.Commit}) | |
| return |
There was a problem hiding this comment.
It keeps coming back!
There was a problem hiding this comment.
Nit: Call this poolscanner.go. And if you put it in runtime/op instead of a separate package, you won't have to duplicate runtime.op.applyMask.
There was a problem hiding this comment.
I'll follow up with a subsequent PR
| return sbuf.NewDematerializer(sctx, scanner), nil | ||
| // XXX We're passing nil for pushdown here. In a subsequent PR, we will | ||
| // work out vector pushdown and put this back. | ||
| return poolscanner.NewPoolScanner(runtime.NewContext(ctx, sctx), l, pool, nil, nil, nil), nil |
There was a problem hiding this comment.
Nit: runtime/op.Robot.open is the only thing that calls this method, so instead of moving runtime/op.Merge into a separate package to break the import cycle, move this method to runtime/op by hanging it on runtime/op.Robot.
There was a problem hiding this comment.
I'll follow up with a subsequent PR
| if sortKey != nil { | ||
| var minVal, maxVal super.Value | ||
| vec, minVal, maxVal = w.sort(vec) | ||
| if sortKey.Order == order.Desc { | ||
| minVal, maxVal = maxVal, minVal |
There was a problem hiding this comment.
sortKey is never nil given sortKey := &key above.
| if sortKey != nil { | |
| var minVal, maxVal super.Value | |
| vec, minVal, maxVal = w.sort(vec) | |
| if sortKey.Order == order.Desc { | |
| minVal, maxVal = maxVal, minVal | |
| var minVal, maxVal super.Value | |
| vec, minVal, maxVal = w.sort(vec) | |
| if w.pool.SortKeys.Primary().Order == order.Desc { | |
| minVal, maxVal = maxVal, minVal |
There was a problem hiding this comment.
I wrote it this way because sortkey will soon change to be optional and the &sortkey will go away.
| uri := object.URI(p.DataPath) | ||
| r, err := p.engine.Get(ctx, uri) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("%s: %w", uri, err) | ||
| } | ||
| scanner, err := bsupio.NewReader(ctx, sctx, r, pushdown, concurrency) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return &struct { | ||
| vio.Scanner | ||
| io.Closer | ||
| }{ | ||
| Scanner: scanner, | ||
| // XXX We block close because vcache does lazy loading of | ||
| // type values. This relies on GC to close files. We need | ||
| // to work out a better way at some point. | ||
| Closer: io.NopCloser(r), | ||
| }, nil | ||
| } |
There was a problem hiding this comment.
It's known to be bsup columns...?
There was a problem hiding this comment.
Yes, that's why you'd pass anyio.ReaderOpts{Format: "bsup"}.
The advantage of anyio.Open is that it takes care of things like closing the storage.Reader if bsupio.NewReader fails, which it looks like we're not doing here.
2 participants
This commit replaces the storage format of the databsae with BSUP columns. We're currently suboptimally writing one big column frame per storage object but this will change in a subsequent PR when we add a row-group-like construct (pruner and vcache just need to understand this).
We put merge and the pool scanners in subdirectories of runtime/op to break import loops. We need to revisit interfaces again to come up with a cleaner design but this will come later.
The ingest path is now native columns though there is an scode boomerang to sort the input. We'll avoid this and add optional sort keys in a subsequent PR.