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

perf(dbless): load declarative schema during init() #10932

Merged
merged 2 commits into from May 25, 2023

Conversation

flrgh
Copy link
Contributor

@flrgh flrgh commented May 23, 2023

Summary

I was curious as to why some admin API tests for /config were so slow and went digging. Turns out this endpoint was reloading the declarative schema on every request, adding several hundred milliseconds of latency.

This updates the logic in Kong.init() to load the declarative config schema and store it in the kong global at kong.db.declarative_config.

before

$ busted spec/02-integration/04-admin_api/15-off_spec.lua --filter concurrency
●
1 success / 0 failures / 0 errors / 0 pending : 42.547819 seconds

after

$ busted spec/02-integration/04-admin_api/15-off_spec.lua --filter concurrency
●
1 success / 0 failures / 0 errors / 0 pending : 3.752036 seconds

No additional tests are needed because the functionality is already tested:

The following tests exercise the declarative config endpoint (POST /config):

spec/02-integration/02-cmd/03-reload_spec.lua
spec/02-integration/04-admin_api/11-reports_spec.lua
spec/02-integration/04-admin_api/15-off_spec.lua
spec/02-integration/05-proxy/02-router_spec.lua
spec/02-integration/08-status_api/01-core_routes_spec.lua
spec/02-integration/08-status_api/03-readiness_endpoint_spec.lua
spec/02-integration/11-dbless/01-respawn_spec.lua
spec/02-integration/11-dbless/03-config_persistence_spec.lua

And the following tests exercise the data plane configuration reload functionality:

spec/02-integration/09-hybrid_mode/01-sync_spec.lua
spec/02-integration/09-hybrid_mode/03-pki_spec.lua
spec/02-integration/09-hybrid_mode/04-cp_cluster_sync_spec.lua
spec/02-integration/09-hybrid_mode/08-lazy_export_spec.lua
spec/02-integration/09-hybrid_mode/09-config-compat_spec.lua
spec/02-integration/09-hybrid_mode/10-forward-proxy_spec.lua

Checklist

  • The Pull Request has tests N/A - tested by existing tests
  • There's an entry in the CHANGELOG
  • There is a user-facing docs PR

This updates the logic in Kong.init() to load the declarative config
schema and store it in the kong global at `kong.db.declarative_config`.

This brings a substantial perf improvement to the /config endpoint,
which was previously reloading the schema on every request.
@flrgh
Copy link
Contributor Author

flrgh commented May 24, 2023

Seems worthy of a backport--at very least to 3.3. @hbagdi feel free to add/remove backport labels.

@hbagdi
Copy link
Member

hbagdi commented May 24, 2023

well, no one has complained.
@rainest How impactful is this to KIC?

Copy link
Contributor

@hanshuebner hanshuebner left a comment

Choose a reason for hiding this comment

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

(if this breaks kong but doesn't cause an existing test to fail, then... that's bad)

Be proactive and find out whether there are tests that would break if this change did not work.

@flrgh
Copy link
Contributor Author

flrgh commented May 24, 2023

(if this breaks kong but doesn't cause an existing test to fail, then... that's bad)

Be proactive and find out whether there are tests that would break if this change did not work.

@hanshuebner what I meant was that this doesn't need any new tests. There are plenty of tests for the components that were changed in this PR, and they would certainly fail if this change yielded unintended breakage. At a quick glance here are some:

The following tests exercise the declarative config endpoint (POST /config):

  • spec/02-integration/02-cmd/03-reload_spec.lua
  • spec/02-integration/04-admin_api/11-reports_spec.lua
  • spec/02-integration/04-admin_api/15-off_spec.lua
  • spec/02-integration/05-proxy/02-router_spec.lua
  • spec/02-integration/08-status_api/01-core_routes_spec.lua
  • spec/02-integration/08-status_api/03-readiness_endpoint_spec.lua
  • spec/02-integration/11-dbless/01-respawn_spec.lua
  • spec/02-integration/11-dbless/03-config_persistence_spec.lua

And the following tests exercise the data plane configuration reload functionality:

  • spec/02-integration/09-hybrid_mode/01-sync_spec.lua
  • spec/02-integration/09-hybrid_mode/03-pki_spec.lua
  • spec/02-integration/09-hybrid_mode/04-cp_cluster_sync_spec.lua
  • spec/02-integration/09-hybrid_mode/08-lazy_export_spec.lua
  • spec/02-integration/09-hybrid_mode/09-config-compat_spec.lua
  • spec/02-integration/09-hybrid_mode/10-forward-proxy_spec.lua

@hanshuebner
Copy link
Contributor

@flrgh I updated the PR description to include the testing information.

@hanshuebner hanshuebner merged commit c339a8c into master May 25, 2023
30 checks passed
@hanshuebner hanshuebner deleted the perf/declarative-config-schema-load branch May 25, 2023 03:53
@team-gateway-bot
Copy link
Collaborator

The backport to release/3.3.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-release/3.3.x release/3.3.x
# Navigate to the new working tree
cd .worktrees/backport-release/3.3.x
# Create a new branch
git switch --create backport-10932-to-release/3.3.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c339a8cb7a854da448c1e7e71944f9e0b46a0956
# Push it to GitHub
git push --set-upstream origin backport-10932-to-release/3.3.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-release/3.3.x

Then, create a pull request where the base branch is release/3.3.x and the compare/head branch is backport-10932-to-release/3.3.x.

flrgh added a commit that referenced this pull request May 25, 2023
* perf(dbless): load declarative schema during init()

This updates the logic in Kong.init() to load the declarative config
schema and store it in the kong global at `kong.db.declarative_config`.

This brings a substantial perf improvement to the /config endpoint,
which was previously reloading the schema on every request.

* docs(changelog): add entry for 10932

(cherry picked from commit c339a8c)
bungle pushed a commit that referenced this pull request May 29, 2023
* perf(dbless): load declarative schema during init()

This updates the logic in Kong.init() to load the declarative config
schema and store it in the kong global at `kong.db.declarative_config`.

This brings a substantial perf improvement to the /config endpoint,
which was previously reloading the schema on every request.

* docs(changelog): add entry for 10932

(cherry picked from commit c339a8c)
hanshuebner pushed a commit that referenced this pull request May 30, 2023
* perf(dbless): load declarative schema during init()

This updates the logic in Kong.init() to load the declarative config
schema and store it in the kong global at `kong.db.declarative_config`.

This brings a substantial perf improvement to the /config endpoint,
which was previously reloading the schema on every request.

* docs(changelog): add entry for 10932
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants