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

fix(pluginservers): restart all instances of a plugin #11306

Merged
merged 1 commit into from
Jul 27, 2023

Conversation

gszr
Copy link
Member

@gszr gszr commented Jul 27, 2023

NOTE to reviewers: this fix has already been merged into Enterprise.

When the plugin -- which acts as a plugin server -- dies for some reason, all entities associated with it must be invalidated, not only the current.

If two instances A and B of an external plugin P exist (e.g., if applied to different route objects), they receive a unique ID [1] defined by the the plugin server when the instance is started. If A and B are started sequentially and are the first instances, they get IDs 0 and 1, respectively. This ID is cached on the Kong side by the plugin server manager [2]. If plugin P dies, Kong plugin server manager will detect this condition and restart it.

As it was, the code would remove whichever instance first detected the condition and leave the other in running_instances; once the plugin server was up, the removed instance would be restarted, in some cases with the same Id already in use by the stale instance that should have been removed from running_instances. Subsequent requests may then run the wrong instance of the external plugin. This change fixes the issue by ensuring that both instances A and B are removed from the running_instances table,

[1] https://github.com/Kong/go-pdk/blob/4deec45a5bfe421a0ceebcbf7931d395250949b0/server/instance.go#L47
[2] kong/runloop/plugin_servers/process.lua

When the plugin -- which acts as a plugin server -- dies for some
reason, all entities associated with it must be invalidated, not only
the current.

If two instances A and B of an external plugin P exist (e.g., if applied
to different route objects), they receive a unique ID [1] defined by the
the plugin server when the instance is started. If A and B are started
sequentially and are the first instances, they get IDs 0 and 1, respectively.
This ID is cached on the Kong side by the plugin server manager [2]. If
plugin P dies, Kong plugin server manager will detect this condition and
restart it.

As it was, the code would remove whichever instance first
detected the condition and leave the other in `running_instances`; once
the plugin server was up, the removed instance would be restarted, in
some cases with the same Id already in use by the stale instance that
should have been removed from `running_instances`. Subsequent requests
may then run the wrong instance of the external plugin. This change
fixes the issue by ensuring that both instances A and B are removed
from the `running_instances` table,

[1] https://github.com/Kong/go-pdk/blob/4deec45a5bfe421a0ceebcbf7931d395250949b0/server/instance.go#L47
[2] `kong/runloop/plugin_servers/process.lua`
@gszr gszr force-pushed the fix/pluginserver-instance-cleaning branch from 785bf34 to 85db724 Compare July 27, 2023 12:33
Copy link
Contributor

@flrgh flrgh left a comment

Choose a reason for hiding this comment

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

👍 merge at your convenience

@gszr gszr merged commit 32528da into master Jul 27, 2023
@gszr gszr deleted the fix/pluginserver-instance-cleaning branch July 27, 2023 19:50
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.

2 participants