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.

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

THREESCALE-8373 Pagination services and proxy config endpoints #1397

Merged
merged 11 commits into from
Mar 21, 2023

Conversation

eguzki
Copy link
Member

@eguzki eguzki commented Mar 16, 2023

what

Load all services and proxy configuration pages

Fixes https://issues.redhat.com/browse/THREESCALE-8373

Verification steps

  • Create more than 500 services in a tenant
  • Deploy APIcast using THREESCALE_PORTAL_ENDPOINT=https://<token>@<provider domain>
  • Send a request to one of last services created
  • Observe 200 OK response (also observe APIcast logs indicate that first 500 services were loaded in a first request and the remaining services in a second request)

@eguzki eguzki changed the title THREESCALE-8373 Pagination services proxy config endpoints THREESCALE-8373 Pagination services and proxy config endpoints Mar 16, 2023
@eguzki eguzki force-pushed the pagination-services-proxy-config-endpoints branch from 9fc9dea to f8fe429 Compare March 20, 2023 14:52
@eguzki eguzki marked this pull request as ready for review March 20, 2023 15:02
@eguzki eguzki requested review from a team as code owners March 20, 2023 15:02
@eguzki
Copy link
Member Author

eguzki commented Mar 20, 2023

@thomasmaas @pehala ready for review

cc @samugi @kevprice83

Copy link
Contributor

@pehala pehala left a comment

Choose a reason for hiding this comment

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

Nice job! Should a combination of this and #1352 improve the overall performance of APIcast?

@@ -241,6 +242,8 @@ Replace `${ID}` with the actual Service ID. The value should be the configuratio

Setting it to a particular version will prevent it from auto-updating and will always use that version.

**Note**: This env var cannot be used with `THREESCALE_PORTAL_ENDPOINT` pointing to custom path (i.e. master path).
Copy link
Contributor

Choose a reason for hiding this comment

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

Does that mean this variable won't work with basic staging/production APIcast created by 3scale operator? Did it work before and did it changed now or has it simply never worked before?

Copy link
Member Author

Choose a reason for hiding this comment

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

Never worked before. The env var was silently ignored. Now (actually in the previous PR), I added a hard check. The check will not kill the process, but will not process the request and will log error.

gateway/src/apicast/configuration_loader/remote_v2.lua Outdated Show resolved Hide resolved
ngx.log(ngx.DEBUG, 'services get error: ', err, ' url: ', url)
return nil, err
end
local SERVICES_PER_PAGE = 500
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason for this particular value? Do different values change performance in any reasonable way?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's the default value used in the 3scale API. The gateway still needs to fix the value because it needs to iterate comparing the number of returned results with that value. If the gateway does not fix that value, a change in the 3scale API can break the gateway pagination feature.

In the future, we can expose as env var

@eguzki eguzki force-pushed the pagination-services-proxy-config-endpoints branch from bedaa39 to 2653fce Compare March 21, 2023 17:24
@eguzki eguzki merged commit 3541cf5 into master Mar 21, 2023
@eguzki eguzki deleted the pagination-services-proxy-config-endpoints branch March 21, 2023 18:39
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.

None yet

3 participants