Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetchShares in the sharesstorageprovider does not parallelize stat requests #9828

Open
butonic opened this issue Aug 16, 2024 · 3 comments
Open
Labels

Comments

@butonic
Copy link
Member

butonic commented Aug 16, 2024

looking at the code ... we are sequentially stating all accepted shares. this will take ages and can be parallelized. maybe even cached

func (s *service) fetchShares(ctx context.Context, opaque *typesv1beta1.Opaque, arbitraryMetadataKeys []string, fieldMask *field_mask.FieldMask) ([]*collaboration.ReceivedShare, map[string]*provider.ResourceInfo, error) {
	sharingCollaborationClient, err := s.sharingCollaborationSelector.Next()
	if err != nil {
		return nil, nil, err
	}

	lsRes, err := sharingCollaborationClient.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{
		// FIXME filter by received shares for resource id - listing all shares is tooo expensive!
	})
	if err != nil {
		return nil, nil, errors.Wrap(err, "sharesstorageprovider: error calling ListReceivedSharesRequest")
	}
	if lsRes.Status.Code != rpc.Code_CODE_OK {
		return nil, nil, fmt.Errorf("sharesstorageprovider: error calling ListReceivedSharesRequest")
	}

	gatewayClient, err := s.gatewaySelector.Next()
	if err != nil {
		return nil, nil, err
	}

	shareMetaData := make(map[string]*provider.ResourceInfo, len(lsRes.Shares))
	for _, rs := range lsRes.Shares {
		// only stat accepted shares
		if rs.State != collaboration.ShareState_SHARE_STATE_ACCEPTED {
			continue
		}
		if rs.Share.ResourceId.SpaceId == "" {
			// convert backwards compatible share id
			rs.Share.ResourceId.StorageId, rs.Share.ResourceId.SpaceId = storagespace.SplitStorageID(rs.Share.ResourceId.StorageId)
		}
		sRes, err := gatewayClient.Stat(ctx, &provider.StatRequest{
			Opaque:                opaque,
			Ref:                   &provider.Reference{ResourceId: rs.Share.ResourceId},
			ArbitraryMetadataKeys: arbitraryMetadataKeys,
			FieldMask:             fieldMask,
		})
		if err != nil {
			appctx.GetLogger(ctx).Error().
				Err(err).
				Interface("resourceID", rs.Share.ResourceId).
				Msg("ListRecievedShares: failed to make stat call")
			continue
		}
		if sRes.Status.Code != rpc.Code_CODE_OK {
			appctx.GetLogger(ctx).Debug().
				Interface("resourceID", rs.Share.ResourceId).
				Interface("status", sRes.Status).
				Msg("ListRecievedShares: failed to stat the resource")
			continue
		}
		shareMetaData[rs.Share.Id.OpaqueId] = sRes.Info
	}

	return lsRes.Shares, shareMetaData, nil
}

fix by concurrently stating resources. make the number of concurrent go routines configurable.

@butonic
Copy link
Member Author

butonic commented Aug 16, 2024

related, but bitrotted: cs3org/reva#3940

@butonic
Copy link
Member Author

butonic commented Aug 19, 2024

fixed by cs3org/reva#4812

@butonic butonic closed this as completed Aug 19, 2024
@butonic
Copy link
Member Author

butonic commented Aug 19, 2024

  • make the concurrency configurable in ocis, it defaults to 5 but this could be tweaked

@butonic butonic reopened this Aug 19, 2024
@micbar micbar mentioned this issue Sep 2, 2024
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Backlog
Development

No branches or pull requests

1 participant