Verify resumable upload logic

[shaileshkm]

Hi,

<Edited the question to remove unrelated content>
Can someone explain resumable upload concepts?

1. Is it correct/necessary to obtain session id after each write operation? I found a comment in header file saying 'might change during upload'.
2. Except for last data packet, I will ensure write size is in multiple of 256K.
3. Is invocation to write atomic? In other word, could it write partial data but set an error on stream?
4. What is the significance of doing 'Suspend' after detecting error on stream?
5. Instead of 'Suspend', could I do 'Close'? Which is correct?
6. I'm assuming _next_expected_byte is 0 based. Is that correct?

Thanks for your help.

[coryan]

• Is it correct/necessary to obtain session id after each write operation? I found a comment in header file saying 'might change during upload'.

That was a mistake, and should not appear in newer versions. If it does, let me know where.

• Except for last data packet, I will ensure write size is in multiple of 256K.

That is more efficient, but the client library does that for you.

• Is invocation to write atomic? In other word, could it write partial data but set an error on stream?

I do not believe there is a guarantee that the full data is committed or not, that is, the client library may successfully transmit 512KiB, and then get told that only 256KiB got written. If something like that happens, the client library will resend the remaining 256KiB until it succeeds or the retry policy is exhausted.

• What is the significance of doing 'Suspend' after detecting error on stream?

I do not know what is your intent, but calling .Suspend() ensures the upload is not finalized. Potentially you could call .WriteObject() with the same upload id and resume the upload from whatever the new "next expected byte" is at that time.

Note, however, that the library will return an error only after multiple retries. You control how many retries (by setting the RetryPolicy when you create the client). It makes sense to .Suspend() if you are going to shutdown the application (after saving the upload_id) or maybe if you will move the upload to another machine. But just if all you will do is retry in a few seconds or minutes: why not set the RetryPolicy to a higher value?

• Instead of 'Suspend', could I do 'Close'? Which is correct?

If you do a .Close() the upload is finalized, you cannot write more data, even if you have the upload id and start a new session.

• I'm assuming _next_expected_byte is 0 based. Is that correct?

Yes.

[shaileshkm]

Thanks for the response.

• Is it correct/necessary to obtain session id after each write operation? I found a comment in header file saying 'might change during upload'.

That was a mistake, and should not appear in newer versions. If it does, let me know where.

Is it sufficient to get session id immediately after creating a write stream and hold on to it till it is closed?

• Is invocation to write atomic? In other word, could it write partial data but set an error on stream?

I do not believe there is a guarantee that the full data is committed or not, that is, the client library may successfully transmit 512KiB, and then get told that only 256KiB got written. If something like that happens, the client library will resend the remaining 256KiB until it succeeds or the retry policy is exhausted.

Ok. The way I understand it is (at least for the case when data size is in multiple of 256K), if write returns success, it guarantees all the data was written. If write returns error, there is no guarantee how much of the data was written or any at all. In such case, I need to use next_expected_byte() to figure out how much was written, if I want to resume.

• What is the significance of doing 'Suspend' after detecting error on stream?

I do not know what is your intent, but calling .Suspend() ensures the upload is not finalized. Potentially you could call .WriteObject() with the same upload id and resume the upload from whatever the new "next expected byte" is at that time.

Note, however, that the library will return an error only after multiple retries. You control how many retries (by setting the RetryPolicy when you create the client). It makes sense to .Suspend() if you are going to shutdown the application (after saving the upload_id) or maybe if you will move the upload to another machine. But just if all you will do is retry in a few seconds or minutes: why not set the RetryPolicy to a higher value?

I wasn't aware the client library does the exponential backoff itself. The user documentation does not talk about it. In fact with this link (https://cloud.google.com/iot/docs/how-tos/exponential-backoff) and the sample code, I got an impression that caller must manage it using Suspend, Open with RestoreResumableUploadSession.
Thanks for mentioning RetryPolicy.
I found good explanation in storage/doc/storage-main.dox on default behavior.
It makes good sense for client library to manage it and makes my life easier. Thanks.

Do you have a code sample for how to create a retry policy and feed it to Client?
I checked couple of unit tests.

new Client{
        std::shared_ptr<internal::RawClient>(mock_),
        LimitedErrorCountRetryPolicy(2),
        ExponentialBackoffPolicy(std::chrono::milliseconds(1),
                                 std::chrono::milliseconds(1), 2.0)}

The argument given seems like first backoff period, max backoff period, backoff scaling. Is that correct?
Is following a correct way to construct a client?

_client_wrapper = gcs::Client(gcs::ClientOptions(*creds), 
                              ExponentialBackoffPolicy(std::chrono::milliseconds(100),
                                                       std::chrono::seconds(60), 2.0)
                              );

In addition, if I want to control max retry period, would following work?
Sorry, if this looks naive to you.

_client_wrapper = gcs::Client(gcs::ClientOptions(*creds), 
                              LimitedTimeRetryPolicy(std::chrono::minutes(5)),
                              ExponentialBackoffPolicy(std::chrono::milliseconds(100),
                                                       std::chrono::seconds(60), 2.0)
                              );

• Instead of 'Suspend', could I do 'Close'? Which is correct?

If you do a .Close() the upload is finalized, you cannot write more data, even if you have the upload id and start a new session.

Understood.

Thanks for your help.

[coryan]

Thanks for the response.

No problem.

That was a mistake, and should not appear in newer versions. If it does, let me know where.

Is it sufficient to get session id immediately after creating a write stream and hold on to it till it is closed?

Correct.

Ok. The way I understand it is (at least for the case when data size is in multiple of 256K), if write returns success, it guarantees all the data was written. If write returns error, there is no guarantee how much of the data was written or any at all. In such case, I need to use next_expected_byte() to figure out how much was written, if I want to resume.

Yes, with one caveat: note that next_expected_byte() is not reliable If there is an error. You want to resume and use the next_expected_byte() returned when you resume the upload.

I wasn't aware the client library does the exponential backoff itself. The user documentation does not talk about it.
In fact with this link (https://cloud.google.com/iot/docs/how-tos/exponential-backoff) and the sample code,

Sadly, this is showing some of the holes in our client library coverage. In general, our client libraries implement retry and backoff. But some services do not have (yet) client libraries. In particular IoT does not, and this example is just using raw protocol-level APIs.

The list of services with a C++ client library can be found in the README.md for this repository.

I got an impression that caller must manage it using Suspend, Open with RestoreResumableUploadSession.

I see what I can do about the IoT documentation. Meanwhile, this is a better source for the GCS C++ client library:

https://googleapis.dev/cpp/google-cloud-storage/latest/

And this is better for GCS client libraries in general:

https://cloud.google.com/storage/docs/reference/libraries#client-libraries-install-cpp

Thanks for mentioning RetryPolicy.
I found good explanation in storage/doc/storage-main.dox on default behavior.
It makes good sense for client library to manage it and makes my life easier. Thanks.

Yay! Thank you for saying so.

Do you have a code sample for how to create a retry policy and feed it to Client?

There is one in, reproduced below

https://googleapis.dev/cpp/google-cloud-storage/latest/

  namespace gcs = google::cloud::storage;
  using ::google::cloud::StatusOr;
  [](std::string const& bucket_name, std::string const& object_name,
     std::string const& contents) {
    // Create a client that only gives up on the third error. The default policy
    // is to retry for several minutes.
    StatusOr<gcs::ClientOptions> options =
        gcs::ClientOptions::CreateDefaultClientOptions();
 
    if (!options) throw std::runtime_error(options.status().message());
    gcs::Client client{*options, gcs::LimitedErrorCountRetryPolicy(3)};
 
    StatusOr<gcs::ObjectMetadata> object_metadata =
        client.InsertObject(bucket_name, object_name, std::move(contents),
                            gcs::IfGenerationMatch(0));
 
    if (!object_metadata) {
      throw std::runtime_error(object_metadata.status().message());
    }
 
    std::cout << "The object " << object_metadata->name()
              << " was created in bucket " << object_metadata->bucket()
              << "\nFull metadata: " << *object_metadata << "\n";
  }

I checked couple of unit tests.

Good idea, but generally the google/cloud/storage/examples/ subdirectory may be a better source to look for sample code.

The argument given seems like first backoff period, max backoff period, backoff scaling. Is that correct?
Is following a correct way to construct a client?

_client_wrapper = gcs::Client(gcs::ClientOptions(*creds), 
                              ExponentialBackoffPolicy(std::chrono::milliseconds(100),
                                                       std::chrono::seconds(60), 2.0)
                              );

That controls the backoff policy, that is, how long to wait between retries. It does not set anything about the number of retries or how long to keep retrying. In this case you would be waiting 100ms, then 200ms, then 400ms, and the waiting period will stop growing at 60s. In general, for GCS, we recommend a minimum backoff period of 1s, it is unlikely that things will recover in 100ms, or 200ms.

In addition, if I want to control max retry period, would following work?
Sorry, if this looks naive to you.

_client_wrapper = gcs::Client(gcs::ClientOptions(*creds), 
                              LimitedTimeRetryPolicy(std::chrono::minutes(5)),
                              ExponentialBackoffPolicy(std::chrono::milliseconds(100),
                                                       std::chrono::seconds(60), 2.0)
                              );

That looks correct, yes. This will keep retrying for 5m.

[shaileshkm]

Thanks Carols.
I appreciate your timely responses.