Skip to content
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.

Sign up for GitHub

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

[JUJU-485] Add per controller and per app limits for downloading resources #13650

Merged
merged 2 commits into from
Jan 25, 2022
Merged

[JUJU-485] Add per controller and per app limits for downloading resources #13650

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a388a1f
Add per controller and per app limits for downloading resources
wallyworld Jan 25, 2022
1676091
Code review fixes
wallyworld Jan 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Code review fixes
  • Loading branch information
@wallyworld
wallyworld committed Jan 25, 2022
commit 16760911ff1024e9e649f0e022b672528e09eae0
7 changes: 2 additions & 5 deletions resource/repositories/operations_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,10 @@ import (
"github.com/golang/mock/gomock"
"github.com/juju/charm/v8"
charmresource "github.com/juju/charm/v8/resource"
"github.com/juju/errors"
jc "github.com/juju/testing/checkers"
gc "gopkg.in/check.v1"

wallyworld marked this conversation as resolved.
Show resolved Hide resolved
"github.com/juju/errors"

"github.com/juju/juju/charmstore"
"github.com/juju/juju/resource"
"github.com/juju/juju/resource/repositories"
Expand Down Expand Up @@ -117,9 +116,7 @@ func (s *OperationsSuite) TestConcurrentGetResource(c *gc.C) {
Repository: er,
Name: "company-icon",
CharmID: repositories.CharmID{URL: charm.MustParseURL("cs:gitlab")},
Done: func() {
done.Done()
},
Done: done.Done,
}

start := sync.WaitGroup{}
Expand Down
9 changes: 5 additions & 4 deletions resource/resourceadapters/limiter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package resourceadapters

import (
"sync"
"time"
)

// ResourceDownloadLock is used to limit the number of concurrent
Expand Down Expand Up @@ -43,8 +44,9 @@ type resourceDownloadLimiter struct {
// Acquire implements ResourceDownloadLock.
func (r *resourceDownloadLimiter) Acquire(appName string) {
if r.globalLock != nil {
logger.Debugf("acquire global resource download lock, current downloads = %d", len(r.globalLock))
start := time.Now()
r.globalLock <- struct{}{}
logger.Debugf("acquire global resource download lock for %q, took %dms", appName, time.Now().Sub(start)/time.Millisecond)
}
if r.applicationLimit <= 0 {
return
Expand All @@ -57,15 +59,15 @@ func (r *resourceDownloadLimiter) Acquire(appName string) {
r.applicationLocks[appName] = lock
}
r.mu.Unlock()
logger.Debugf("acquire application resource download lock, current downloads = %d", len(lock))
start := time.Now()
lock <- struct{}{}
logger.Debugf("acquire app resource download lock for %q, took %dms", appName, time.Now().Sub(start)/time.Millisecond)
}

// Release implements ResourceDownloadLock.
func (r *resourceDownloadLimiter) Release(appName string) {
if r.globalLock != nil {
<-r.globalLock
logger.Debugf("release global resource download lock, current downloads = %d", len(r.globalLock))
}
if r.applicationLimit <= 0 {
return
Expand All @@ -77,7 +79,6 @@ func (r *resourceDownloadLimiter) Release(appName string) {
if !ok {
return
}
logger.Debugf("release global resource download lock, current downloads = %d", len(lock))
<-lock
if len(lock) == 0 {
delete(r.applicationLocks, appName)
Expand Down
16 changes: 16 additions & 0 deletions resource/resourceadapters/limiter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,21 @@ func (s *LimiterSuite) TestNoLimits(c *gc.C) {

totalAcquiredCount := int32(0)
trigger := make(chan struct{})
started := sync.WaitGroup{}
finished := sync.WaitGroup{}
for i := 0; i < totalToAcquire; i++ {
started.Add(1)
finished.Add(1)
go func() {
defer finished.Done()
started.Done()
limiter.Acquire("app1")
wallyworld marked this conversation as resolved.
Show resolved Hide resolved
atomic.AddInt32(&totalAcquiredCount, 1)
<-trigger
limiter.Release("app1")
}()
}
started.Wait()

done := make(chan bool)
go func() {
Expand Down Expand Up @@ -88,17 +92,21 @@ func (s *LimiterSuite) TestGlobalLimit(c *gc.C) {

totalAcquiredCount := int32(0)
trigger := make(chan struct{})
started := sync.WaitGroup{}
finished := sync.WaitGroup{}
for i := 0; i < totalToAcquire; i++ {
started.Add(1)
finished.Add(1)
go func() {
defer finished.Done()
started.Done()
limiter.Acquire("app1")
wallyworld marked this conversation as resolved.
Show resolved Hide resolved
atomic.AddInt32(&totalAcquiredCount, 1)
<-trigger
limiter.Release("app1")
}()
}
started.Wait()

done := make(chan bool)
go func() {
Expand Down Expand Up @@ -152,21 +160,25 @@ func (s *LimiterSuite) TestApplicationLimit(c *gc.C) {

totalAcquiredCount := int32(0)
trigger := make(chan struct{})
started := sync.WaitGroup{}
finished := sync.WaitGroup{}
for i := 0; i < numApplications*totalToAcquirePerApplication; i++ {
started.Add(1)
finished.Add(1)
uuid := "app1"
if i >= totalToAcquirePerApplication {
uuid = "app2"
}
go func(uui string) {
defer finished.Done()
started.Done()
limiter.Acquire(uuid)
wallyworld marked this conversation as resolved.
Show resolved Hide resolved
atomic.AddInt32(&totalAcquiredCount, 1)
<-trigger
limiter.Release(uuid)
}(uuid)
}
started.Wait()

done := make(chan bool)
go func() {
Expand Down Expand Up @@ -222,8 +234,10 @@ func (s *LimiterSuite) TestGlobalAndApplicationLimit(c *gc.C) {

totalAcquiredCount := int32(0)
trigger := make(chan struct{})
started := sync.WaitGroup{}
finished := sync.WaitGroup{}
for i := 0; i < numApplications*totalToAcquirePerApplication; i++ {
started.Add(1)
finished.Add(1)
uuid := "app1"
if i >= 2*totalToAcquirePerApplication {
Expand All @@ -233,12 +247,14 @@ func (s *LimiterSuite) TestGlobalAndApplicationLimit(c *gc.C) {
}
go func(uui string) {
defer finished.Done()
started.Done()
limiter.Acquire(uuid)
atomic.AddInt32(&totalAcquiredCount, 1)
<-trigger
limiter.Release(uuid)
}(uuid)
}
started.Wait()

done := make(chan bool)
go func() {
Expand Down