NuGet / NuGetGallery

NuGet Gallery is a package repository that powers https://www.nuget.org. Use this repo for reporting NuGet.org issues.
https://www.nuget.org/
Apache License 2.0
1.55k stars 641 forks source link

[Orchestrator] The Orchestrator may delete available packages #6579

Open loic-sharma opened 6 years ago

loic-sharma commented 6 years ago

Problem

When the package is made available for the first time and the database update fails, V2 will be in an inconsistent state: the package will exist in the public packages container but won't be marked as available in the database. The Orchestrator tries to fix this up by deleting the package if the database commit fails (see MarkPackageAsAvailableAsync). The Orchestrator may do this incorrectly on a package that should be available.

MarkPackageAsAvailableAsync only deletes a package if it believes it copied the package for the first time. This is fine, except, the Orchestrator sometimes believes it copied the package when it hasn't. This happens because CopyFileAsync may not copy anything at all if the source and destination blobs are identical (see this).

This can cause the Orchestrator to delete a package that is available in the following scenario:

  1. A new package is uploaded
  2. A new validation set is created to validate the package
  3. The validation set's message is duplicated
  4. All validations complete successfully
  5. Message 1 copies the package to the packages container
  6. Message 2 attempts to copy the package to the packages container. It sees a package with the same hash is already present, so it does not copy anything. Message 2 believes it has copied the package to the public location
  7. Message 1 marks the package as available and consumes its message
  8. Message 2 attempts to mark the package as available. Committing the transaction fails, so message 2 deletes the package from the public container

Fix

The Orchestrator shouldn't delete the package from the public container if the copy operation was skipped due to the source/destination hash matching.

skofman1 commented 6 years ago

Never happened. Low priority.