LcTwisk / google-api-objectivec-client

Automatically exported from code.google.com/p/google-api-objectivec-client
0 stars 0 forks source link

When upload large file to Google Drive, upload progress goes backward #85

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Create empty iOS app project.
2. Checkout google-api-objectivec-client from SVN, and add to project:
svn checkout http://google-api-objectivec-client.googlecode.com/svn/trunk/ 
google-api-objectivec-client-read-only 
3. In AppDelegate:

static NSString* const GOOGLE_CLIENT_ID     = <GOOGLE_CLIENT_ID>;
static NSString* const GOOGLE_CLIENT_SECRET = <GOOGLE_CLIENT_SECRET>;

@implementation AppDelegate

- (BOOL)application:(UIApplication *)application 
didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{
    // Do later (after root view controller constructed).
    dispatch_async(dispatch_get_main_queue(), ^{
        // Show view controller for Google login.
        GTMOAuth2ViewControllerTouch* newViewController = [[GTMOAuth2ViewControllerTouch alloc] initWithScope:kGTLAuthScopeDriveFile clientID:GOOGLE_CLIENT_ID clientSecret:GOOGLE_CLIENT_SECRET keychainItemName:nil completionHandler:^(GTMOAuth2ViewControllerTouch *viewController, GTMOAuth2Authentication *auth, NSError *error) {
            // Handle error.
            if(error)
            {
                NSLog(@"What?! %@", error);
                return;
            }

            // Hide view controller.
            [viewController dismissViewControllerAnimated:true completion:nil];

            // Init Google Drive Service.
            GTLServiceDrive* googleDriveService = [[GTLServiceDrive alloc] init];
            googleDriveService.authorizer = auth;

            // Prepare file for upload.
            GTLDriveFile* googleDriveFile = [GTLDriveFile object];
            googleDriveFile.title = @"testUploadFile";
            googleDriveFile.mimeType = @"text/plain";

            char fileDataBuffer[2000000];
            memset(fileDataBuffer, 'a', sizeof(fileDataBuffer));
            NSData* fileData = [NSData dataWithBytes:fileDataBuffer length:sizeof(fileDataBuffer)];
            GTLUploadParameters* uploadPara = [GTLUploadParameters uploadParametersWithData:fileData MIMEType:googleDriveFile.mimeType];
            GTLQueryDrive* queryObj = [GTLQueryDrive queryForFilesInsertWithObject:googleDriveFile uploadParameters:uploadPara];

            // Upload to Google Drive.
            GTLServiceTicket* serviceTicket = [googleDriveService executeQuery:queryObj completionHandler:^(GTLServiceTicket *ticket, GTLDriveFile *insertedFile, NSError *error) {
                NSLog(@"Upload completed");
            }];
            if(!serviceTicket)
            {
                NSLog(@"What?!");
                return;
            }

            // Set upload progress callback.
            __block unsigned long long lastTotalBytesWritten = 0;
            [serviceTicket setUploadProgressBlock:^(GTLServiceTicket *ticket, unsigned long long totalBytesWritten, unsigned long long totalBytesExpectedToWrite) {
                NSLog(@"totalBytesWritten=%llu totalBytesExpectedToWrite=%llu", totalBytesWritten, totalBytesExpectedToWrite);
                if(totalBytesWritten < lastTotalBytesWritten)
                {
                    NSLog(@"totalBytesWritten DECREASED?");
                }
                lastTotalBytesWritten = totalBytesWritten;
            }];
        }];
        [self.window.rootViewController presentViewController:newViewController animated:true completion:nil];
    });

    return YES;
}

@end

4. What the code does:
   a. Login to Google.
   b. Upload 2MB of data to Google Drive.
   c. Print progress in UploadProgressBlock. Notice that in UploadProgressBlock, it checks if "totalBytesWritten" decreases (normally it should only increase as the data is uploaded).
4. Run the app, and login to Google. It uploads the data to Google Drive.
5. Read the console output. Notice that "totalBytesWritten" has actually 
decreased.

This is a sample output (simplified for clarity):
    totalBytesWritten=152 totalBytesExpectedToWrite=2000152
    totalBytesWritten=4248 totalBytesExpectedToWrite=2000152
    totalBytesWritten=8344 totalBytesExpectedToWrite=2000152
    totalBytesWritten=12440 totalBytesExpectedToWrite=2000152
    totalBytesWritten=16536 totalBytesExpectedToWrite=2000152
    totalBytesWritten=20632 totalBytesExpectedToWrite=2000152
    totalBytesWritten=24728 totalBytesExpectedToWrite=2000152
    <...>
    totalBytesWritten=995480 totalBytesExpectedToWrite=2000152
    totalBytesWritten=999576 totalBytesExpectedToWrite=2000152
    totalBytesWritten=1000152 totalBytesExpectedToWrite=2000152
    totalBytesWritten=790680 totalBytesExpectedToWrite=2000152
    totalBytesWritten DECREASED?
    totalBytesWritten=794776 totalBytesExpectedToWrite=2000152
    totalBytesWritten=798872 totalBytesExpectedToWrite=2000152
    totalBytesWritten=802968 totalBytesExpectedToWrite=2000152
    totalBytesWritten=807064 totalBytesExpectedToWrite=2000152
    <...>
    totalBytesWritten=1777232 totalBytesExpectedToWrite=2000152
    totalBytesWritten=1781328 totalBytesExpectedToWrite=2000152
    totalBytesWritten=1785424 totalBytesExpectedToWrite=2000152
    totalBytesWritten=1786584 totalBytesExpectedToWrite=2000152
    totalBytesWritten=1577112 totalBytesExpectedToWrite=2000152
    totalBytesWritten DECREASED?
    totalBytesWritten=1581208 totalBytesExpectedToWrite=2000152
    totalBytesWritten=1585304 totalBytesExpectedToWrite=2000152
    totalBytesWritten=1589400 totalBytesExpectedToWrite=2000152
    totalBytesWritten=1593496 totalBytesExpectedToWrite=2000152
    totalBytesWritten=1597592 totalBytesExpectedToWrite=2000152
    <...>
    totalBytesWritten=1990808 totalBytesExpectedToWrite=2000152
    totalBytesWritten=1994904 totalBytesExpectedToWrite=2000152
    totalBytesWritten=1999000 totalBytesExpectedToWrite=2000152
    totalBytesWritten=2000152 totalBytesExpectedToWrite=2000152
    Upload completed

What is the expected output? What do you see instead?
  Expected result:
    "totalBytesWritten" should not decrease, since it is the number of bytes uploaded so far.
  Actual result:
    "totalBytesWritten" decreases.

What version of the product are you using? On what operating system?
* google-api-objectivec-client revision 426
Xcode Version 6.1.1 (6A2008a)
OS X Yosemite Version 10.10.2 (14C109)
iOS 8, simulator or device

Please provide any additional information below.
After investigation, I found that:
* GTMHTTPUploadFetcher is uploading the data in chunks.
* After the first chunk is uploaded, GTMHTTPUploadFetcher.currentOffset_ 
changes from 0 to 786432, and totalBytesWritten is decreased (near line 793 of 
GTMHTTPUploadFetcher.m).
* 786432 comes from line 706 of GTMHTTPUploadFetcher.m, which comes from 
"rangeStr" being
    Range = "bytes=0-786431"
  which happens when the HTTP response status code is 308, i.e., "resume incomplete"
* The problem is, the upload chunk size is actually 1000000 (line 2332 of 
GTLService.m).
* That means the upload chunk size (1000000) and the actual uploaded chunk size 
(786432) are different.
* Interestingly, 786432 equals 1024 * 768.
* Upon further investigation, I found in 
https://developers.google.com/drive/web/manage-uploads#resumable that:
>>>> Chunk size restriction: All chunks must be a multiple of 256 KB (256 x 
1024 bytes) in size, except for the final chunk that completes the upload. If 
you use chunking, it is important to keep the chunk size as large as possible 
to keep the upload efficient.

So I think the problem is that all those values in -setServiceUploadChunkSize: 
of GTLService are not multiples of 256K, causing chunks in resumable uploads to 
go back.

Attached is the sample Xcode project.

Original issue reported on code.google.com by forcodep...@gmail.com on 5 Mar 2015 at 8:34

Attachments:

GoogleCodeExporter commented 9 years ago
This expected behavior in the sense that the server may always ask the client 
to resend all or part of the upload, hence "bytes sent so far" can fall back as 
a chunk is resent in part or in full. 

So the app should not expect the bytes sent total to increase monotonically. 
Whether or not the app should display that progress regression to the user is a 
user experience decision.

But you're right that the logic for setting the chunk size is out of date and 
should be improved; thanks for raising the issue.

Original comment by grobb...@google.com on 7 Mar 2015 at 1:26

GoogleCodeExporter commented 9 years ago
Seems fixed in revision 428.
Tested ok for me.

Great work.

Original comment by forcodep...@gmail.com on 13 Apr 2015 at 9:49

GoogleCodeExporter commented 9 years ago

Original comment by grobb...@google.com on 1 May 2015 at 12:47