Skip to content

feat: the traffic-split plugin supports upstream_id#3512

Merged
spacewander merged 12 commits into
apache:masterfrom
Firstsawyou:traffic-split-health-check
Feb 25, 2021
Merged

feat: the traffic-split plugin supports upstream_id#3512
spacewander merged 12 commits into
apache:masterfrom
Firstsawyou:traffic-split-health-check

Conversation

@Firstsawyou

@Firstsawyou Firstsawyou commented Feb 4, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

close #3094

How to implement the health check and retry mechanism:

Use the upstream_id method in the plugin to refer to the defined upstream so that we can reuse the health detection and retry mechanism that the upstream already has.

Reasons for the init.lua file to adjust the execution order of plugins:

Since the upstream_id in the init.lua file is processed before the plugin is executed, it will cause the upstream_id setting in the traffic-split plugin to take effect. Therefore, this PR adjusts the logic code executed by the plug-in in the init.lua file before the processing of upstream_id.

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? If it is not backward compatible, please discuss on the mailing list first

@Firstsawyou Firstsawyou changed the title feat: the traffic-split plugin supports health check and retry mechanism feat: the traffic-split plugin supports upstream_id Feb 4, 2021
membphis
membphis requested changes Feb 5, 2021
View reviewed changes
Comment thread t/plugin/traffic-split.t Outdated
@Firstsawyou
Firstsawyou marked this pull request as ready for review February 6, 2021 10:42
@moonming

moonming commented Feb 6, 2021

Copy link
Copy Markdown
Member

What is the relationship between this pr and #3094? I didn't see it

@Firstsawyou

Firstsawyou commented Feb 6, 2021

Copy link
Copy Markdown
Contributor Author

What is the relationship between this pr and #3094? I didn't see it

The feat of #3094 needs to be realized by supporting upstream_id in the plugin, and this pr is to solve the problem that the plugin supports upstream_id.

@moonming

moonming commented Feb 6, 2021

Copy link
Copy Markdown
Member

I still don't know why health check and retry mechanism are depends on upstream id？

@Firstsawyou

Firstsawyou commented Feb 7, 2021

Copy link
Copy Markdown
Contributor Author

I still don't know why health check and retry mechanism are depends on upstream id？

Use the upstream_id method in the plugin to refer to the upstream that has been defined, so that we can reuse the health checks and retries that upstream already has, instead of repeatedly implementing them in the plugin.

tokers
tokers reviewed Feb 7, 2021
View reviewed changes
Comment thread doc/plugins/traffic-split.md Outdated
Comment thread apisix/init.lua
Comment thread apisix/plugins/traffic-split.lua Outdated
Comment thread apisix/plugins/traffic-split.lua Outdated
Comment thread apisix/plugins/traffic-split.lua
@moonming

moonming commented Feb 7, 2021

Copy link
Copy Markdown
Member

I still don't know why health check and retry mechanism are depends on upstream id？

Use the upstream_id method in the plugin to refer to the upstream that has been defined, so that we can reuse the health checks and retries that upstream already has, instead of repeatedly implementing them in the plugin.

These should be explained in detail as much as possible in the issue and PR

@Firstsawyou
Firstsawyou force-pushed the traffic-split-health-check branch from d1112f4 to fd3f4c7 Compare February 20, 2021 06:17
tokers
tokers reviewed Feb 22, 2021
View reviewed changes
Comment thread apisix/plugins/traffic-split.lua
Comment thread apisix/init.lua
Comment thread doc/plugins/traffic-split.md
Comment thread doc/zh-cn/plugins/traffic-split.md
Comment thread t/plugin/traffic-split.t Outdated
Comment thread t/plugin/traffic-split.t Outdated
Comment thread t/plugin/traffic-split.t Outdated
@Firstsawyou
Firstsawyou requested a review from tokers February 22, 2021 11:25
@spacewander
spacewander merged commit 1d79ce5 into apache:master Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@membphis membphis membphis approved these changes
@tokers tokers tokers approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat: the traffic-split plugin supports health check and retry mechanism

5 participants

@Firstsawyou @moonming @membphis @tokers @spacewander