You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "nic-6443 (via GitHub)" <gi...@apache.org> on 2025/08/14 09:56:53 UTC
[PR] fix(etcd): load full data from etcd while worker restart [apisix]
nic-6443 opened a new pull request, #12523:
URL: https://github.com/apache/apisix/pull/12523
### Description
When using etcd as the config server, the apisix master process performs a full data load during the init phase and saves the result in `loaded_configuration`. After the master forks other worker processes, these worker processes will directly initialize configurations based on `loaded_configuration`:
https://github.com/apache/apisix/blob/1b0bb84ccf343bd1aedc4bfff1afa9fcefb7c4d5/apisix/core/config_etcd.lua#L1023-L1031
Since `loaded_configuration` in the master process is not continuously updated, after running for some time, `loaded_configuration` may become outdated. If a worker restarts at this point (e.g., executing `apisix reload` or crash), it will use an outdated configuration for initialization. If any requests are sent to this worker at that time, these requests may be processed by incorrect confiurations.
#### Which issue(s) this PR fixes:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
Fixes #
### Checklist
- [ ] I have explained the need for this PR and the problem it solves
- [ ] I have explained the changes or the new features added to this PR
- [ ] I have added tests corresponding to this change
- [ ] I have updated the documentation to reflect this change
- [ ] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(etcd): load full data from etcd while worker restart [apisix]
Posted by "membphis (via GitHub)" <gi...@apache.org>.
membphis commented on code in PR #12523:
URL: https://github.com/apache/apisix/pull/12523#discussion_r2284702495
##########
apisix/core/config_etcd.lua:
##########
@@ -74,6 +74,7 @@ if not is_http then
end
local created_obj = {}
local loaded_configuration = {}
+local configuration_loaded_time
Review Comment:
I canโt sure if we can use Lua global var to cache the value. If I do this, will store it in sharedict
Pls add test it to confirm
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(etcd): load full data from etcd while worker restart [apisix]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 merged PR #12523:
URL: https://github.com/apache/apisix/pull/12523
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(etcd): load full data from etcd while worker restart [apisix]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 commented on code in PR #12523:
URL: https://github.com/apache/apisix/pull/12523#discussion_r2281496008
##########
apisix/core/config_etcd.lua:
##########
@@ -1157,8 +1168,14 @@ function _M.init_worker()
return nil, err
end
- if table.try_read_attr(local_conf, "apisix", "disable_sync_configuration_during_start") then
- return true
+ -- if the startup time of a worker differs significantly from that of the master process,
+ -- we consider it to have restarted, and at this point,
+ -- it is necessary to reload the full configuration from etcd.
+ if configuration_loaded_time and ngx_time() - configuration_loaded_time > 60 then
Review Comment:
This is an empirical value. I think 60 seconds is a sufficient duration to distinguish between the worker's first-time and restart scenarios. It can be made into a configuration parameter, but I don't think users would know how to set this configuration parameter properly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(etcd): load full data from etcd while worker restart [apisix]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on code in PR #12523:
URL: https://github.com/apache/apisix/pull/12523#discussion_r2278763590
##########
apisix/core/config_etcd.lua:
##########
@@ -1157,8 +1168,14 @@ function _M.init_worker()
return nil, err
end
- if table.try_read_attr(local_conf, "apisix", "disable_sync_configuration_during_start") then
- return true
+ -- if the startup time of a worker differs significantly from that of the master process,
+ -- we consider it to have restarted, and at this point,
+ -- it is necessary to reload the full configuration from etcd.
+ if configuration_loaded_time and ngx_time() - configuration_loaded_time > 60 then
Review Comment:
why hardcode it as 60 seconds? could it not be configured?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(etcd): load full data from etcd while worker restart [apisix]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on code in PR #12523:
URL: https://github.com/apache/apisix/pull/12523#discussion_r2283022477
##########
apisix/core/config_etcd.lua:
##########
@@ -1157,8 +1168,14 @@ function _M.init_worker()
return nil, err
end
- if table.try_read_attr(local_conf, "apisix", "disable_sync_configuration_during_start") then
- return true
+ -- if the startup time of a worker differs significantly from that of the master process,
+ -- we consider it to have restarted, and at this point,
+ -- it is necessary to reload the full configuration from etcd.
+ if configuration_loaded_time and ngx_time() - configuration_loaded_time > 60 then
Review Comment:
> but I don't think users would know how to set this configuration parameter properl
makes sense ๐๐ผ
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(etcd): load full data from etcd while worker restart [apisix]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 commented on code in PR #12523:
URL: https://github.com/apache/apisix/pull/12523#discussion_r2284714538
##########
apisix/core/config_etcd.lua:
##########
@@ -74,6 +74,7 @@ if not is_http then
end
local created_obj = {}
local loaded_configuration = {}
+local configuration_loaded_time
Review Comment:
Okay, I will add a test case, but this one is obviously working. You can see another variable above `loaded_configuration`, which uses the same mechanism to allow worker processes to reuse the etcd data loaded when the master process starts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(etcd): load full data from etcd while worker restart [apisix]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on code in PR #12523:
URL: https://github.com/apache/apisix/pull/12523#discussion_r2278763590
##########
apisix/core/config_etcd.lua:
##########
@@ -1157,8 +1168,14 @@ function _M.init_worker()
return nil, err
end
- if table.try_read_attr(local_conf, "apisix", "disable_sync_configuration_during_start") then
- return true
+ -- if the startup time of a worker differs significantly from that of the master process,
+ -- we consider it to have restarted, and at this point,
+ -- it is necessary to reload the full configuration from etcd.
+ if configuration_loaded_time and ngx_time() - configuration_loaded_time > 60 then
Review Comment:
why hardcode it as 60 seconds? could it not be made a configurable parameter?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org