Fix for UpdateIsLatestAsync retry connection string #3632
Conversation
|
|
| @@ -113,12 +113,21 @@ public V2FeedExtendedTests(ITestOutputHelper testOutputHelper) | |||
| "7.0.0-a", "7.0.0-b", "7.0.0", "7.0.1", "7.0.2-abc" | |||
| }; | |||
|
|
|||
| // push all and verify; ~15-20 concurrency conflicts seen in testing | |||
| // first push should not be concurrent to avoid conflict on creation of package registration. | |||
There was a problem hiding this comment.
What happens when there is a conflict on creation of the package registration? Is it an issue we should fix?
There was a problem hiding this comment.
I mainly did this so there wouldn't be lots of first chance exceptions for the conflict when debugging this test. Client does a retry, so I was still seeing the test pass.
I suppose the test could have failed or been flaky if the first push was done concurrently and there were a lot of conflicts which caused the client to exceed retry.
I don't think this is anything to fix here. The conflict I'm trying to reproduce is with UpdateIsLatest, not package registration creation.
There was a problem hiding this comment.
To clarify, I agree that it's out of the scope of this PR, but I think it may be something to look into in the future.
If the same person pushes two separate versions of the same new package at once, there shouldn't be issues--we may want to consider adding some failure logic when the package registration commit fails.
There was a problem hiding this comment.
Fair enough - agree, sounds like another useful functional test to have.
* Revert "UpdateIsLatest concurrent unlist fix (#3695)" This reverts commit 551fd86. * Revert "Fix concurrent push test by disabling search hijacking on feed (#3641)" This reverts commit 00fb3fb. * Revert "IsLatest Fix: wrong connection string passed to retry context (#3632)" This reverts commit 1b6b5b0. * Revert "Fix #2514 - Use optimistic concurrency to prevent multiple UpdateIsLatest calls on same package (#3548)" This reverts commit 27ea9bc.
…7.03.27 * tag 'v2017.03.27': (205 commits) Revert UpdateIsLatest optimistic concurrency changes (NuGet#3707) UpdateIsLatest concurrent unlist fix (NuGet#3695) Change telemetry time to use correct format (NuGet#3690) Fix typo of "publically" (NuGet#3636) Fix regression (NuGet#3667) Add credential to Register and RequestPasswordReset audits (NuGet#3666) Functional test for temp keys (NuGet#3664) Telemetry for temp keys (NuGet#3662) Temp keys implementation (NuGet#3563) (NuGet#3646) Extracting code: single type per file (NuGet#3644) Telemetry for package push (NuGet#3649) Upgrade to NuGet.* v4.0.0 dependencies (NuGet#3643) Fix concurrent push test by disabling search hijacking on feed (NuGet#3641) Fixing Package Description truncation (NuGet#3638) Fix Microsoft Account removal (NuGet#3639) Send e-mail when a new API key is created (NuGet#3634) IsLatest Fix: wrong connection string passed to retry context (NuGet#3632) Update WindowsAzure.Storage to 7.0.0 (NuGet#3633) Depend on signed version of Elmah (NuGet#3609) Move AzureEntityList and TableErrorLog to NuGetGallery.Core (NuGet#3607) ...
Regression from PR #3548, found during dev deployment
EntitiesContext default ctor pulls connection string from web.config (localdb), but we needed cloud configuration.
Updated functional test to push first package non-concurrently to avoid conflict on registration creation, and to spread remaining pushes over 5s period (~6 pkg/second) which is more realistic.