feat(bootstrap): support multiple upstreams #782
Conversation
| if existingPrefix, ok := logger.Data[prefixField]; ok { | ||
| prefix = fmt.Sprintf("%s.%s", existingPrefix, prefix) | ||
| } | ||
|
|
||
| return logger.WithField(prefixField, prefix) |
There was a problem hiding this comment.
I haven't looked into it, but this might break the dashboards or whatever else depends on the prefix being exactly the name of the resolver.
There was a problem hiding this comment.
LGTM 👍
I added one test case to check the conversion of the new YAML structure (list of bootstrap entries).
Could you please change the documentation (configuration.md and config.yaml in docs folder)?
I think it would also make sense to write some e2e tests to test the bootstrap resolution, but this is not a part of this PR/issue. I'll take a look, how it can be achieved.
ThinkChaos
force-pushed
the
feat/parallel-bootstrap
branch
from
d403ae5 to
1e14e06
Compare
| if cachingCfg.MinCachingTime == 0 { | ||
| // Set a min time in case the user didn't to avoid prefetching too often | ||
| cachingCfg.MinCachingTime = config.Duration(time.Hour) | ||
| } |
There was a problem hiding this comment.
Testing this some more, I noticed that caching.minTime has no default.
Since with this MR, the caching resolver is always enabled with prefetching as part of the Bootstrap, this can cause a lot of churn for not much gains.
I noticed because the domain s3.amazonaws.com used by the list https://s3.amazonaws.com/lists.disconnect.me/simple_ad.txt I have in my test config has a TTL of 3, and was being prefetched every 10 seconds.
I added a default of 1h here, only if the user hasn't specified anything (or they put 0). Let me know if you think I should revert this and we should just trust the TTLs.
Codecov ReportBase: 93.29% // Head: 92.97% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## development #782 +/- ##
===============================================
- Coverage 93.29% 92.97% -0.32%
===============================================
Files 42 42
Lines 4845 4943 +98
===============================================
+ Hits 4520 4596 +76
- Misses 257 276 +19
- Partials 68 71 +3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
Added the docs, rebased and cleaned up the commits. This should be all good now if you approve! |
If more than one upstream is configured, they are raced via a `ParallelBestResolver`.
Also make `tcp+udp` upstreams use any IPs provided.
The goal is to be able to tell apart logs from difference sources, such as `bootstrap.parallel_best_resolver` and `parallel_best_resolver`.
Otherwise, a request to blocky could end up waiting for 2 DNS requests: 1. lookup the DNS server IP 2. forward the user request to the server looked-up in 1
ThinkChaos
force-pushed
the
feat/parallel-bootstrap
branch
from
1e14e06 to
9e9a23c
Compare
If more than one upstream is configured, they are raced via a
ParallelBestResolver.Other improvements:
Also make
tcp+udpupstreams use any IPs provided.Otherwise, a request to blocky could end up waiting for 2 DNS requests:
The goal is to be able to tell apart logs from difference sources, such as
bootstrap.parallel_best_resolverandparallel_best_resolver.Also includes #781 so the lint passes here.
Fixes #760.