-
Notifications
You must be signed in to change notification settings - Fork 507
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use a key-based mutex lock to download resources/agent bins once. #13215
Merged
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
wallyworld
approved these changes
Aug 5, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, we can add a unit test for the simultaneous fetching. I guess we can use a gomock and spin up say 100 goroutines and for the actual fetch api call to the backend repo, EXPECT() at most 1 call.
hpidcock
force-pushed
the
download-resource-once
branch
2 times, most recently
from
August 6, 2021 01:39
d7c6306
to
9e9e4c1
Compare
hpidcock
force-pushed
the
download-resource-once
branch
from
August 6, 2021 03:20
9e9e4c1
to
e309e88
Compare
|
Merged
jujubot
added a commit
that referenced
this pull request
Aug 8, 2021
#13220 Merge 2.9 #13211 Fix data race (accessing charmInfo) in TestV2CharmExitsApplicationWorker #13212 Fix retry-provisioning command #13210 Add note and todo #13209 Improve readablity of 'no prdesc not found' error message #13214 Adds remaining authorization objects to ignore #13215 Use a key-based mutex lock to download resources/agent bins once #13218 Show application data set by the remote app in show-unit output #13219 Add impish to the ubuntu series list Trivial conflicts in imports/code comments. ``` Conflicts: # apiserver/facades/client/client/client.go # apiserver/facades/schema.json # resource/resourceadapters/opener.go ``` ## QA steps See PRs
jujubot
added a commit
that referenced
this pull request
Jan 25, 2022
#13650 This follows on from the work done in #13215 which throttled resource downloads from the store. Here we introduce limits to resource downloads from the controller to the workload nodes (initiated by the unit agents calling `resource-get`). There are 2 new controller config attributes: - controller-resource-download-limit - application-resource-download-limit The first limits the number of total downloads for any app hosted in a model on the controller. The latter limits the number of downloads per application per model. The default values for these limits are 0, meaning no throttling; the new behaviour is opt in. ## QA steps I deployed the `dummy-resource` charm with slightly modified hooks; the line `status-set maintenance $(cat $RES_PATH)` was commented out. First create a number of machines to run the charm juju bootstrap juju controller-config controller-resource-download-limit=5 juju add-machine -n 20 Optionally turn on debug logging for `juju.resource.resourceadapters` juju model-config logging-config="juju.resource.resourceadapters=DEBUG" Let the machines start. watch -c juju status --color Then deploy the charm twice with different app name, using a large zip file juju deploy /path/to/dummy-resource -n 10 --resource dummy=./dummy-resource.zip --to 1,2,3,4,5,6,7,8,9,10 juju deploy /path/to/dummy-resource -n 10 app2 --resource dummy=./dummy-resource.zip --to 11,12,13,14,15,16,17,18,19,20 status will show the message for each unit change to indicate the resource is delivered. Typically a few change at once. debug-log will show the resource download lock being acquired in batches. ssh into the machines and check the zip file is present in the unit resources directory. ## Bug reference https://bugs.launchpad.net/juju/+bug/1940219
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.
QA steps
juju bootstrap localhost
juju model-config -m controller logging-config="<root>=TRACE"
juju deploy test-resource-pull --channel=beta -n 10
juju run-action test-resource-pull/0 test-resource-pull/1 test-resource-pull/2 test-resource-pull/3 test-resource-pull/4 test-resource-pull/5 test-resource-pull/6 test-resource-pull/7 test-resource-pull/8 test-resource-pull/9 pull
juju debug-log -m controller --replay | grep GetResource
Documentation changes
N/A
Bug reference
https://bugs.launchpad.net/juju/+bug/1905703