-
Notifications
You must be signed in to change notification settings - Fork 170
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-9003 in boot mode on worker init check configuration expiration #1399
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thomasmaas
approved these changes
Mar 24, 2023
ernaniaz
approved these changes
Mar 31, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on a quicklab and solved the issue.
coderbydesign
added a commit
to RedHatInsights/APIcast
that referenced
this pull request
Jun 7, 2024
…sabled Details of the issue as well as local reproducers are documented in 3scale#1467. This change ensures that when `boot.init_worker` checks to see whether or not the configuration needs to be reloaded, it also checks to confirm that the `boot.ttl()` value set from `APICAST_CONFIGURATION_CACHE` is a positive number, in order to avoid an immediate and infinite recursive loop of refreshing the configuration. This should still preserve the initial fix [1] where this regression appears to have been introduced, as we should never schedule reloading with the configuration settings of: ``` APICAST_CONFIGURATION_LOADER: boot APICAST_CONFIGURATION_CACHE: -1 ``` We should also never have a situation where we'd need to account for `interval = 0` since we fail out in `boot.init` in that case [2]. Tested locally as documented in the issue, however based on the comment in the initial fix PR, I've not added any tests in code. Happy to do so if there's now a good way to accomplish that! [1] 3scale#1399 [2] https://github.com/3scale/APIcast/blob/7e7eaf6f1d584c78d999c0d09f5b65203161c402/gateway/src/apicast/configuration_loader.lua#L146-L149
coderbydesign
added a commit
to coderbydesign/APIcast
that referenced
this pull request
Jun 7, 2024
…sabled Details of the issue as well as local reproducers are documented in 3scale#1467. This change ensures that when `boot.init_worker` checks to see whether or not the configuration needs to be reloaded, it also checks to confirm that the `boot.ttl()` value set from `APICAST_CONFIGURATION_CACHE` is a positive number, in order to avoid an immediate and infinite recursive loop of refreshing the configuration. This should still preserve the initial fix [1] where this regression appears to have been introduced, as we should never schedule reloading with the configuration settings of: ``` APICAST_CONFIGURATION_LOADER: boot APICAST_CONFIGURATION_CACHE: -1 ``` We should also never have a situation where we'd need to account for `interval = 0` since we fail out in `boot.init` in that case [2]. Tested locally as documented in the issue, however based on the comment in the initial fix PR, I've not added any tests in code. Happy to do so if there's now a good way to accomplish that! [1] 3scale#1399 [2] https://github.com/3scale/APIcast/blob/7e7eaf6f1d584c78d999c0d09f5b65203161c402/gateway/src/apicast/configuration_loader.lua#L146-L149
tkan145
pushed a commit
to tkan145/APIcast
that referenced
this pull request
Jul 9, 2024
…sabled Details of the issue as well as local reproducers are documented in 3scale#1467. This change ensures that when `boot.init_worker` checks to see whether or not the configuration needs to be reloaded, it also checks to confirm that the `boot.ttl()` value set from `APICAST_CONFIGURATION_CACHE` is a positive number, in order to avoid an immediate and infinite recursive loop of refreshing the configuration. This should still preserve the initial fix [1] where this regression appears to have been introduced, as we should never schedule reloading with the configuration settings of: ``` APICAST_CONFIGURATION_LOADER: boot APICAST_CONFIGURATION_CACHE: -1 ``` We should also never have a situation where we'd need to account for `interval = 0` since we fail out in `boot.init` in that case [2]. Tested locally as documented in the issue, however based on the comment in the initial fix PR, I've not added any tests in code. Happy to do so if there's now a good way to accomplish that! [1] 3scale#1399 [2] https://github.com/3scale/APIcast/blob/7e7eaf6f1d584c78d999c0d09f5b65203161c402/gateway/src/apicast/configuration_loader.lua#L146-L149
tkan145
pushed a commit
to tkan145/APIcast
that referenced
this pull request
Jul 10, 2024
…sabled Details of the issue as well as local reproducers are documented in 3scale#1467. This change ensures that when `boot.init_worker` checks to see whether or not the configuration needs to be reloaded, it also checks to confirm that the `boot.ttl()` value set from `APICAST_CONFIGURATION_CACHE` is a positive number, in order to avoid an immediate and infinite recursive loop of refreshing the configuration. This should still preserve the initial fix [1] where this regression appears to have been introduced, as we should never schedule reloading with the configuration settings of: ``` APICAST_CONFIGURATION_LOADER: boot APICAST_CONFIGURATION_CACHE: -1 ``` We should also never have a situation where we'd need to account for `interval = 0` since we fail out in `boot.init` in that case [2]. Tested locally as documented in the issue, however based on the comment in the initial fix PR, I've not added any tests in code. Happy to do so if there's now a good way to accomplish that! [1] 3scale#1399 [2] https://github.com/3scale/APIcast/blob/7e7eaf6f1d584c78d999c0d09f5b65203161c402/gateway/src/apicast/configuration_loader.lua#L146-L149
tkan145
pushed a commit
to tkan145/APIcast
that referenced
this pull request
Jul 17, 2024
…sabled Details of the issue as well as local reproducers are documented in 3scale#1467. This change ensures that when `boot.init_worker` checks to see whether or not the configuration needs to be reloaded, it also checks to confirm that the `boot.ttl()` value set from `APICAST_CONFIGURATION_CACHE` is a positive number, in order to avoid an immediate and infinite recursive loop of refreshing the configuration. This should still preserve the initial fix [1] where this regression appears to have been introduced, as we should never schedule reloading with the configuration settings of: ``` APICAST_CONFIGURATION_LOADER: boot APICAST_CONFIGURATION_CACHE: -1 ``` We should also never have a situation where we'd need to account for `interval = 0` since we fail out in `boot.init` in that case [2]. Tested locally as documented in the issue, however based on the comment in the initial fix PR, I've not added any tests in code. Happy to do so if there's now a good way to accomplish that! [1] 3scale#1399 [2] https://github.com/3scale/APIcast/blob/7e7eaf6f1d584c78d999c0d09f5b65203161c402/gateway/src/apicast/configuration_loader.lua#L146-L149
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Fixes https://issues.redhat.com/browse/THREESCALE-9003
Note; no test provided.
boot.init
andboot.init_worker
are hard to test and they are not tested. Manual test required.Verification steps
Test 1 === Re-spawn nginx worker before TTL expired
boot
with a non-zero TTL (i.e. 30s) and 1 workerTest 2 === Re-spawn nginx worker after TTL expired
boot
with a non-zero TTL (i.e. 30s) and 1 workerThe user_key value is irrelevant because the backend endpoint is overridden. The host must match the 3scale product host.
The log lines show that within one second, the worker process is re-spawned and configuration is loaded.