Skip to content

Commit

Permalink
Make retry for list download configurable (#308) (#338)
Browse files Browse the repository at this point in the history 
* DownloadAttempts & DownloadCooldown added to BlockingConfig

* implementation of downloadAttempts & downloadCooldown

* extended NewListCache call

* unit test fix(use old default values)

* documentation of downloadAttempts & downloadCooldown

* linter error(line length)
  • Loading branch information
@kwitsch
kwitsch authored Nov 14, 2021
1 parent bd1886d commit 2f79086
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 30 deletions.
2 changes: 2 additions & 0 deletions config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,8 @@ type BlockingConfig struct {
BlockType string `yaml:"blockType" default:"ZEROIP"`
BlockTTL Duration `yaml:"blockTTL" default:"6h"`
DownloadTimeout Duration `yaml:"downloadTimeout" default:"60s"`
DownloadAttempts int `yaml:"downloadAttempts" default:"3"`
DownloadCooldown Duration `yaml:"downloadCooldown" default:"1s"`
RefreshPeriod Duration `yaml:"refreshPeriod" default:"4h"`
FailStartOnListError bool `yaml:"failStartOnListError" default:"false"`
}
Expand Down
13 changes: 10 additions & 3 deletions docs/configuration.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,16 +345,23 @@ Negative value will deactivate automatically refresh.

Refresh every hour.

### Download timeout
### Download

You can override the default download timeout (**duration format**) of 60 seconds (for each URL) for big lists or slow
internet connection:
You can configure the list download attempts according to your internet connection:

| Parameter | Type | Mandatory | Default value | Description |
| ----------------------------- | --------------- | --------- | ------------------ | ------------------------------------------------- |
| downloadTimeout | duration format | no | 60s | Download attempt timeout |
| downloadAttempts | int | no | 3 | How many download attempts schould be performed |
| downloadCooldown | duration format | no | 1s | Time between the download attempts|

!!! example

```yaml
blocking:
downloadTimeout: 4m
downloadAttempts: 5
downloadCooldown: 10s
```

### Fail on start
Expand Down
28 changes: 16 additions & 12 deletions lists/list_cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,12 @@ type ListCache struct {
groupCaches map[string]cache
lock sync.RWMutex

groupToLinks map[string][]string
refreshPeriod time.Duration
downloadTimeout time.Duration
listType ListCacheType
groupToLinks map[string][]string
refreshPeriod time.Duration
downloadTimeout time.Duration
downloadAttempts int
downloadCooldown time.Duration
listType ListCacheType
}

// Configuration returns current configuration and stats
Expand Down Expand Up @@ -85,15 +87,17 @@ func (b *ListCache) Configuration() (result []string) {

// NewListCache creates new list instance
func NewListCache(t ListCacheType, groupToLinks map[string][]string, refreshPeriod time.Duration,
downloadTimeout time.Duration) (*ListCache, error) {
downloadTimeout time.Duration, downloadAttempts int, downloadCooldown time.Duration) (*ListCache, error) {
groupCaches := make(map[string]cache)

b := &ListCache{
groupToLinks: groupToLinks,
groupCaches: groupCaches,
refreshPeriod: refreshPeriod,
downloadTimeout: downloadTimeout,
listType: t,
groupToLinks: groupToLinks,
groupCaches: groupCaches,
refreshPeriod: refreshPeriod,
downloadTimeout: downloadTimeout,
downloadAttempts: downloadAttempts,
downloadCooldown: downloadCooldown,
listType: t,
}
initError := b.refresh(true)

Expand Down Expand Up @@ -232,7 +236,7 @@ func (b *ListCache) downloadFile(link string) (io.ReadCloser, error) {

attempt := 1

for attempt <= 3 {
for attempt <= b.downloadAttempts {
//nolint:bodyclose
if resp, err = client.Get(link); err == nil {
if resp.StatusCode == http.StatusOK {
Expand All @@ -259,7 +263,7 @@ func (b *ListCache) downloadFile(link string) (io.ReadCloser, error) {
attempt).Warnf("Name resolution err, retrying... %s", dnsErr.Err)
}

time.Sleep(time.Second)
time.Sleep(b.downloadCooldown)
attempt++
}

Expand Down
26 changes: 13 additions & 13 deletions lists/list_cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ var _ = Describe("ListCache", func() {
lists := map[string][]string{
"gr0": {emptyFile.Name()},
}
sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 30*time.Second)
sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 30*time.Second, 3, time.Second)

found, group := sut.Match("", []string{"gr0"})
Expect(found).Should(BeFalse())
Expand All @@ -59,7 +59,7 @@ var _ = Describe("ListCache", func() {
lists := map[string][]string{
"gr1": {emptyFile.Name()},
}
sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 30*time.Second)
sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 30*time.Second, 3, time.Second)

found, group := sut.Match("google.com", []string{"gr1"})
Expect(found).Should(BeFalse())
Expand All @@ -85,7 +85,7 @@ var _ = Describe("ListCache", func() {
"gr1": {s.URL},
}

sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 100*time.Millisecond)
sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 100*time.Millisecond, 3, time.Second)
time.Sleep(time.Second)
found, group := sut.Match("blocked1.com", []string{"gr1"})
Expect(found).Should(BeTrue())
Expand All @@ -111,7 +111,7 @@ var _ = Describe("ListCache", func() {
"gr1": {s.URL, emptyFile.Name()},
}

sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 4*time.Hour, 100*time.Millisecond)
sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 4*time.Hour, 100*time.Millisecond, 3, time.Second)
time.Sleep(time.Second)
By("Lists loaded without timeout", func() {
found, group := sut.Match("blocked1.com", []string{"gr1"})
Expand Down Expand Up @@ -147,7 +147,7 @@ var _ = Describe("ListCache", func() {
"gr1": {s.URL},
}

sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 30*time.Second)
sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 30*time.Second, 3, time.Second)
time.Sleep(time.Second)
By("Lists loaded without err", func() {
found, group := sut.Match("blocked1.com", []string{"gr1"})
Expand All @@ -171,7 +171,7 @@ var _ = Describe("ListCache", func() {
"gr2": {server3.URL},
}

sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 30*time.Second)
sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 30*time.Second, 3, time.Second)

found, group := sut.Match("blocked1.com", []string{"gr1", "gr2"})
Expect(found).Should(BeTrue())
Expand All @@ -192,7 +192,7 @@ var _ = Describe("ListCache", func() {
"withDeadLink": {"http://wrong.host.name"},
}

sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 30*time.Second)
sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 30*time.Second, 3, time.Second)

found, group := sut.Match("blocked1.com", []string{})
Expect(found).Should(BeFalse())
Expand All @@ -211,7 +211,7 @@ var _ = Describe("ListCache", func() {
resultCnt = cnt
})

sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 30*time.Second)
sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 30*time.Second, 3, time.Second)

found, group := sut.Match("blocked1.com", []string{})
Expect(found).Should(BeFalse())
Expand All @@ -226,7 +226,7 @@ var _ = Describe("ListCache", func() {
"gr2": {"file://" + file3.Name()},
}

sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 0)
sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 0, 3, time.Second)

found, group := sut.Match("blocked1.com", []string{"gr1", "gr2"})
Expect(found).Should(BeTrue())
Expand All @@ -247,7 +247,7 @@ var _ = Describe("ListCache", func() {
"gr1": {"inlinedomain1.com\n#some comment\n#inlinedomain2.com"},
}

sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 0)
sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 0, 3, time.Second)

found, group := sut.Match("inlinedomain1.com", []string{"gr1"})
Expect(found).Should(BeTrue())
Expand All @@ -264,7 +264,7 @@ var _ = Describe("ListCache", func() {
"gr1": {"/^apple\\.(de|com)$/\n"},
}

sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 0)
sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 0, 3, time.Second)

found, group := sut.Match("apple.com", []string{"gr1"})
Expect(found).Should(BeTrue())
Expand All @@ -284,7 +284,7 @@ var _ = Describe("ListCache", func() {
"gr2": {"inline\ndefinition\n"},
}

sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 0)
sut, _ := NewListCache(ListCacheTypeBlacklist, lists, 0, 0, 3, time.Second)

c := sut.Configuration()
Expect(c).Should(HaveLen(11))
Expand All @@ -296,7 +296,7 @@ var _ = Describe("ListCache", func() {
"gr1": {"file1", "file2"},
}

sut, _ := NewListCache(ListCacheTypeBlacklist, lists, -1, 0)
sut, _ := NewListCache(ListCacheTypeBlacklist, lists, -1, 0, 3, time.Second)

c := sut.Configuration()
Expect(c).Should(ContainElement("refresh: disabled"))
Expand Down
7 changes: 5 additions & 2 deletions resolver/blocking_resolver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,11 @@ func NewBlockingResolver(cfg config.BlockingConfig) (ChainedResolver, error) {
blockHandler := createBlockHandler(cfg)
refreshPeriod := time.Duration(cfg.RefreshPeriod)
timeout := time.Duration(cfg.DownloadTimeout)
blacklistMatcher, blErr := lists.NewListCache(lists.ListCacheTypeBlacklist, cfg.BlackLists, refreshPeriod, timeout)
whitelistMatcher, wlErr := lists.NewListCache(lists.ListCacheTypeWhitelist, cfg.WhiteLists, refreshPeriod, timeout)
cooldown := time.Duration(cfg.DownloadCooldown)
blacklistMatcher, blErr := lists.NewListCache(lists.ListCacheTypeBlacklist, cfg.BlackLists, refreshPeriod,
timeout, cfg.DownloadAttempts, cooldown)
whitelistMatcher, wlErr := lists.NewListCache(lists.ListCacheTypeWhitelist, cfg.WhiteLists, refreshPeriod,
timeout, cfg.DownloadAttempts, cooldown)
whitelistOnlyGroups := determineWhitelistOnlyGroups(&cfg)

var err error
Expand Down

0 comments on commit 2f79086

Please sign in to comment.