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

Multiple folders on Google Drive with the same name when uploading? #588

Closed
mehrdadn opened this issue Sep 3, 2017 · 38 comments
Closed
Milestone

Comments

@mehrdadn
Copy link

mehrdadn commented Sep 3, 2017

Every few days I wake up to a new folder made on my Google Drive with the exact same name as the one already there, which the program is supposed to upload into. It seems like every so often the program decides to create a new folder to upload its archive to? I don't understand why this is, since normally it just updates the previous file just fine, but my hunch is that this may occur whenever the connection drops sometime during the upload... maybe it thinks the folder doesn't exist because a connection error, so it creates a new one? I'm not sure, but if you could look into this that would be fantastic. Thank you!

@mendhak
Copy link
Owner

mendhak commented Sep 3, 2017

I modified the logic, previously it was searching by folder name. Somehow this must be failing intermittently and no idea why - permissions lost?

The new logic is to check for the folder ID in preferences and do a basic check against GDrive to see if it's still there. If it doesn't exist, search by name, if still not there, then create it.

image

Here's an APK to test with

APK, Signature, Checksum

@mendhak mendhak added this to the v90 milestone Sep 3, 2017
@mehrdadn
Copy link
Author

mehrdadn commented Sep 3, 2017

Thanks for the quick response! I'll install it right away and see if the issue arises again.

@mehrdadn
Copy link
Author

mehrdadn commented Sep 7, 2017

Update: It seems this has solved the folder issue, so it's much better than before, but I noticed it still doesn't always fix the file issue -- I got two ZIP files with the same name in the folder today. Not sure why that would have happened but it seems duplicate files can still get created.

@rfkinline
Copy link

On my end the folder issue started the 7th of September and it is still creating a new folder every time

@rfkinline
Copy link

Update: the system stopped creating every time a copy of the folder. Now it creates one copy (GPSLogger for Android (1)) and stores the files in this folder. The folder in the settings is: "GPSLogger for Android". If I change that to GPSLogger for AndroidT, then the system will create GPSLogger for AndroidT (1)

@mendhak
Copy link
Owner

mendhak commented Sep 22, 2017

@rfkinline are you seeing this problem with the APK I've posted above or the regular market version?

@rfkinline
Copy link

with the regular App

@mendhak
Copy link
Owner

mendhak commented Sep 22, 2017

If you are able, try to test with the above APK - it should be more stable now - the testing will help. Or you can wait a bit eventually I'll release this fix.

@mendhak
Copy link
Owner

mendhak commented Nov 8, 2017

Eventually, in this case, meant 2 months -> v90 released to market.

@mendhak mendhak closed this as completed Nov 8, 2017
@mehrdadn
Copy link
Author

mehrdadn commented Jul 12, 2018

Hi @mendhak, this issue still isn't fixed unfortunately (version 94). Every few weeks I check my Google Drive and I see 2-3 folders by GPSLogger with the same name. I don't know what the edge case is, but it still seems like it sometimes creates a new folder instead of updating an existing one.

I have a suspicion the problem occurs when there is a network or server or other unexpected error (for whatever reason). For example, what do you do if checking for an existing folder fails? Looking at your code, it seems to me that you simply assume the folder doesn't exist and therefore you create a new one, hence causing the problem. What I think you would instead want to do is to try again for a few times with e.g. exponential backoff (or until the next upload time, the minimum of the two), unless you know the error is unrecoverable (e.g. access denied due to changed sharing permissions, etc.), in which case I wouldn't retry at all. And as long as there is no success, I'd treat it as an upload error rather than trying to create a new folder—after all, the user required you to upload to a particular path, and the user really would expect an error if you cannot access that path. (Or you could provide an option to create a new folder on error, but I don't think it should be on by default.)

And as to why the retrieve-by-ID would fail, I have no idea... I haven't reinstalled or even changed the app settings in months, but the problem still keeps occurring.

@rfkinline
Copy link

It did work for some time, but it looks like the fix was removed again. I also have multiple directories again, but I learned to live with it. There are now 4 directories and everything is stored/updated in the last directory. Not sure if it helps. The original directory was created 14Nov17, second and third: 27Mar18. Last and current one 22Apr18. Directories 2 and 3 are empty

@mendhak mendhak reopened this Jul 12, 2018
@mendhak
Copy link
Owner

mendhak commented Jul 12, 2018

Hey that is an interesting idea - I wouldn't even have expected such a basic request to fail. I think the simplest thing I can do here is to allow that particular exception to bubble up. In turn the bubbling should get captured in the job queue... and in turn allow the next file-upload to do the job. Or change the job to retry after a little while.

@mehrdadn
Copy link
Author

I don't actually know if it's failing, it's just my hunch that it's failing because of network connectivity or something like that. If you have a way for me to check let me know.

@xentavo
Copy link

xentavo commented Jul 12, 2018

You never know with connectivity, but in my case the system has not created a directory in over 4 months. I have the app running 24/7 and I have been moving all over the place. Also why would there be two empty directories? Maybe something to do with time? Here is something curious: The timestamp of those two empty directories is before their time of creation (creation: 6:24, modified 2:38; same day). I was in the same time-zone that day.
https://imgur.com/Tfgzwhv

@rfkinline
Copy link

Sorry, above message from xentavo should have come from my other github account rfkinline

@mehrdadn
Copy link
Author

There's also another bug I just saw, which is that if I trash the folder it previously uploaded to, it will still continue to receive the uploads, meaning I won't see the uploads in my drive (even if a folder with the correct name already existed there).

It's possible to work around this by discovering the path on every upload, but I feel it might actually mean that using the folder or file IDs is just a bad idea in the first place, and that attempting to discover the path based on the ID might still encounter another edge case I'm not aware of...

@mendhak
Copy link
Owner

mendhak commented Jul 14, 2018

Storing the folder ID was introduced after the search-by-name kept behaving in strange ways. It's actually reduced a lot of emails about the Google Drive multiple-folder problem. So in other words, discovering the path on every upload was introducing more problems than it was solving.

The workaround here would be if you trash the folder, delete it permanently instead; or if you want a fresh start, empty the contents of the folder.

@mehrdadn
Copy link
Author

Well personally I of course realize the workaround, but it's not realistic to expect users to know the bug and the workaround, and to be willing or even able to use this workaround—you're effectively asking the users to stop using the "Trash" feature of Google Drive.

I think there must be a proper solution here, so if you'd like to provider a link or other information, I'm happy to help you figure out a better solution for whatever problems people were experiencing...

@mendhak
Copy link
Owner

mendhak commented Jul 14, 2018

The problem was exactly the same as described here, just with a higher rate of occurrence among users. The "GPSLogger for Android" folder was being created multiple times in Google Drive. The getFileIdFromFileName method would do its /drive/v2/files request and I believe that's where something was happening; introducing the persisted folder ID helped a lot.

I say 'believe' because - the difficulty with troubleshooting this is not I don't know how to reproduce it. Also it's unrealistic to ask users to run a debuglog file for months waiting for the problem to happen again.

Of course removing the locally stored ID would solve the trashing case at the expense of again increasing the rate of the original issue.

Yep, any suggestions would be good. Perhaps extra calls to the Drive API to check that the folder hasn't been trashed. Requires extra calls.

Drive REST API v3 has been introduced. This could be worth trying; from experience Google tend to 'forget' that older versions exist.

This also exists, the Drive library. I've been avoiding this for a long time since it will increase the APK size.

@mehrdadn
Copy link
Author

OK, I see. Have you tried distinguishing between the different types of errors and handling them differently as I suggested above? I suspect that's the underlying problem's solution, and is likely to address what other users have encountered too. Specifically:

  • Errors reported by Google Drive should result in failure to upload (and possibly user notification?)
  • Network & other errors should result in a retry with increasing timeouts, bounded by the next upload time
  • Non-existent folders/files should result in a new folder/file being created, not an error

@rfkinline
Copy link

As advised I just deleted all the folders (GPSLogger) on google drive (local and cloud). I changed the destination folder in the app to GPS but the system is still using the old folder. Looks like the issue is a bit more complex as I don't want to touch the cash.

@mehrdadn
Copy link
Author

mehrdadn commented Jul 14, 2018

@rfkinline Have you ensured the folder is not in your Trash? Merely "deleting" them would not be enough if it sends the folder to Trash. The fact that the name change doesn't help suggests that the ID is still being used, which I guess supports my claim that the ID should not be used naively like this.

@mendhak:
As for optimization & reducing the number of API calls:

Note that you cannot check to see if the folder is trashed, because one of its ancestors may have been trashed instead—or it may not have been even trashed, but moved. Furthermore, Google Drive's file system model supports multiple parent folders, and while I'm not too familiar with how it works, I think it means that it doesn't even makes sense to ask if the folder is trashed, since it may be trashed via path A but not via path B.

You probably can't reduce the number of API calls, because you are required to traverse all components of the path to make sure they are correct. However, you can parallelize them if you cache the IDs of all the folders in the path. (This is a bit tedious as it requires retrying until you retrieve all of the path components' parents. Again, you have to check for name changes and such, and again, you would need to distinguish between network errors and API errors.) I would not perform this optimization in the beginning, though... I would first get it correct and then try to optimize once I know the behavior is correct, because it's likely that the optimization will introduce bugs of its own, and it won't be clear where the problem is.

@rfkinline
Copy link

Yes, I emptied the trash folder before changing the folder in the app.

@mehrdadn
Copy link
Author

Confused, how do you say it "still uses the old folder" if the old folder has been permanently deleted?

@rfkinline
Copy link

Sorry.
Old in the app = GPSlogger. Old in Google Drive = GPSLogger, GPSLogger(1), GPSLogger(2) and GPSLogger(3).
New in the app = GPS. New in Google Drive = GPSLogger, GPSLogger(1)
(after deleting and also re-testing the connection): https://imgur.com/igUcpM4,

@mehrdadn
Copy link
Author

Huh, so basically the name change in the app isn't taking effect? That seems like a second bug...

@rfkinline
Copy link

Correct. It might be that it is the cache, but I am afraid of emptying it as it might mess up my settings.

@rfkinline
Copy link

Some screenshots:
https://imgur.com/6iWTm4c
https://imgur.com/pVMaL9R

@mendhak
Copy link
Owner

mendhak commented Jul 14, 2018

Ah twofold bug 😞 . When changing the folder name the old ID is still cached. Rename should have cleared the ID, so I'll add this to the list if still going with the caching-ID route.

The other issue is - Google Drive even after you permanently delete something... doesn't delete it. The API will still report that a resource exists and it will continue sending your uploads to that location. There is some internal schedule that Google Drive follows at which point the object is actually deleted.

I have a crude workaround for you. In the GPSLogger folder on your phone, create a file called gpslogger.properties and in that put

GDRIVE_FOLDER_ID=

Close everything, reopen GPSLogger. This will cause your cached ID to be reset. Go back and delete that file. Now try your upload again.

@mendhak
Copy link
Owner

mendhak commented Jul 21, 2018

Working on a new branch - gdrivefix.

So far I've:

  • Switched to Google Drive API v3.
  • Removed the cached folder ID - now the folder will be searched each time. If you change the folder name it should reflect right away.
  • Removed catch around the ID search, so now the exception will bubble and cause the Job Queue re-queues it. This is new logic, haven't tried before. I hope this should cater to the problem that may have been occurring - search for folder failing, causing it to be recreated. But now the search failure should be retried.
  • Latest version of Job Queue - supports exponential backoff. Applied to Google Drive at 3s.

If you're interested you can try to sideload. I will be testing at home.

APK, signature, checksum

@rfkinline
Copy link

rfkinline commented Jul 21, 2018

Great!. I will test too on a new/clean android phone

@mehrdadn
Copy link
Author

Oh cool, thanks! I'll check it out.

@rfkinline
Copy link

Feedback: it works like a charm.
I have now GPS logger running on 2 phones and I noticed that on one phone the battery usage is 20% (600mAH) and on the other one 5% (160mAH). With exactly the same settings. Is this of any use for you?

@mehrdadn
Copy link
Author

It does seem to be working for me as well, but it seems too early to tell for sure—it usually took a few weeks to start misbehaving, so it's hard to tell right now.

@mendhak
Copy link
Owner

mendhak commented Jul 27, 2018

Yes totally agree, a few days may not show the issue but it can appear totally unexpected later. So I'm still running it on and off, over several weeks. Will come back to this later.

@mehrdadn
Copy link
Author

mehrdadn commented Aug 4, 2018

For what it's worth I have not seen any uploading problems yet, so that's good news for now. :-)
Regarding the battery, I have a weird feeling that my battery usage has also increased—even during periods in which my data is turned off—since around the same time I installed this, but it's really hard for me to say if it's actually related, since I can't do a proper experiment without leaving my phone unused for a while. (I use BetterBatteryStat and it doesn't indicate a clear culprit unfortunately.) There's a good chance that it's a coincidence (or I'm just imagining things), but if you get the same report from other people, then I would maybe look into it. I'll let you know if I can narrow it down.

@mehrdadn
Copy link
Author

Yeah I think the battery issue was unrelated. The patch is all good as far as I can tell so far :) but if I see anything to the contrary I will let you know.

@mendhak
Copy link
Owner

mendhak commented Sep 30, 2018

Fixed in v96, will reopen if it happens again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants