converted dns_lookup_adapter_resolver_pool_size warning message to use dynamic value instead of hard-coded value (10) #16475
Conversation
…e dynamic value instead of hard-coded value (10)
ryan-carroll-graylog
requested review from
danotorrey,
kingzacko1 and
ryan-carroll-graylog
There was a problem hiding this comment.
@williamtrelawny Looks great, thanks for knocking this out!
I think this does warrant a changelog entry since it is a user facing change in behavior. There's more info about changelogs here: https://graylogdocumentation.atlassian.net/wiki/spaces/GLE/pages/2518908931/Changelog+Entries
The TLDR is that you should add a changelog file in graylog2-server/changelog/unreleased/ to this PR.
Other than that this looks good and tests successfully! I moved the logging slightly to test and the results look good to me:
2023-09-14 11:43:34,334 WARN : org.graylog2.lookup.adapters.dnslookup.DnsResolverPool - Lease for resolver [04a8099f-1283-4627-b187-e8bea687b907] is in-use. Skipping refresh. This will be attempted again in [10] seconds. If this happens frequently for high message rates, consider increasing the [dns_lookup_adapter_resolver_pool_size = 99] server configuration property to allow more DNS resolvers.
There was a problem hiding this comment.
LGTM and tests successfully! Well done @williamtrelawny! Good call on the change log @ryan-carroll-graylog!
williamtrelawny
force-pushed
the
patch-dns_lookup_adapter_resolver_pool_size
branch
from
50e84d4
to
ff1be5c
Compare
|
Sorry about the force-push - I hit the darn "sync forks" button in the browser and muddled the commit history of this PR with an unrelated commit. Fixed now, adding the changelog entry now! Thanks for your all's support 😄 |
Signed-off-by: William Trelawny <will@graylog.com>
|
|
Signed-off-by: William Trelawny <william.trelawny@graylog.com>
Co-authored-by: Dan Torrey <dan@graylog.com>
williamtrelawny
force-pushed
the
patch-dns_lookup_adapter_resolver_pool_size
branch
from
86333b3
to
f26d5c6
Compare
|
Sorry again- I had the wrong email address in the signed-off-by so the CLA check failed... Amended and should all be good now! Next PR won't be this messy, I swear! 😅 |
|
All good @williamtrelawny! Fork PRs are always a bit challenging. Nice work! |
williamtrelawny
deleted the
patch-dns_lookup_adapter_resolver_pool_size
branch
Description
The value of dns_lookup_adapter_resolver_pool_size in the warning message for when this value should be increased should be dynamic and based on the actual value configured in server.conf. Previously, it was hard-coded to the default of "10".
Motivation and Context
Fixes inaccurate feedback to user who has modified the dns_lookup_adapter_resolver_pool_size value. Now the user will at least receive acknowledgement that their modified value is in use by Graylog.
How Has This Been Tested?
Not tested unfortunately as I do not have the knowledge/ability to. However this being a very minor change, I felt it appropriate to submit this PR anyway and welcome someone to test it who has the ability to.
Types of changes
Checklist: