mirror of
https://github.com/opencloud-eu/opencloud.git
synced 2026-09-12 21:58:58 -04:00
perf(search): prune the space fan-out for drive-pinned queries
When top-level AND conjuncts pin the query to a single root (driveId/RootID restrictions), only that space's index is asked; the restriction itself stays in the query, so this is purely an optimization. Conservative by design: any top-level OR, negated or group-nested restriction leaves the fan-out untouched, searching a space too many is wasted work while skipping one would be wrong. Mountpoints are kept for result path mapping. Costs one extra parse of the query in the service; parsing once and handing the AST to the engines (which currently re-parse per space) is a follow-up that changes the engine interface.
This commit is contained in:
1 parent
094baab7be
commit
95f175fe10
5 files changed
+116
-15
No files matched your search
@@ -6,10 +6,10 @@ import (
|
||||
|
||||
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
|
||||
"reflect"
|
||||
"strings"
|
||||
|
||||
"github.com/opencloud-eu/opencloud/pkg/ast"
|
||||
"github.com/opencloud-eu/opencloud/services/search/pkg/query/mimetype"
|
||||
"github.com/opencloud-eu/opencloud/services/search/pkg/search"
|
||||
)
|
||||
|
||||
// Normalize is the shared KQL lowering pass between parse and compile: it
|
||||
@@ -43,7 +43,7 @@ func normalizeNodes(nodes []ast.Node, resolve func(string) string, defaultKey st
|
||||
node.Value = resourceType(node.Value)
|
||||
}
|
||||
if node.Key == "RootID" {
|
||||
node.Value = completeRootID(node.Value)
|
||||
node.Value = search.CompleteRootID(node.Value)
|
||||
}
|
||||
if exp := mimetype.Expand(node.Key, node.Value); exp != nil {
|
||||
out = append(out, normalizeNodes(exp, resolve, defaultKey)...)
|
||||
@@ -75,19 +75,6 @@ func normalizeNodes(nodes []ast.Node, resolve func(string) string, defaultKey st
|
||||
return out
|
||||
}
|
||||
|
||||
// completeRootID turns a driveId ("storage$space") into the full root
|
||||
// resource id ("storage$space!space") stored in the index: a space root's
|
||||
// opaque id is its space id. Full ids pass through untouched.
|
||||
func completeRootID(v string) string {
|
||||
if strings.Contains(v, "!") {
|
||||
return v
|
||||
}
|
||||
if i := strings.LastIndex(v, "$"); i >= 0 && i+1 < len(v) {
|
||||
return v + "!" + v[i+1:]
|
||||
}
|
||||
return v
|
||||
}
|
||||
|
||||
// toPointer returns n as a pointer; the parser emits some nodes by value and the
|
||||
// in-place key rewrites would be lost on those.
|
||||
func toPointer(n ast.Node) ast.Node {
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
package search_test
|
||||
|
||||
import (
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
|
||||
"github.com/opencloud-eu/opencloud/services/search/pkg/search"
|
||||
)
|
||||
|
||||
var _ = DescribeTable("PinnedRootID",
|
||||
func(query, want string) {
|
||||
Expect(search.PinnedRootID(query)).To(Equal(want))
|
||||
},
|
||||
Entry("bare driveId is completed to the root id", `driveId:"1$2"`, "1$2!2"),
|
||||
Entry("full root id passes through", `driveId:"1$2!4" name:x`, "1$2!4"),
|
||||
Entry("RootID works as well", `RootID:"1$2" AND name:x`, "1$2!2"),
|
||||
Entry("top-level AND conjuncts pin", `driveId:"1$2" AND name:x AND tag:y`, "1$2!2"),
|
||||
Entry("repeated identical AND restrictions pin", `driveId:"1$2" AND driveId:"1$2"`, "1$2!2"),
|
||||
// KQL groups adjacent same-key restrictions into an implicit OR group
|
||||
Entry("implicitly repeated restrictions do not pin", `driveId:"1$2" driveId:"1$2"`, ""),
|
||||
Entry("no restriction, no pin", `name:x`, ""),
|
||||
Entry("a top-level OR disables pruning", `name:x OR driveId:"1$2"`, ""),
|
||||
Entry("OR between other terms disables pruning", `driveId:"1$2" AND (a OR b)`, "1$2!2"),
|
||||
Entry("a negated restriction disables pruning", `NOT driveId:"1$2" AND name:x`, ""),
|
||||
Entry("restrictions inside groups do not pin", `(driveId:"1$2") AND name:x`, ""),
|
||||
Entry("two different roots disable pruning", `driveId:"1$2" AND driveId:"1$3"`, ""),
|
||||
Entry("invalid query, no pin", `((`, ""),
|
||||
)
|
||||
@@ -11,6 +11,8 @@ import (
|
||||
gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
|
||||
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
|
||||
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
|
||||
"github.com/opencloud-eu/opencloud/pkg/ast"
|
||||
"github.com/opencloud-eu/opencloud/pkg/kql"
|
||||
"github.com/opencloud-eu/reva/v2/pkg/conversions"
|
||||
"github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool"
|
||||
"github.com/opencloud-eu/reva/v2/pkg/storage/utils/grants"
|
||||
@@ -223,6 +225,65 @@ func convertToWebDAVPermissions(isShared, isMountpoint, isDir bool, p *provider.
|
||||
return b.String()
|
||||
}
|
||||
|
||||
// CompleteRootID completes a bare driveId ("storage$space") to the root
|
||||
// resource id stored in the index; a root's opaque id is its space id.
|
||||
func CompleteRootID(v string) string {
|
||||
if strings.Contains(v, "!") {
|
||||
return v
|
||||
}
|
||||
if i := strings.LastIndex(v, "$"); i >= 0 && i+1 < len(v) {
|
||||
return v + "!" + v[i+1:]
|
||||
}
|
||||
return v
|
||||
}
|
||||
|
||||
// PinnedRootID returns the single space root the query is pinned to via
|
||||
// top-level AND driveId/RootID conjuncts, or "". OR, NOT and group-nested
|
||||
// restrictions never pin: skipping a space would be wrong.
|
||||
func PinnedRootID(query string) string {
|
||||
a, err := kql.Builder{}.Build(query)
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
pinned := ""
|
||||
negated := false
|
||||
for _, n := range a.Nodes {
|
||||
switch node := n.(type) {
|
||||
case *ast.OperatorNode:
|
||||
if strings.EqualFold(node.Value, "OR") {
|
||||
return ""
|
||||
}
|
||||
negated = strings.EqualFold(node.Value, "NOT")
|
||||
continue
|
||||
case ast.OperatorNode:
|
||||
if strings.EqualFold(node.Value, "OR") {
|
||||
return ""
|
||||
}
|
||||
negated = strings.EqualFold(node.Value, "NOT")
|
||||
continue
|
||||
}
|
||||
var key, value string
|
||||
switch node := n.(type) {
|
||||
case *ast.StringNode:
|
||||
key, value = node.Key, node.Value
|
||||
case ast.StringNode:
|
||||
key, value = node.Key, node.Value
|
||||
}
|
||||
if strings.EqualFold(key, "driveid") || strings.EqualFold(key, "rootid") {
|
||||
if negated {
|
||||
return ""
|
||||
}
|
||||
v := CompleteRootID(value)
|
||||
if pinned != "" && pinned != v {
|
||||
return ""
|
||||
}
|
||||
pinned = v
|
||||
}
|
||||
negated = false
|
||||
}
|
||||
return pinned
|
||||
}
|
||||
|
||||
// ParseScope extract a scope value from the query string and returns search, scope strings
|
||||
func ParseScope(query string) (string, string) {
|
||||
match := scopeRegex.FindStringSubmatch(query)
|
||||
|
||||
@@ -167,6 +167,17 @@ func (s *Service) Search(ctx context.Context, req *searchsvc.SearchRequest) (*se
|
||||
Path: gpRes.Path,
|
||||
}
|
||||
}
|
||||
// drive-pinned queries only need that space's index; the restriction
|
||||
// stays in the query, mountpoints are kept for path mapping
|
||||
var pinnedRoot *provider.ResourceId
|
||||
if req.Ref == nil {
|
||||
if pinned := PinnedRootID(req.Query); pinned != "" {
|
||||
if rid, err := storagespace.ParseID(pinned); err == nil {
|
||||
pinnedRoot = &rid
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
filters := []*provider.ListStorageSpacesRequest_Filter{
|
||||
{
|
||||
Type: provider.ListStorageSpacesRequest_Filter_TYPE_USER,
|
||||
@@ -195,6 +206,9 @@ func (s *Service) Search(ctx context.Context, req *searchsvc.SearchRequest) (*se
|
||||
// We still need the mountpoint in order to map the result paths to the according share
|
||||
continue
|
||||
}
|
||||
if pinnedRoot != nil && space.SpaceType != _spaceTypeMountpoint && pinnedRoot.GetSpaceId() != space.Root.GetSpaceId() {
|
||||
continue
|
||||
}
|
||||
spaces = append(spaces, space)
|
||||
}
|
||||
|
||||
|
||||
@@ -498,6 +498,17 @@ var _ = Describe("Searchprovider", func() {
|
||||
}, nil)
|
||||
})
|
||||
|
||||
It("prunes the fan-out when the query pins a drive", func() {
|
||||
res, err := s.Search(ctx, &searchsvc.SearchRequest{
|
||||
Query: `driveId:"storageid$personalspace" AND foo`,
|
||||
})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(res).ToNot(BeNil())
|
||||
Expect(len(res.Matches)).To(Equal(1))
|
||||
Expect(res.Matches[0].Entity.Id.OpaqueId).To(Equal("foo-id"))
|
||||
indexClient.AssertNumberOfCalls(GinkgoT(), "Search", 1)
|
||||
})
|
||||
|
||||
It("considers the search Ref parameter", func() {
|
||||
res, err := s.Search(ctx, &searchsvc.SearchRequest{
|
||||
Query: "foo",
|
||||
|
||||
Reference in new issue
Block a user