Cloud backup should check free space of cloud storage service before import |
|||||||||||||||||||||
Issue descriptionChrome Version: 57.0.2958.0 Chrome OS Version: 9034.0.0 Chrome OS Platform: Chromebook Pixel, and others Steps To Reproduce: (1) Check available space size of Google Drive. (2) Prepare an SD card with many new photos in DCIM/ folder. The total file size should exceed the available space of Google Drive. (3) Insert the SD card. Wait until Cloud Backup details dialog pops up. (4) Click "BACK UP" button. (5) Wait until the import finishes. Expected Result: - Cloud Backup should not start when the remote storage cannot store all the files being backed up. - Otherwise, it should allow retrying import of files that didn't fit the remote storage later. Actual Result: The import finishes successfully, but error occurs while syncing. - It first copies all files to local SSD saying "importing %d photos...". It finishes successfully as long as there's enough free space in the SSD. - After that, it starts to sync the files to Drive. - When the files on Drive exceeds the quota, it shows an error saying "%s was not uploaded. There is not enough free space in your Google Drive." How frequently does this problem reproduce? (Always, sometimes, hard to reproduce?) Always. What is the impact to the user, and is there a workaround? If so, what is it? - The files that could not be synced will be marked as "copying (cyclic arrows icon, instead of Drive icon)" in its status column. - Those files will be treated as already imported by Cloud Backup. - Those files will not be synced even after user emptied the Drive space.
,
Mar 22 2017
,
Oct 17 2017
Started investigating as part of work on infini-backup ( crbug.com/524313 ). Relevant file that will contain the code: ui/file_manager/file_manager/foreground/js/import_controller.js As soon as I have figured out how to check free Drive storage, I will start coding.
,
Oct 17 2017
Code fix written, tests will follow tomorrow
,
Oct 18 2017
,
Oct 18 2017
,
Oct 20 2017
Tests are taking much longer than expected, because the old tests were broken (there were errors in the console that didn't make the test fail) - working on fixing that because they seem to cause an infinite loop on my test
,
Oct 20 2017
Marianne, could you state how behavior changes with you cl and let it be reviewed by Weifang and Fukino ?
,
Oct 23 2017
Sure! Relevant method: https://cs.chromium.org/chromium/src/ui/file_manager/file_manager/foreground/js/import_controller.js?l=344&rcl=62e2e57b1d70dafad060d270ecd88e86a936ab1f Old behavior: After scanning the directory & selection for stuff that can get backed up, if there the scan was complete, it checks if there is enough space on the local disc and updates the UI accordingly (importer.ActivityState.INSUFICCIENT_SPACE or importer.ActivityState.READY). New behavior: Instead of only checking the local disc, also check the Drive Quota, and only if both have enough space, set the state to importer.ActivityState.READY. This is in preparation for the actual project that will take care of uploading in chunks if there is not enough space on the local disc, once I work on that the local space won't be relevant anymore for that check (or much less), but the check for the quota should be present nevertheless.
,
Oct 24 2017
The Cl without tests is in for review, as debugging the old tests take up too much time - once they are fixed I will adjust my test drafts.
,
Oct 25 2017
weifangsun@, mcirimele@: What do you think of the behavior I am suggesting? * Old behavior After scanning the directory & selection for stuff that can get backed up, if there the scan was complete, it checks if there is enough space on the local disc and updates the UI accordingly (importer.ActivityState.INSUFICCIENT_SPACE or importer.ActivityState.READY). If there is not enough space, cloud import is not possible, and clicking on the icon will provide the error message x new photos found Not enough space on device. Try selecting a few photos to start. * New behavior Instead of only checking the local disc, also check the Drive Quota, and only if both have enough space, set the state to importer.ActivityState.READY. If there is not enough space, cloud import is not possible, and clicking on the icon will provide the appropriate of the following error messages: x new photos found Not enough free space on device available. Try selecting fewer photos to start. x new photos found Not enough drive quota available. Try selecting fewer photos to start. * Comments on the behavior change This is in preparation of my intern project which will make backing up possible even if the local disc space is not enough by not backing up everything at the same time, but by regularly purging the cache to make space for more stuff - in the end, the only relevant space that needs to be free is the drive quota, after all.
,
Oct 25 2017
A few screenshots to illustrate the situation after my fix - there is one for enough space being available, one for the local storage error message, one for the drive quota error message, and one that I also have a question for (weifangsun@, mcirimele@) - this is the current behavior if Drive is available via the settings (chrome://settings => "Downlaods" in Advanced section => "Disconnect Google Drive account" dis-activated, the default state), but if no internet connection is possible - the scan will just seem to take forever because it is waiting for an answer from drive to get to know the remaining quota. What behavior is desired in this last case? The 2 options I think are most plausible is to hide the entire Cloud backup feature (as is currently already happening if the settings are used to deactivate Drive), or to show an error message similar to the ones for not enough space available, wdyt?
,
Nov 1 2017
Friendly ping :)
,
Nov 6 2017
weifangsun@: I have checked the code, it is no problem to add the size, as long as we choose only one unit for display (right now, it is in bytes, but I can change that to something else - if we want flexible units based on the size, it will be much more complicated. Another question: what should the expected behavior be in the case of no internet connection being available? Right now, it just scans forever, I guess we should update the error message accordingly?
,
Nov 6 2017
New messages: x new photos found with a total size of y bytes. Only z bytes available in drive quota. Try selecting fewer photos to start. and x new photos found with a total size of y bytes. Only z bytes available on device. Try selecting fewer photos to start.
,
Nov 6 2017
Follow-up on the question about no connection being possible to Drive: I guess that the old behavior (seemingly scanning forever because the UI never gets updated) is one that is definitely not wanted, so I have currently defaulted to option 1 of the 2 I can think of: 1. Hide the cloud import option. Downside: if there are a lot of files that get scanned, scanning will take some time, so the user might see the scan in progress with the scan then vanishing. 2. Show an error message. Downside: the actual cause cannot be tracked with the current structure (except for a generic: couldn't read the free storage size of some volume, which might be drive or the local storage), so the error message will have to be very generic. If you prefer to show an error message, do you have any idea of what kind of message would be appropriate for this case?
,
Nov 14 2017
Friendly ping! Is the error message with variable units ok?
,
Nov 15 2017
Looking good to me modulo TB is not realistic. Weifang, Maria. Could you review the error message in #18?
,
Nov 15 2017
Sorry for the delay! Was OOO on the 10th and missed this. +cotter@ for copy review - Could you review the error message in the UI under Comment #18? Context: In Chrome OS, we have an auto-backup feature to the cloud for photos. For this bug, we are trying to better communicate when there is not enough available space to (1) backup to the cloud or (2) retrieve the photos from the SD card locally prior to cloud backup.
,
Nov 16 2017
Small update for completeness: the exact wording for the error message on Drive is x new photos found. Not enough quota available on Google Drive: y missing. Try selecting fewer photos to start. With x being the number of photos, and y being the space that is missing, dynamically adjusting the unit for display. So the messages to be reviewed are in #18 and #22, with #18 having the layout as it appears in the UI.
,
Nov 17 2017
Issue 760816 has been merged into this issue.
,
Nov 20 2017
Friendly ping!
,
Nov 27 2017
cotter@, please review the message in #22.
,
Nov 29 2017
Re backup-available.png in comment 12: "back up" should be two words in the last sentence, as a verb (and to match spelling with two words above, as adverb), so the whole thing looks like this:
Cloud backup
Back up your media device’s photos and videos to Google Drive.
|| 1 new photos found
Ready to back up to Google Drive
Note also that the apostrophe in "device’s" should be curly: that is, ’ or \u2019. And if possible the code should account for singular "1 photo" vs. plural "X photos".
Revised text for screenshot in comment 18:
|| 1 new photos found
Not enough space available in local storage. Additional 9.1 GB needed.
Try selecting fewer photos.
Revised text for version in comment 22:
|| 1 new photos found
Your Google Drive quota isn't large enough. Additional 9.1 GB needed.
Try selecting fewer photos.
,
Nov 29 2017
An UX issue came up about this bug: Without the check, it is possible to back up things with no internet connection (by saving them locally temporarily until they can get synced). The check for Drive quota itself needs internet connection, though - the way the behavior is currently implemented, it will hide the option for Cloud Import in case of no internet connection being available. What is the desired behavior from a UX point of view? Options: * only take the Drive quota into account if there is internet connection * removing the option to back up things if there is no connection (what my CL that is not yet submitted proposed) * whatever option you can think of.
,
Nov 30 2017
> * removing the option to back up things if there is no connection (what my CL that is not yet submitted proposed) This option kills the existing scenario (starting cloud backup in offline environment), so we should be careful to decide it. How about starting with the first option (* only take the Drive quota into account if there is internet connection)? It will not be taken as a feature regression.
,
Dec 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f3e71f1fca66add51716aced9659e7dc09ad1f50 commit f3e71f1fca66add51716aced9659e7dc09ad1f50 Author: mariannet <mariannet@google.com> Date: Fri Dec 01 09:45:54 2017 Check available drive quota before offering the option to back up. Old behavior: Only local disc space gets checked. If enough local disc space was available, the option to back up was offered. When the drive quota got filled while backing up, the process of backing up data was silently aborted. New behavior: In addition to checking local disc space, the available drive quote also gets checked. If the user is offline, nothing changes - if he is online, both Drive and local storage must be sufficient for backup. Bug: 677104 Test: manually Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I0dbebc5c29f8f896a4dfa2c60d5860e6850dd69c Reviewed-on: https://chromium-review.googlesource.com/734927 Commit-Queue: Naoki Fukino <fukino@chromium.org> Reviewed-by: Naoki Fukino <fukino@chromium.org> Reviewed-by: Keigo Oka <oka@chromium.org> Cr-Commit-Position: refs/heads/master@{#520899} [modify] https://crrev.com/f3e71f1fca66add51716aced9659e7dc09ad1f50/chrome/app/chromeos_strings.grdp [modify] https://crrev.com/f3e71f1fca66add51716aced9659e7dc09ad1f50/chrome/browser/chromeos/extensions/file_manager/private_api_strings.cc [modify] https://crrev.com/f3e71f1fca66add51716aced9659e7dc09ad1f50/ui/file_manager/file_manager/common/js/importer_common.js [modify] https://crrev.com/f3e71f1fca66add51716aced9659e7dc09ad1f50/ui/file_manager/file_manager/foreground/js/import_controller.js
,
Dec 11 2017
The assigned owner "mariannet@google.com" is not able to receive e-mails, please re-triage. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 11 2017
fukino@, just double checking - Looks like this fix for this issue will target M65?
,
Dec 12 2017
I think we have fixed the issue. The new behavior is - If offline, we don't check the quota, and always show cloud-backup start button. - If online, we check the quota and disable cloud-backup when the remaining storage is not enough.
,
Dec 12 2017
Yes, it's already fixed by the commit in comment #29.
,
Dec 12 2017
,
Dec 27 2017
Hi, I think this problem is still not fixed. Here are my observations for this scenario: Prepared an SD card with bunch of photos in DCIM folder and inserted this SD card. This SD card had two partitions and had photos in both the partitions. When in insufficient Google Drive space, 1. Backup button was disabled in one partition and didn't allow photos for cloud backup. The other partition did allow cloud backup. Is this the expected behavior? 2. Cloud back up header was missing in the popup notification. 3. Even when many photos were available for back up in SD card partition 2, it always detected only 1 photo and the notification showed as '1 new photo found'. 4. When clicked on Backup button from partition 2/DCIM, not all photos were backed up. But all the photos in DCIM folder of partition 2 had status as badged. 5. Even though there were photos present in cloud backup folder of Google Drive, the same corresponding files in DCIM folder of partition 1 were not badged. Please refer the screenshots. Tested on M65 build (10254.0.0/ 65.0.3299.0).
,
Dec 28 2017
> 1. Backup button was disabled in one partition and didn't allow photos for cloud backup. The other partition did allow cloud backup. Is this the expected behavior? Will you give us the exact condition? - How much megabytes of files did each partition contain? - Did you select one file or no file? (also see #3 below) > 2. Cloud back up header was missing in the popup notification. I think "Cloud back up header" means the "Cloud backup / back up your media device's photos and videos to Google Drive" banner. If so, that's WAI. It is the same behavior since as existing one. (since 2 years ago) We hide the details panel once user completes backup. (I think it's because it's an introduction message). https://bugs.chromium.org/p/chromium/issues/detail?id=420680#c88 > 3. Even when many photos were available for back up in SD card partition 2, it always detected only 1 photo and the notification showed as '1 new photo found'. Please confirm these things: A: When you select 1 or more files, the cloud back up button shows whether it's possible to back up the new files among the selected file. On the other hand, when you select no file, the button shows whether it's possible to back up the new files among the all files in the folder. B: We have a de-duplication mechanism. Once a file is backed up, it is locally marked as already backed up, so that it'll be shown as already backed up regardless of if you removed the backed up file in My Drive or not. (though we have a known issue around this feature since the past. See https://bugs.chromium.org/p/chromium/issues/detail?id=672017&desc=3) > 4. When clicked on Backup button from partition 2/DCIM, not all photos were backed up. But all the photos in DCIM folder of partition 2 had status as badged. I think this may be WAI depending on the test condition. We also have another more careful de-duplication mechanism. If cloud backup found a file in user's Drive with the exactly same content, it'll stop copying the file again but mark it as already backed up. > 5. Even though there were photos present in cloud backup folder of Google Drive, the same corresponding files in DCIM folder of partition 1 were not badged. This is probably WAI. What do you mean by the "same corresponding files"? This happens when user copied file manually using the copy and paste, when using Cloud Import on another device, or when you had copy of that file with different file name backed up by Cloud Import. Even when the identical copy of the file exist somewhere in Drive, the file is only marked as backed up after it attempted to do cloud back up on each ChromeOS device. (and it's also reset when you erase the user account on that device). Expected behavior is that, if you trigger the cloud backup, it'll detect the duplication and mark the file as backed up, without actually copying the file to Drive duplicatedly.
,
Dec 28 2017
Re #36, Hi yamaguchi, here are my answers: >1. How much megabytes of files did each partition contain? --->One partition had around 60 files (around 200mb) for backup for first time. Hence back up button was disabled. Second partition had only few files (may be around few kbs) and back up option was available. I agree to your point in #3 that new files' back up was possible and hence the back up button showed up. Did you select one file or no file? (also see #3 below) --->No files were selected. >3. On the other hand, when you select no file, the button shows whether it's possible to back up the new files among the all files in the folder. ---> I had not selected any file. I agree with you on the de-duplication mechanism. But in this case, 3 new files were available for back up and were not existing before in cloud backup folder in Drive. But everytime backup notification says '1 new photo found' which is a wrong indication. And, when clicked on back up button all 3 files got backed up. >4. I agree with your point. But in this case, after clicking on back up button all photos had badged status in DCIM folder. But not all of them were backed up to Drive. Few files didn't back up even though they were not existing before. You can refer 'Screenshot_NoBadgedFiles.png'. The file names starting with 'adler' and 'african-lion' in SD_Card_2>DCIM folder have badged status but not backed up to Drive. >5. I meant, for those photos which were present in cloud back up folder in Drive, were not marked with badged status. I was expecting the file to be marked because of de-duplication mechanism. This file was not manually copied by way of copy-paste, but it was an old file which was existing before in cloud back up folder.
,
Jan 16 2018
Filed Issue 802230 for #3 in Comment 37. #4 still needs a bit more context. I assume partition 2 is the volume entitled as "SD_CARD_2" in the screenshot. Were the files in question already badged before triggering the backup? If a file were already badged as already-backed-up, it will never get scanned again regardless of if it actually exists in Google Drive folder. #5 Thanks, now I understood the situation. Currently the algorithm doesn't even try to find duplications (among the falsely detected new files) when there are not enough space in both. If it can detect duplications and feedback to the total file size computation, it'll get more chance to get the right conclusion. This is a remaining problem now.
,
Jan 16 2018
Hi yamaguchi, My reply for #4, The files in question got badged status after triggering the Back up. They were not badged before. These files were newly added to DCIM folder. But, after triggering backup, they really didn't get backed up. So, the issue here is if they are not backed up, then status should not be shown as badged.
,
Jan 17 2018
#4 I could not reproduce it on my environment. Can you try with another user to get more specific repro condition? One possible cause is that the file in question had a copy somewhere in Drive, not limited to the "cloud backup" folder. The matching remote file might be a different name than the local one.
,
Jan 18 2018
Hi yamaguchi, I couldn't repro the issue mentioned in #39 on latest M65 ToT build. Back up was enabled only when sufficient space was available in Drive. After clicking on Back up button, the files were synced to cloud back up and the same had badged status in DCIM folder.
,
Jan 18 2018
Thanks. Then can we consider that the issue (originally reported in Comment 35) >4. When clicked on Backup button from partition 2/DCIM, not all photos were backed up. But all the photos in DCIM folder of partition 2 had status as badged. was WAI, assuming that the user had an exactly the same file somewhere in Google Drive?
,
Jan 20 2018
Hi yamaguchi, for my comment in #41: I ran this scenario for a fresh user account (no prior cloud back up done), insufficient space in Drive and USB drive inserted on the device. With this set up, I couldn't repro this issue. Here are the steps I did for my comment in #39: I ran this scenario on a normal user account with SD card inserted and issue was repro'd. 1. Files> Google drive folder had limited space (around 45MB space). 2. Copied a .MOV file (of 174MB) to SD card> DCIM folder. This file was a recently created one. 3. Back up button was still enabled. Hence, clicked on BACK UP button. 4. This file didn't sync up to Drive (i.e. file didn't save to cloud back up folder in Drive). 5. But, the same file had badged status in DCIM folder. Attaching the issue video for better clarity. https://pantheon.corp.google.com/storage/browser/chromiumos-test-logs/bugfiles/cr/677104/
,
Jan 22 2018
I could reproduce it and identified the root cause. This happens when the cloud import scanned the file for the first time while the file size is still 0 byte. This happens when copying is in progress like above. The metadata cache is not flushed after file is overwritten. https://cs.chromium.org/chromium/src/ui/file_manager/file_manager/background/js/metadata_proxy.js?q=metadataProxy.getEntryMetadata&sq=package:chromium&l=26
,
Jan 31 2018
,
Feb 7 2018
,
Feb 7 2018
,
Feb 7 2018
,
Feb 16 2018
,
Mar 27 2018
Verified on M66 10452.28.0, 66.0.3359.62 beta-channel |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by yamaguchi@chromium.org
, Jan 6 2017