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

[BUG] Blob Storage's downloadToFile deletes files after toFuture is called #8716

Closed
3 tasks done
hongee opened this issue Mar 5, 2020 · 7 comments
Closed
3 tasks done
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Service Attention Workflow: This issue is responsible by Azure service team. Storage Storage Service (Queues, Blobs, Files)

Comments

@hongee
Copy link

hongee commented Mar 5, 2020

Describe the bug
After calling downloadToFile (with or without overwrite = true), the operation succeeds, and the file, for a brief moment, is successfully downloaded and persisted on disk. However, if a toFuture subscriber is attached and then subsequently converted to a Scala Future (using either Scala's built in compat, or via Reactor's Scala extensions), a cancel signal is fired, and the handler here fires and deletes the file:

edit - quick update - Mono's toFuture does indeed fire off a cancel: https://github.com/reactor/reactor-core/blob/c6b75f2578ee5f201ac06add2239322cc5b7bfcd/reactor-core/src/main/java/reactor/core/publisher/MonoToCompletableFuture.java#L59, which makes the downloadToFile methods impossible to use if a CompletableFuture is desired. May I suggest loosening the check for deletion? 😄

I'm not very familiar with Reactor, and would be happy to find out if I'm doing something wrong, or if there's a workaround. We are on a Scala codebase that relies heavily on Futures, so not using a Future is not quite an option.

Exception or Stack Trace
N/A

To Reproduce
Steps to reproduce the behavior:
See the following snippet

Code Snippet

blobContainerClient
  .getBlobAsyncClient(myBlobKey)
  .downloadToFile("my/path")
  .onComplete { ...file is fine here... }
  .doFinally { ... eventually `cancel` is fired instead of `complete` ... }
  .toFuture.asScala.map { ... file disappears ... }

Expected behavior
Don't delete my file :(

Screenshots
N/A

Setup (please complete the following information):

  • OS: macOS Catalina
  • IDE : Intellij Ultimate
  • Version of the Library used: 12.4.0

Additional context
Could be related to spring-projects/spring-framework#22952, even though this appears to be fixed?

Information Checklist
Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report

  • Bug Description Added
  • Repro Steps Added
  • Setup information Added
@rickle-msft
Copy link
Contributor

@hongee Thank you for reporting this, and sorry for the delay! I am just now noticing we haven't responded.

We do need to delete files in the case of cancellation because we don't want to leave unwanted/incomplete/corrupted files hanging around on the disk. So our best path forward is to try to understand why Reactor has this behavior when being converted to a Future. @anuchandy, do you have any insight into why Reactor would be doing this? Or if perhaps there is another safe way of cleaning up resources that would permit customers to use Futures more readily?

@rickle-msft rickle-msft self-assigned this Mar 9, 2020
@rickle-msft rickle-msft added Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Storage Storage Service (Queues, Blobs, Files) labels Mar 9, 2020
@triage-new-issues triage-new-issues bot removed the triage label Mar 9, 2020
@hongee
Copy link
Author

hongee commented Mar 10, 2020

Thanks for looking into this! :) Yeah it doesn't seem like wrong behavior - just not sure why toFuture sends a close signal after complete, and why those aren't mutually exclusive. Not familiar enough to say how possible this is, but perhaps a flag can be set if the completion signal is received, and a close coming after complete could be ignored?

Currently we're using a hacky workaround where we pass a symlink of the desired path to the method so at least we don't lose the downloaded file :)

@rickle-msft
Copy link
Contributor

That's a good suggestion. I can investigate that a bit along with trying to understand Reactor's behavior a bit better.

@anuchandy
Copy link
Member

@rickle-msft - I've opened an issue/question reactor/reactor-core#2070 to understand more about this behavior.

@anuchandy
Copy link
Member

The issue is fixed in the reactor-core.

openapi-sdkautomation-test bot pushed a commit to openapi-env-test/azure-sdk-for-java that referenced this issue Mar 24, 2020
Merge branch 'master' of https://github.com/Azure/azure-rest-api-specs into keyvault_multiapi_readme

* 'master' of https://github.com/Azure/azure-rest-api-specs: (101 commits)
  add cli.md for automation (Azure#8411)
  adjust assignment (Azure#8782)
  Remove Microsoft.Backup.Admin 2016-05-01 API version (Azure#8588)
  Updating global setting in PostgreSQL/MySQL readme file (Azure#8777)
  update package name and output folder in readme.typescript.md (Azure#8764)
  add package-2019-12 python define (Azure#8769)
  Fix Parameter Description for validate resource move (Azure#8524)
  Edit pass for GA swagger (Azure#8759)
  Update proxy.json (Azure#8596)
  Model enums that may change in the future as strings (Azure#8760)
  Add api-version 2019-11-01 for resources/subscriptions (Azure#8728)
  regenerated all-api-versions
  PrivateLinkResources for Microsoft.Automation (Azure#8369)
  add cli.md for serialconsole (Azure#8401)
  add cli.md for mariadb (Azure#8466)
  [Computer Vision] Create CV API v3.0-preview (Azure#7402)
  Publish Microsoft.ContainerService api-version 2020-03-01 (Azure#8756)
  Update swagger based on auto-gen process change. (Azure#8766)
  add assignment-bot config (Azure#8716)
  add tag package-2019-12 to batch (Azure#8751)
  ...
openapi-sdkautomation-test bot pushed a commit to openapi-env-test/azure-sdk-for-java that referenced this issue Mar 24, 2020
Merge branch 'master' of https://github.com/Azure/azure-rest-api-specs into keyvault_multiapi_readme

* 'master' of https://github.com/Azure/azure-rest-api-specs: (101 commits)
  add cli.md for automation (Azure#8411)
  adjust assignment (Azure#8782)
  Remove Microsoft.Backup.Admin 2016-05-01 API version (Azure#8588)
  Updating global setting in PostgreSQL/MySQL readme file (Azure#8777)
  update package name and output folder in readme.typescript.md (Azure#8764)
  add package-2019-12 python define (Azure#8769)
  Fix Parameter Description for validate resource move (Azure#8524)
  Edit pass for GA swagger (Azure#8759)
  Update proxy.json (Azure#8596)
  Model enums that may change in the future as strings (Azure#8760)
  Add api-version 2019-11-01 for resources/subscriptions (Azure#8728)
  regenerated all-api-versions
  PrivateLinkResources for Microsoft.Automation (Azure#8369)
  add cli.md for serialconsole (Azure#8401)
  add cli.md for mariadb (Azure#8466)
  [Computer Vision] Create CV API v3.0-preview (Azure#7402)
  Publish Microsoft.ContainerService api-version 2020-03-01 (Azure#8756)
  Update swagger based on auto-gen process change. (Azure#8766)
  add assignment-bot config (Azure#8716)
  add tag package-2019-12 to batch (Azure#8751)
  ...
openapi-sdkautomation-test bot pushed a commit to openapi-env-test/azure-sdk-for-java that referenced this issue Mar 24, 2020
Merge branch 'master' of https://github.com/Azure/azure-rest-api-specs into keyvault_multiapi_readme

* 'master' of https://github.com/Azure/azure-rest-api-specs: (101 commits)
  add cli.md for automation (Azure#8411)
  adjust assignment (Azure#8782)
  Remove Microsoft.Backup.Admin 2016-05-01 API version (Azure#8588)
  Updating global setting in PostgreSQL/MySQL readme file (Azure#8777)
  update package name and output folder in readme.typescript.md (Azure#8764)
  add package-2019-12 python define (Azure#8769)
  Fix Parameter Description for validate resource move (Azure#8524)
  Edit pass for GA swagger (Azure#8759)
  Update proxy.json (Azure#8596)
  Model enums that may change in the future as strings (Azure#8760)
  Add api-version 2019-11-01 for resources/subscriptions (Azure#8728)
  regenerated all-api-versions
  PrivateLinkResources for Microsoft.Automation (Azure#8369)
  add cli.md for serialconsole (Azure#8401)
  add cli.md for mariadb (Azure#8466)
  [Computer Vision] Create CV API v3.0-preview (Azure#7402)
  Publish Microsoft.ContainerService api-version 2020-03-01 (Azure#8756)
  Update swagger based on auto-gen process change. (Azure#8766)
  add assignment-bot config (Azure#8716)
  add tag package-2019-12 to batch (Azure#8751)
  ...
@Petermarcu
Copy link
Member

Can this issue be closed?

@Petermarcu Petermarcu added the Service Attention Workflow: This issue is responsible by Azure service team. label Jul 30, 2020
@ghost
Copy link

ghost commented Jul 30, 2020

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @xgithubtriage.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Service Attention Workflow: This issue is responsible by Azure service team. Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

No branches or pull requests

4 participants