You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/09/21 18:35:04 UTC
[GitHub] [kafka] piotrrzysko opened a new pull request #9315: KAFKA-10496: Removed relying on external DNS servers in tests
piotrrzysko opened a new pull request #9315:
URL: https://github.com/apache/kafka/pull/9315
As ticket suggested I’ve tried to introduce in-memory DNS server for testing purposes, but unfortunately capability to change default DNS provider has been removed in Java 9: https://bugs.openjdk.java.net/browse/JDK-8134577. There is a request for restoring it: https://bugs.openjdk.java.net/browse/JDK-8192780, but so far has not been implemented.
Therefore this PR decouples DNS resolution from `InetAddress.getAllByName` implementation in order to mock its behavior in tests.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mumrah commented on pull request #9315: KAFKA-10496: Removed relying on external DNS servers in tests
Posted by GitBox <gi...@apache.org>.
mumrah commented on pull request #9315:
URL: https://github.com/apache/kafka/pull/9315#issuecomment-883454382
Hi @piotrrzysko 👋. Earlier this year, https://github.com/apache/kafka/commit/131d4753cfed65ed6dee0a8c754765c97c3d513f was committed which added an interface very similar to what you have here along with other host resolution fixes.
Looking over this PR, it seems like your ClientUtilsTest fixes could remove some potential flaky tests. Could you rebase this PR with trunk?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] piotrrzysko commented on pull request #9315: KAFKA-10496: Removed relying on external DNS servers in tests
Posted by GitBox <gi...@apache.org>.
piotrrzysko commented on pull request #9315:
URL: https://github.com/apache/kafka/pull/9315#issuecomment-696860022
@jolshan @mumrah @dajac Could you please take look at this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] piotrrzysko commented on pull request #9315: KAFKA-10496: Removed relying on external DNS servers in tests
Posted by GitBox <gi...@apache.org>.
piotrrzysko commented on pull request #9315:
URL: https://github.com/apache/kafka/pull/9315#issuecomment-696860022
@jolshan @mumrah @dajac Could you please take look at this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] piotrrzysko commented on pull request #9315: KAFKA-10496: Removed relying on external DNS servers in tests
Posted by GitBox <gi...@apache.org>.
piotrrzysko commented on pull request #9315:
URL: https://github.com/apache/kafka/pull/9315#issuecomment-697717628
@jolshan @mumrah Thanks for the review! Of course, I agree with you that it would be nice to use the mock in all tests, but after rethinking the solution suggested in this PR I came to the conclusion that it is not so easy.
I assumed that `InetAddress#getAllByName` is the only place where DNS resolution happens - it’s not true. The constructor of `InetSocketAddress` that is used by `ClientUtils#parseAndValidateAddresses` in some circumstances (when a passed hostname is a machine name rather than a literal IP address) may also call an external DNS server. This behavior is much harder to mock. Perhaps it is not a problem for tests from `ClientUtilsTest`, because as I checked only `localhost` is resolved in this way. However, I wouldn’t like to introduce abstraction over DNS resolving mechanism that is unable to cover all cases.
Unfortunately, the only solution that comes to my mind is to try a hack based on reflection that will allow replacing the default DNS provider in tests. It was something that I wanted to avoid because it can cause problems with future versions of Java. Nevertheless, I can check if it is possible and how ugly it will be :).
WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org