Skip to content
This repository has been archived by the owner on Oct 11, 2024. It is now read-only.

Speedup OneDrive backup by pulling urls in delta API #1842

Merged
merged 16 commits into from
Dec 20, 2022
Merged

Conversation

meain
Copy link
Member

@meain meain commented Dec 16, 2022

Description

This brings in another huge improvement in the backup speed dropping from ~15m to ~1.5m for an account with ~5000 files. This one also drastically reduces the number of requests we have to make for the same account from 5505 to just ~10. This also means that we don't get throttled anymore and we can easily run multiple backup jobs in parallel before we hit the 1024 limit.

The code works as of now, but I have to double check the numbers as well as fix an issue with us opening too many files(can't seem to reproduce it now) and causing program to crash with 'too many open files' when we bump up the numbers. With the current numbers, it works, but I wanna double check and optimize them. Plus some code cleanup.

Does this PR need a docs update or release note?

  • ✅ Yes, it's included
  • 🕐 Yes, but in a later PR
  • ⛔ No

Type of change

  • 🌻 Feature
  • 🐛 Bugfix
  • 🗺️ Documentation
  • 🤖 Test
  • 💻 CI/Deployment
  • 🐹 Trivial/Minor

Issue(s)

  • #

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@meain meain self-assigned this Dec 16, 2022
@meain meain temporarily deployed to Testing December 16, 2022 10:06 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 16, 2022 10:06 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 16, 2022 10:07 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 16, 2022 10:07 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 16, 2022 10:07 — with GitHub Actions Inactive
Copy link
Contributor

@ryanfkeepers ryanfkeepers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally seems okay. I'm a little curious about memory concerns (it'd have to be a very, very large drive). But nothing stands out as an immediate concern.

src/internal/connector/onedrive/drive.go Outdated Show resolved Hide resolved
@meain meain temporarily deployed to Testing December 19, 2022 10:04 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 19, 2022 10:04 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 19, 2022 10:04 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 19, 2022 10:05 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 19, 2022 10:40 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 19, 2022 10:40 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 19, 2022 10:40 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 19, 2022 10:41 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 19, 2022 10:48 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 19, 2022 10:48 — with GitHub Actions Inactive
@aviator-app aviator-app bot added the blocked Upstream item prevents completion label Dec 20, 2022
@aviator-app
Copy link
Contributor

aviator-app bot commented Dec 20, 2022

PR failed to merge with reason: some CI status(es) failed.
Failed CI(s): Website-Linting

@aviator-app aviator-app bot removed the blocked Upstream item prevents completion label Dec 20, 2022
@meain meain temporarily deployed to Testing December 20, 2022 09:20 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 20, 2022 09:20 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 20, 2022 09:20 — with GitHub Actions Inactive
@sonarcloud
Copy link

sonarcloud bot commented Dec 20, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
17.3% 17.3% Duplication

@meain meain temporarily deployed to Testing December 20, 2022 09:21 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 20, 2022 09:21 — with GitHub Actions Inactive
@meain meain temporarily deployed to Testing December 20, 2022 09:21 — with GitHub Actions Inactive
@aviator-app aviator-app bot merged commit a4147f5 into main Dec 20, 2022
@aviator-app aviator-app bot deleted the less-requests branch December 20, 2022 09:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants