SubscriptionProcessorJob perf updates #480
Conversation
agr
requested review from
loic-sharma,
xavierdecoster,
shishirx34,
ryuyu,
chenriksson,
cristinamanum,
skofman1 and
scottbommarito
| @@ -8,7 +8,8 @@ | |||
| "ServiceBus": { | |||
| "ConnectionString": "Endpoint=sb://nugetdev.servicebus.windows.net/;SharedAccessKeyName=extract-and-validate-signature;SharedAccessKey=$$Dev-ServiceBus-SharedAccessKey-Validation-ExtractAndValidatePackageSignature$$", | |||
| "TopicPath": "validate-signature", | |||
| "SubscriptionName": "extract-and-validate-signature" | |||
| "SubscriptionName": "extract-and-validate-signature", | |||
There was a problem hiding this comment.
The deployment settings should be updated as well.
There was a problem hiding this comment.
Talked offline. No changes needed.
| } | ||
|
|
||
| processor.Start(); | ||
| processor.Start(configuration.Value.MaxConcurrentCalls); |
There was a problem hiding this comment.
Why not trow and use a default when the setting is not present?
There was a problem hiding this comment.
I wanted to keep the change backwards compatible: if the code that uses it had no changes to opt-in to setting the MaxConcurrentCalls, it would continue working as if no change was made at all. This PR does not include changes to the Orchestrator as I want to test it separately.
All the jobs descending from the SubcriptionProcessorJob<T> are pretty much required not to use the default setting, so I made it impossible to not have it set.
| @@ -8,7 +8,8 @@ | |||
| "ServiceBus": { | |||
| "ConnectionString": "Endpoint=sb://nugetdev.servicebus.windows.net/;SharedAccessKeyName=extract-and-validate-signature;SharedAccessKey=$$Dev-ServiceBus-SharedAccessKey-Validation-ExtractAndValidatePackageSignature$$", | |||
| "TopicPath": "validate-signature", | |||
| "SubscriptionName": "extract-and-validate-signature" | |||
| "SubscriptionName": "extract-and-validate-signature", | |||
| "MaxConcurrentCalls": 10 | |||
There was a problem hiding this comment.
int/prod.json coming in separate PR?
There was a problem hiding this comment.
Added all as the change would break int/prod unless the configuration is updated.
* MaxConcurrentCalls update. * Test and configuration update * Updated int and prod settings for the ProcessSignature and ValidateCertificate jobs.
Progress on https://github.com/NuGet/Engineering/issues/1481.
Bringing in latest changes in NuGet.Server.Common related to ServiceBus multi-threading. For now changes don't affect orchestrator, but only the jobs downstream.
This change enables the ScanAndSign job update, which we can use for testing the change (since it runs in allowed to fail mode) beyond what we can do in dev environment. After we verify ScanAndSign stability, we'll update the Orchestrator itself.