From fdbfd89b855735c2980f7a095d7a6eecd39d125a Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Wed, 21 Aug 2024 15:10:34 +0300 Subject: [PATCH] [#471] tree: Don't use sorted GetSubTree for nodes without FileName Sorted GetSubTree doesn't return nodes without FileName attribute if there are more than 1000 nodes in subtree Signed-off-by: Denis Kirillov --- internal/frostfs/services/pool_wrapper.go | 8 ++++++-- pkg/service/tree/tree.go | 16 ++++++++++------ pkg/service/tree/tree_client_in_memory.go | 6 +++++- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/internal/frostfs/services/pool_wrapper.go b/internal/frostfs/services/pool_wrapper.go index a1ee198..a96b198 100644 --- a/internal/frostfs/services/pool_wrapper.go +++ b/internal/frostfs/services/pool_wrapper.go @@ -102,14 +102,18 @@ func (w *PoolWrapper) GetNodes(ctx context.Context, prm *tree.GetNodesParams) ([ return res, nil } -func (w *PoolWrapper) GetSubTree(ctx context.Context, bktInfo *data.BucketInfo, treeID string, rootID []uint64, depth uint32) ([]tree.NodeResponse, error) { +func (w *PoolWrapper) GetSubTree(ctx context.Context, bktInfo *data.BucketInfo, treeID string, rootID []uint64, depth uint32, sort bool) ([]tree.NodeResponse, error) { + order := treepool.NoneOrder + if sort { + order = treepool.AscendingOrder + } poolPrm := treepool.GetSubTreeParams{ CID: bktInfo.CID, TreeID: treeID, RootID: rootID, Depth: depth, BearerToken: getBearer(ctx, bktInfo), - Order: treepool.AscendingOrder, + Order: order, } if len(rootID) == 1 && rootID[0] == 0 { // storage node interprets 'nil' value as []uint64{0} diff --git a/pkg/service/tree/tree.go b/pkg/service/tree/tree.go index 5e19fc6..1026c3c 100644 --- a/pkg/service/tree/tree.go +++ b/pkg/service/tree/tree.go @@ -32,7 +32,7 @@ type ( // Each method must return ErrNodeNotFound or ErrNodeAccessDenied if relevant. ServiceClient interface { GetNodes(ctx context.Context, p *GetNodesParams) ([]NodeResponse, error) - GetSubTree(ctx context.Context, bktInfo *data.BucketInfo, treeID string, rootID []uint64, depth uint32) ([]NodeResponse, error) + GetSubTree(ctx context.Context, bktInfo *data.BucketInfo, treeID string, rootID []uint64, depth uint32, sort bool) ([]NodeResponse, error) GetSubTreeStream(ctx context.Context, bktInfo *data.BucketInfo, treeID string, rootID []uint64, depth uint32) (SubTreeStream, error) AddNode(ctx context.Context, bktInfo *data.BucketInfo, treeID string, parent uint64, meta map[string]string) (uint64, error) AddNodeByPath(ctx context.Context, bktInfo *data.BucketInfo, treeID string, path []string, meta map[string]string) (uint64, error) @@ -699,7 +699,7 @@ func (c *Tree) getTreeNode(ctx context.Context, bktInfo *data.BucketInfo, nodeID } func (c *Tree) getTreeNodes(ctx context.Context, bktInfo *data.BucketInfo, nodeID uint64, keys ...string) (map[string]*treeNode, error) { - subtree, err := c.service.GetSubTree(ctx, bktInfo, versionTree, []uint64{nodeID}, 2) + subtree, err := c.service.GetSubTree(ctx, bktInfo, versionTree, []uint64{nodeID}, 2, false) if err != nil { return nil, err } @@ -1093,7 +1093,7 @@ func (c *Tree) getSubTreeByPrefix(ctx context.Context, bktInfo *data.BucketInfo, return nil, "", err } - subTree, err := c.service.GetSubTree(ctx, bktInfo, treeID, rootID, 2) + subTree, err := c.service.GetSubTree(ctx, bktInfo, treeID, rootID, 2, false) if err != nil { if errors.Is(err, layer.ErrNodeNotFound) { return nil, "", nil @@ -1251,7 +1251,11 @@ func (c *Tree) GetMultipartUploadsByPrefix(ctx context.Context, bktInfo *data.Bu } func (c *Tree) getSubTreeMultipartUploads(ctx context.Context, bktInfo *data.BucketInfo, nodeID []uint64, parentFilePath string) ([]*data.MultipartInfo, error) { - subTree, err := c.service.GetSubTree(ctx, bktInfo, systemTree, nodeID, maxGetSubTreeDepth) + // sorting in getSubTree leads to skipping nodes that doesn't have FileName attribute + // so when we are only interested in multipart nodes, we can set this flag + // (despite we sort multiparts in above layer anyway) + // to skip its children (parts) that don't have FileName + subTree, err := c.service.GetSubTree(ctx, bktInfo, systemTree, nodeID, maxGetSubTreeDepth, true) if err != nil { return nil, err } @@ -1340,7 +1344,7 @@ func (c *Tree) GetMultipartUpload(ctx context.Context, bktInfo *data.BucketInfo, } func (c *Tree) AddPart(ctx context.Context, bktInfo *data.BucketInfo, multipartNodeID uint64, info *data.PartInfo) (oldObjIDToDelete oid.ID, err error) { - parts, err := c.service.GetSubTree(ctx, bktInfo, systemTree, []uint64{multipartNodeID}, 2) + parts, err := c.service.GetSubTree(ctx, bktInfo, systemTree, []uint64{multipartNodeID}, 2, false) if err != nil { return oid.ID{}, err } @@ -1390,7 +1394,7 @@ func (c *Tree) AddPart(ctx context.Context, bktInfo *data.BucketInfo, multipartN } func (c *Tree) GetParts(ctx context.Context, bktInfo *data.BucketInfo, multipartNodeID uint64) ([]*data.PartInfo, error) { - parts, err := c.service.GetSubTree(ctx, bktInfo, systemTree, []uint64{multipartNodeID}, 2) + parts, err := c.service.GetSubTree(ctx, bktInfo, systemTree, []uint64{multipartNodeID}, 2, false) if err != nil { return nil, err } diff --git a/pkg/service/tree/tree_client_in_memory.go b/pkg/service/tree/tree_client_in_memory.go index 55a81a7..4c27fd7 100644 --- a/pkg/service/tree/tree_client_in_memory.go +++ b/pkg/service/tree/tree_client_in_memory.go @@ -234,7 +234,7 @@ func (c *ServiceClientMemory) GetNodes(_ context.Context, p *GetNodesParams) ([] return res2, nil } -func (c *ServiceClientMemory) GetSubTree(_ context.Context, bktInfo *data.BucketInfo, treeID string, rootID []uint64, depth uint32) ([]NodeResponse, error) { +func (c *ServiceClientMemory) GetSubTree(_ context.Context, bktInfo *data.BucketInfo, treeID string, rootID []uint64, depth uint32, sort bool) ([]NodeResponse, error) { cnr, ok := c.containers[bktInfo.CID.EncodeToString()] if !ok { return nil, nil @@ -254,6 +254,10 @@ func (c *ServiceClientMemory) GetSubTree(_ context.Context, bktInfo *data.Bucket return nil, ErrNodeNotFound } + if sort { + sortNode(tr.treeData) + } + // we depth-1 in case of uint32 and 0 as mark to get all subtree leads to overflow and depth is getting quite big to walk all tree levels return node.listNodes(nil, depth-1), nil }