[Hotfix] Allow unrelated entity changes to be committed in PackageDeprecationService #9951
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Mitigation for #9950 in the package deprecation path.
Hotfix justification: deprecation API calls that would otherwise no-op sometimes return HTTP 500. Since this is a private preview API, there is no data correctness impact, and non-no-op deprecations work just fine, we could choose to not do a hotfix. I would just to prefer to get this fixed ASAP to eliminate a source of HTTP 500s.
Currently, if a deprecation call no-ops all package version and the node handling the deprecation has a different download count cached for a package version than is what in the DB, an HTTP 500 occurs.
This is because the package entity has changes (per the issue referenced above) but the deprecation service does not detect any changes on the entity.
IPackageUpdateService.UpdatePackagesAsyncis called with an empty list and throws an argument exception which is causing the HTTP 500. We have seen this about 300 times on the deprecation API in the past week, all of which are first party packages or are related to DefinitelyTyped/NugetAutomation#21.This was not detected locally because storage-based statistics are not used. This was not detected on DEV or INT because the package used for testing had the same download count in DB and in the downloads.v1.json file therefore the
DownloadCountObjectMaterializedInterceptordid not introduce any entity changes.