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

Plugins/DAO: incorrect function field reassignment #8788

Closed
1 task done
marc-charpentier opened this issue May 11, 2022 · 3 comments · Fixed by #8896
Closed
1 task done

Plugins/DAO: incorrect function field reassignment #8788

marc-charpentier opened this issue May 11, 2022 · 3 comments · Fixed by #8896
Labels

Comments

@marc-charpentier
Copy link
Contributor

marc-charpentier commented May 11, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Kong version ($ kong version)

Kong 2.8.1

Current Behavior

Hello,
In order to load a new plugin entity, a worker executes the Plugins.select_by_cache_key function (call to kong.db.plugins:select_by_cache_key(...), which delegates the load to DAO:select_by_cache_key, and checks whether the core.009_200_to_210 migration was executed previously.
If yes, before returning the loaded entity, the Plugins.select_by_cache_key function tries to self-replace by the DAO's one, but it doesn't work because the assigned field's object isn't the right one.
As an impact, an important extra latency (kong) is added to the plugin's load one, for each request involving such load.

Expected Behavior

Once a worker as ensured that the core.009_200_to_210 migration was executed, it should no longer recheck for any plugin updated / created afterward.

Steps To Reproduce

Problem encountered with Kong in "traditional" role.
On a local VM environnement (Kong and Cassandra containers), extra latency is about 10-15 ms, but on an integration environment (production-like with multi-DCs/multi-nodes Cassandra and Kong clusters), extra latency is about 500 ms.

Steps to reproduce (local environment):
- Start Kong with one single nginx worker.
- Use the Admin API to create a route (hosts[]=localhost) and a request-termination plugin on it (message=OK&status_code=200).
- Curl the route with -i switch in order to check the X-Kong-Response-Latency header:
  - Expected value: 0-2 ms.
  - Real value: 10-12 ms.
From now the request-termination plugin is in the cache.
- Curl again the same route, X-Kong-Response-Latency should be about 0-2 ms.
- Update the request-termination plugin (message=Hello).
- Curl again the same route, X-Kong-Response-Latency should be again about 10-12 ms.

On the integration environment, the X-Kong-Response-Latency value was about 500 ms for the first response after plugin creation/update.

Anything else?

To confirm that Plugins:select_by_cache_key function never succeeds to disable itself, I also added a notice message in its code, just before the instruction that should do the self-disabling. It should have logged the notice message only once (per worker, provided plugin is created/updated as many times as there are workers), but it did logged it for the first response after each plugin creation/update.

bungle added a commit that referenced this issue Jun 3, 2022
### Summary

When Kong 2.1.0 was release we needed to add special workaround to enable
blue-green deployment with the introduction of workspaces.

We needed to add special `select_by_cache_key` to `Plugins` DAO. The check
that we added is hurting the Kong performance, as seen reported in #8788.
We got further PR from the author with #8789. While the fix is good,
I do think we can remove this by now altogether. The only downside of this
is that is people migrate directly from Kong < 2.1.x to `3.0.0` we don't
support the blue-green deployment.

### Issues Resolved

Fixes #8788
Closes #8789
@jschmid1
Copy link
Contributor

jschmid1 commented Jun 9, 2022

I see that @bungle referenced this issue in #8896. Could you maybe have a look at this and confirm if that addresses that issue?

@bungle
Copy link
Member

bungle commented Jun 9, 2022

@jschmid1, I think @marc-charpentier already answered what they decided to do here:
#8789 (comment)

TL;DR; they decided to do what #8896 does.

bungle added a commit that referenced this issue Jun 14, 2022
### Summary

When Kong 2.1.0 was release we needed to add special workaround to enable
blue-green deployment with the introduction of workspaces.

We needed to add special `select_by_cache_key` to `Plugins` DAO. The check
that we added is hurting the Kong performance, as seen reported in #8788.
We got further PR from the author with #8789. While the fix is good,
I do think we can remove this by now altogether. The only downside of this
is that is people migrate directly from Kong < 2.1.x to `3.0.0` we don't
support the blue-green deployment.

### Issues Resolved

Fixes #8788
Closes #8789
@marc-charpentier
Copy link
Contributor Author

Added #8789 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants