Skip to content
This repository has been archived by the owner on Mar 16, 2021. It is now read-only.

[Package Renames 5] Transfer package popularity #769

Merged
merged 1 commit into from
Apr 23, 2020
Merged

Conversation

loic-sharma
Copy link
Contributor

@loic-sharma loic-sharma commented Apr 16, 2020

This updates auxiliary2azuresearch to push download changes whenever a popularity transfer changes, or, whenever a download change affects a popularity transfer.

Previous changes: #765, #766, #767, and #768

Config changes: https://nuget.visualstudio.com/NuGetMicrosoft/_git/NuGetDeployment/pullrequest/1402?_a=overview

Part of NuGet/NuGetGallery#7898

@loic-sharma loic-sharma marked this pull request as draft April 17, 2020 17:41
@loic-sharma loic-sharma force-pushed the loshar-popxfer-xferrer branch from 653afdb to 49cac2c Compare April 20, 2020 18:01
@loic-sharma loic-sharma marked this pull request as ready for review April 20, 2020 20:56
@zhhyu
Copy link
Contributor

zhhyu commented Apr 22, 2020

I am thinking that there is an edge case, but as I am not sure about the whole picture here, like how Azure Search actually works, so I just list it here:
If the original package is hard deleted, it seem that we will still keep it with the download counts 0, which will be fed to Azure Search, in my understanding. ("affectedPackages" will contain this deleted package Id.) Is this expected?

@joelverhagen
Copy link
Member

If the original package is hard deleted, it seem that we will still keep it with the download counts 0, which will be fed to Azure Search, in my understanding. ("affectedPackages" will contain this deleted package Id.) Is this expected?

Great thing to be thinking about. Given renames are on the package registration level, deletes would only cascade to the rename record if we release the package registration, I think. Given this option is unchecked by default and only available if all versions are deleted, I think it would very rarely occur. This begs the question, if all versions are deleted, should we clear the rename?

@loic-sharma
Copy link
Contributor Author

loic-sharma commented Apr 22, 2020

If the original package is hard deleted, it seem that we will still keep it with the download counts 0....

Yup, you are correct that this will produce a transfer of 0. This behavior is covered by the DownloadTransferrerFacts.UnknownPackagesTransferZeroDownloads test.

we will still keep it with the download counts 0, which will be fed to Azure Search

The search pipeline has a "hidden" V3 resource called the version list, which is used by catalog2azuresearch and auxiliary2azuresearch to know which search documents should be updated. Using this resource, we detect and then skip updates to packages that do not exist. This behavior is covered by SearchIndexActionBuilderFacts.UpdatesSearchDocumentsWithNoVersions.

This begs the question, if all versions are deleted, should we clear the rename?

The popularity transfer feature does little for packages with low download counts. I doubt we will need to clean "dangling" renames on "small" packages. Now say that we delete all versions of a popular package but keep its registration... This seems like a truly exceptional scenario, which is hard to anticipate. We may want to evaluate keeping/removing the popularity transfers on a case-by-case basis.

Copy link
Member

@joelverhagen joelverhagen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the added tests. Looks good.

Update tests

Update comments

Update comments

Update Db2AzureSearchCommandFacts

Update new package registration producer facts

Add test

Add tests for download transferrer

Rename method

Fix test

Don't save popularity transfers for now

Save popularity transfers data file

Initial

More tests

Add tests

Updates

Clean

Fix

Undo change

Undo change

Add asserts

Fix whitespace

Tweak

Bring test to live branch

Start integration tests

Update

Remove old test

Fix build

Clean

Fix whitespace
@loic-sharma loic-sharma changed the base branch from loshar-popxfer-xferrer to dev April 23, 2020 22:49
@loic-sharma loic-sharma merged commit 42d2c4f into dev Apr 23, 2020
@loic-sharma loic-sharma deleted the loshar-popxfer branch April 23, 2020 22:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants