Remove Graph call that could cause throttling issues (#2727)

The logic added in [2632](https://github.com/alcionai/corso/pull/2632) introduces an extra Graph call for
every item that is being backed up - which can lead to throttling errors.

Removing this till we figure out the right solution.

This commit also adds some extra logging to help debug throttling errors.

#### Does this PR need a docs update or release note?

- [ ]  Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x]  No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] 🌻 Feature
- [x] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Test
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* #2632 

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [x] 💪 Manual
- [ ]  Unit test
- [ ] 💚 E2E
This commit is contained in:
Vaibhav Kamra 2023-03-07 20:05:29 -08:00 committed by GitHub
parent 1ca49c53a9
commit 9c1d04e3f1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 43 additions and 31 deletions

View File

@ -29,6 +29,9 @@ const (
defaultMaxRetries = 3 defaultMaxRetries = 3
defaultDelay = 3 * time.Second defaultDelay = 3 * time.Second
absoluteMaxDelaySeconds = 180 absoluteMaxDelaySeconds = 180
rateLimitHeader = "RateLimit-Limit"
rateRemainingHeader = "RateLimit-Remaining"
rateResetHeader = "RateLimit-Reset"
) )
// AllMetadataFileNames produces the standard set of filenames used to store graph // AllMetadataFileNames produces the standard set of filenames used to store graph
@ -312,7 +315,12 @@ func (handler *LoggingMiddleware) Intercept(
} else { } else {
// special case for supportability: log all throttling cases. // special case for supportability: log all throttling cases.
if resp.StatusCode == http.StatusTooManyRequests { if resp.StatusCode == http.StatusTooManyRequests {
logger.Ctx(ctx).Infow("graph api throttling", "method", req.Method, "url", req.URL) logger.Ctx(ctx).Infow("graph api throttling",
"method", req.Method,
"url", req.URL,
"limit", resp.Header.Get(rateLimitHeader),
"remaining", resp.Header.Get(rateRemainingHeader),
"reset", resp.Header.Get(rateResetHeader))
} else if resp.StatusCode == http.StatusBadRequest { } else if resp.StatusCode == http.StatusBadRequest {
respDump, _ := httputil.DumpResponse(resp, true) respDump, _ := httputil.DumpResponse(resp, true)
logger.Ctx(ctx).Infow( logger.Ctx(ctx).Infow(

View File

@ -334,13 +334,17 @@ func (oc *Collection) populateItems(ctx context.Context, errs *fault.Bus) {
"backup_item_size", itemSize, "backup_item_size", itemSize,
) )
pr, err := fetchParentReference(ctx, oc.service, item.GetParentReference()) // TODO: Removing the logic below because it introduces an extra Graph API call for
if err != nil { // every item being backed up. This can lead to throttling errors.
el.AddRecoverable(clues.Wrap(err, "getting parent reference").Label(fault.LabelForceNoBackupCreation)) //
return // pr, err := fetchParentReference(ctx, oc.service, item.GetParentReference())
} // if err != nil {
// el.AddRecoverable(clues.Wrap(err, "getting parent reference").Label(fault.LabelForceNoBackupCreation))
// return
// }
// item.SetParentReference(pr)
item.SetParentReference(pr)
isFile := item.GetFile() != nil isFile := item.GetFile() != nil
if isFile { if isFile {

View File

@ -400,33 +400,33 @@ func constructWebURL(adtl map[string]any) string {
return url return url
} }
func fetchParentReference( // func fetchParentReference(
ctx context.Context, // ctx context.Context,
service graph.Servicer, // service graph.Servicer,
orig models.ItemReferenceable, // orig models.ItemReferenceable,
) (models.ItemReferenceable, error) { // ) (models.ItemReferenceable, error) {
if orig == nil || service == nil || ptr.Val(orig.GetName()) != "" { // if orig == nil || service == nil || ptr.Val(orig.GetName()) != "" {
return orig, nil // return orig, nil
} // }
options := &msdrives.DriveItemRequestBuilderGetRequestConfiguration{ // options := &msdrives.DriveItemRequestBuilderGetRequestConfiguration{
QueryParameters: &msdrives.DriveItemRequestBuilderGetQueryParameters{ // QueryParameters: &msdrives.DriveItemRequestBuilderGetQueryParameters{
Select: []string{"name"}, // Select: []string{"name"},
}, // },
} // }
driveID := ptr.Val(orig.GetDriveId()) // driveID := ptr.Val(orig.GetDriveId())
if driveID == "" { // if driveID == "" {
return orig, nil // return orig, nil
} // }
drive, err := service.Client().DrivesById(driveID).Get(ctx, options) // drive, err := service.Client().DrivesById(driveID).Get(ctx, options)
if err != nil { // if err != nil {
return nil, graph.Stack(ctx, err) // return nil, graph.Stack(ctx, err)
} // }
orig.SetName(drive.GetName()) // orig.SetName(drive.GetName())
return orig, nil // return orig, nil
} // }