Skip to content

Conversation

@membphis
Copy link
Member

@membphis membphis commented Sep 25, 2020

What this PR does / why we need it:

when service works with upstream that contains a domain name, the merged configuration is always changing.
if the configuration is always changing, it won't hit the LRU item, which causes some plugins to work abnormally, such as limit-req.

fix: #2320

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible?

@membphis membphis changed the title Bug upstream parse lru bugfix: when service works with upstream that contains a domain name, the merged configuration is always changing. Sep 26, 2020
@membphis membphis changed the title bugfix: when service works with upstream that contains a domain name, the merged configuration is always changing. bugfix: when service works with upstream that contains a domain name, the merged configuration should not always changing. Sep 26, 2020
},
"upstream": {
"nodes": {
"www.baidu.com:80": 1
Copy link
Member

Choose a reason for hiding this comment

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

seems visit baidu.com unstable in github actions before?

@moonming moonming added this to the 2.0 milestone Sep 26, 2020
apisix/init.lua Outdated
return_direct, nil)
local err
route, err = lru_resolved_domain(route, api_ctx.conf_version,
parse_domain_in_route, route, api_ctx)
Copy link
Contributor

Choose a reason for hiding this comment

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

api_ctx need to be removed ?

Copy link
Member Author

Choose a reason for hiding this comment

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

nice, will fix it later

@membphis membphis changed the title bugfix: when service works with upstream that contains a domain name, the merged configuration should not always changing. [WIP] bugfix: when service works with upstream that contains a domain name, the merged configuration should not always changing. Sep 26, 2020
@membphis membphis removed this from the 2.0 milestone Sep 26, 2020
@membphis
Copy link
Member Author

@moonming this is an old bug for long time, and it seems that I need more time to fix it.
Therefore, I think it is not necessary for the 2.0 milestone.

@moonming
Copy link
Member

ok

@membphis membphis changed the title [WIP] bugfix: when service works with upstream that contains a domain name, the merged configuration should not always changing. bugfix: when service works with upstream that contains a domain name, the merged configuration should not always changing. Sep 27, 2020
@spacewander spacewander merged commit c5dcced into apache:master Oct 4, 2020
@membphis membphis deleted the bug-upstream-parse-lru branch October 5, 2020 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: limit-req plugin can not work properly for this case.

6 participants