-
Notifications
You must be signed in to change notification settings - Fork 170
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.
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
Resolver: using Fully qualified names. #1235
Conversation
gateway/src/resty/resolver.lua
Outdated
return nil, err | ||
end | ||
|
||
if string.sub(qname, -1) == "." then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not optimal. Would be better to cache the string.sub
to a local variable. The same for qname:sub
.
gateway/src/resty/resolver.lua
Outdated
return nil, err | ||
end | ||
|
||
local qname_last_char = string.sub(qname, -1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not optimal. Would be better to cache the string.sub
to a local variable. The same for goes for qname:sub
. Not the output of the call, but the function being called local sub = string.sub
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As explained in the video, using string.sub
needs to lookup string
, which is in _G
and going that far up in stack breaks JIT.
This might not be on a hot path, but better make it everywhere, so you don't have to think about it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sleepy morning :-( Fixed it now 😅
dc4a00f
to
68454f7
Compare
When adding a dot to the end of the domain, the search string from resolv.conf should not be added, because the domain is absolute. Adding this, will save a few DNS request to the DNS server because no cluster.local, etc.. will be added to the Qname. More info in relation to this: https://superuser.com/a/1468139 Fix THREESCALE-4752 Signed-off-by: Eloy Coto <[email protected]>
When adding a dot to the end of the domain, the search string from
resolv.conf should not be added, because the domain is absolute.
Adding this, will save a few DNS request to the DNS server because no
cluster.local, etc.. will be added to the Qname.
More info in relation to this:
https://superuser.com/a/1468139
Fix THREESCALE-4752
Signed-off-by: Eloy Coto [email protected]