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

Jump to bottom

[fix][broker] DnsResolverUtil.TTL should be greater than zero #18565

Merged
merged 3 commits into from
Nov 22, 2022
Merged

[fix][broker] DnsResolverUtil.TTL should be greater than zero #18565

merged 3 commits into from
Nov 22, 2022

Conversation

Technoboy-
Copy link
Contributor

Motivation

When the user set DNS cache maxTTl to 0, it will throw below exception :

Caused by: java.lang.IllegalArgumentException: maxTtl : 0 (expected: > 0)
	at io.netty.util.internal.ObjectUtil.checkPositive(ObjectUtil.java:100)
	at io.netty.resolver.dns.DefaultDnsCnameCache.<init>(DefaultDnsCnameCache.java:60)
	at io.netty.resolver.dns.DnsNameResolverBuilder.newCnameCache(DnsNameResolverBuilder.java:451)
	at io.netty.resolver.dns.DnsNameResolverBuilder.build(DnsNameResolverBuilder.java:485)
	at io.netty.resolver.dns.DnsAddressResolverGroup.newNameResolver(DnsAddressResolverGroup.java:114)
	at io.netty.resolver.dns.DnsAddressResolverGroup.newResolver(DnsAddressResolverGroup.java:92)
	at io.netty.resolver.dns.DnsAddressResolverGroup.newResolver(DnsAddressResolverGroup.java:77)
	at io.netty.resolver.AddressResolverGroup.getResolver(AddressResolverGroup.java:70)

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@Technoboy- Technoboy- added this to the 2.12.0 milestone Nov 22, 2022
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 22, 2022
@Technoboy- Technoboy- self-assigned this Nov 22, 2022
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 22, 2022
@codecov-commenter
Copy link

codecov-commenter commented Nov 22, 2022 •
edited
Loading

Codecov Report

Merging #18565 (5e5a8b4) into master (be1d07e) will increase coverage by 10.48%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #18565       +/-   ##
=============================================
+ Coverage     36.82%   47.30%   +10.48%     
- Complexity     7842    10466     +2624     
=============================================
  Files           698      698               
  Lines         68060    68060               
  Branches       7277     7277               
=============================================
+ Hits          25062    32198     +7136     
+ Misses        39675    32271     -7404     
- Partials       3323     3591      +268
Flag Coverage Δ
unittests 47.30% <ø> (+10.48%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...java/org/apache/pulsar/proxy/stats/TopicStats.java 58.82% <0.00%> (-41.18%) ⬇️
...roker/service/persistent/MessageDeduplication.java 47.16% <0.00%> (-7.43%) ⬇️
...ction/buffer/impl/TransactionBufferClientImpl.java 76.74% <0.00%> (-4.66%) ⬇️
...pulsar/broker/service/PulsarCommandSenderImpl.java 73.84% <0.00%> (-4.62%) ⬇️
.../buffer/impl/TransactionBufferClientStatsImpl.java 82.00% <0.00%> (-4.00%) ⬇️
...pulsar/broker/TransactionMetadataStoreService.java 58.51% <0.00%> (-3.94%) ⬇️
...g/apache/pulsar/client/impl/ConnectionHandler.java 50.00% <0.00%> (-3.20%) ⬇️
...va/org/apache/pulsar/client/impl/HandlerState.java 67.56% <0.00%> (-2.71%) ⬇️
...tion/buffer/impl/TransactionBufferHandlerImpl.java 50.00% <0.00%> (-2.54%) ⬇️
...ervice/AbstractDispatcherSingleActiveConsumer.java 69.15% <0.00%> (-1.87%) ⬇️
... and 123 more

@tisonkun tisonkun changed the title [fix][broker] Fix DNS cache maxTtl issue. [fix][broker] DnsResolverUtil.TTL should be greater than zero Nov 22, 2022
tisonkun
tisonkun approved these changes Nov 22, 2022
View reviewed changes
Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. One minor suggestion.

pulsar-common/src/main/java/org/apache/pulsar/common/util/netty/DnsResolverUtil.java Outdated Show resolved Hide resolved
@nodece nodece merged commit 67f9461 into apache:master Nov 22, 2022
codelipenghui pushed a commit that referenced this pull request Nov 23, 2022
@Technoboy- @codelipenghui
[fix][broker] DnsResolverUtil.TTL should be greater than zero (#18565)
4fbf457 
(cherry picked from commit 67f9461)
codelipenghui pushed a commit that referenced this pull request Nov 23, 2022
@Technoboy- @codelipenghui
[fix][broker] DnsResolverUtil.TTL should be greater than zero (#18565)
9118645 
(cherry picked from commit 67f9461)
codelipenghui pushed a commit that referenced this pull request Nov 23, 2022
@Technoboy- @codelipenghui
[fix][broker] DnsResolverUtil.TTL should be greater than zero (#18565)
d698ac7 
(cherry picked from commit 67f9461)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Nov 23, 2022
@Technoboy- @nicoloboschi
[fix][broker] DnsResolverUtil.TTL should be greater than zero (apache…
b9d3d6f 
…#18565)

(cherry picked from commit 67f9461)
(cherry picked from commit 9118645)
congbobo184 pushed a commit that referenced this pull request Nov 26, 2022
@Technoboy-
[fix][broker] DnsResolverUtil.TTL should be greater than zero (#18565)
9cf76fd 
(cherry picked from commit 67f9461)
lifepuzzlefun pushed a commit to lifepuzzlefun/pulsar that referenced this pull request Dec 9, 2022
@Technoboy- @lifepuzzlefun
[fix][broker] DnsResolverUtil.TTL should be greater than zero (apache…
93f9f67 
…#18565)
lifepuzzlefun pushed a commit to lifepuzzlefun/pulsar that referenced this pull request Jan 10, 2023
@Technoboy- @lifepuzzlefun
[fix][broker] DnsResolverUtil.TTL should be greater than zero (apache…
c75d9ab 
…#18565)
@Technoboy- Technoboy- deleted the fix-dns-resolver branch September 14, 2023 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yaalsn yaalsn yaalsn approved these changes

@coderzc coderzc coderzc approved these changes

@tisonkun tisonkun tisonkun approved these changes

@nodece nodece nodece approved these changes

@eolivelli eolivelli eolivelli approved these changes

Assignees

@Technoboy- Technoboy-

Labels
area/broker cherry-picked/branch-2.9 Archived: 2.9 is end of life cherry-picked/branch-2.10 cherry-picked/branch-2.11 doc-not-needed Your PR changes do not impact docs ready-to-test release/2.8.5 release/2.9.4 release/2.10.3 release/2.11.1
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.
9 participants
@Technoboy- @codecov-commenter @eolivelli @yaalsn @nodece @tisonkun @coderzc @codelipenghui @congbobo184