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

google cloud storage: NPE from BlobId.java:119 #709

Closed
egh opened this issue Feb 12, 2021 · 6 comments · Fixed by #726
Closed

google cloud storage: NPE from BlobId.java:119 #709

egh opened this issue Feb 12, 2021 · 6 comments · Fixed by #726
Assignees
Labels
api: storage Issues related to the googleapis/java-storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@egh
Copy link

egh commented Feb 12, 2021

Environment details

  1. google cloud storage
  2. OS type and version: Ubuntu 18.04.4, running in docker container on k8s.
  3. Java version:
    java version "1.8.0_241"
    Java(TM) SE Runtime Environment (build 1.8.0_241-b07)
    Java HotSpot(TM) 64-Bit Server VM (build 25.241-b07, mixed mode)
  4. storage version(s):
    google-cloud-storage-1.113.9.jar
    google-cloud-core-1.94.0.jar
    google-cloud-core-http-1.94.0.jar

Steps to reproduce

  1. Upload content per code below

Code example

    BlobId blobId = BlobId.of(bucketName, key);
    String md5;
    try (InputStream inputStream = getInputStream()) {
      md5 = Base64.encodeBase64String(DigestUtils.md5(inputStream));
    }
    BlobInfo blobInfo = BlobInfo.newBuilder(blobId).setContentType(getContentType())
      .setContentDisposition(String.format("attachment; filename=%s", getDownloadName()))
      .setMd5(md5)
      .build();
    storage.createFrom(blobInfo, getInputStream(), Storage.BlobWriteOption.md5Match());

Stack trace

Caused by: java.lang.NullPointerException
    at com.google.cloud.storage.BlobId.fromPb(BlobId.java:119)
    at com.google.cloud.storage.BlobInfo.fromPb(BlobInfo.java:1160)
    at com.google.cloud.storage.Blob.fromPb(Blob.java:958)
    at com.google.cloud.storage.StorageImpl.createFrom(StorageImpl.java:267)
    at com.google.cloud.storage.StorageImpl.createFrom(StorageImpl.java:253)
    at org.ambraproject.rhino.service.impl.GCSObjectStorageServiceImpl.storeFile(GCSObjectStorageServiceImpl.java:72)
    at org.ambraproject.rhino.service.impl.GCSObjectStorageServiceImpl.lambda$null$0(GCSObjectStorageServiceImpl.java:78)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)

This is an intermittent issue - I’ve only seen it once, as far as I know. I can’t see how the NPE could be coming from our code. I can’t seem to find any similar issue in the issues list or online.

Thanks for any help you can provide!

@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/java-storage API. label Feb 12, 2021
@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Feb 13, 2021
@tritone tritone added status: investigating The issue is under investigation, which is determined to be non-trivial. and removed triage me I really want to be triaged. labels Feb 16, 2021
@tritone tritone self-assigned this Feb 16, 2021
@egh
Copy link
Author

egh commented Feb 16, 2021

Please let me know if there is any more information I can provide.

@tritone
Copy link
Contributor

tritone commented Feb 17, 2021

Thanks for your report.

From digging into the stack a bit it looks like the StorageObject proto is null here where the client is trying to construct the object for for the newly created Blob. This seems like a bug; it should not be something that can happen if the upload was successful and I would expect a different error if the upload failed (not NPE).

@frankyn @BenWhitehead any ideas how this could have happened? I know there have been some recent updates to the upload logic.

@egh
Copy link
Author

egh commented Feb 17, 2021

We saw this for a second time yesterday. I have updated the stack trace slightly to reflect that this is coming from a ThreadPoolExecutor. We have found that we need to send content to GCS in parallel in order to get acceptable speeds. I'm not sure if that affects anything.

@frankyn
Copy link
Member

frankyn commented Feb 17, 2021

Hi @egh,

Did you have these issues with 1.113.3 or is your usage relatively new?

I think this is not a concurrency issue. It is most likely due an edge case that was not handled correctly in PR #604.
I'm writing a unit test to confirm, but I think the case is:

  1. Retry is handled.
  2. The object is not saved in Case 4 or 7

@egh
Copy link
Author

egh commented Feb 17, 2021

Hi @frankyn -

We were previously using 1.113.6 - before that we were not using GCS.

We did encounter #666 (com.google.cloud.storage.StorageException: Resumable upload is already complete.) when using that version.

@frankyn
Copy link
Member

frankyn commented Feb 17, 2021

Thanks for clarifying and for filing this issue, @egh. This is a relatively new change which tried to make the library more reliable but introduced a few issues.

One recommendation if you need to get unblocked now is to revert to the earlier version 1.113.4 while we work out this issue.

@tritone tritone added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed status: investigating The issue is under investigation, which is determined to be non-trivial. labels Feb 17, 2021
@frankyn frankyn self-assigned this Feb 17, 2021
@tritone tritone removed their assignment Feb 19, 2021
gcf-merge-on-green bot pushed a commit that referenced this issue Feb 26, 2021
…#726)

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
- [x] Ensure the tests and linter pass
- [x] Code coverage does not decrease (if any source code was changed)
- [x] Appropriate docs were updated (if necessary)


I added three things in this PR:
1. Retries around getting remote offset from GCS
2. Retrying last chunk failures correctly. 
3. Updated documentation for the retry cases


Fixes #709 #687 ☕️
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/java-storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants