AlinsRan commented on code in PR #13717:
URL: https://github.com/apache/apisix/pull/13717#discussion_r3662610076
##########
apisix/core/config_etcd.lua:
##########
@@ -637,12 +667,35 @@ local function load_full_data(self, dir_res, headers)
if self.filter then
self.filter(item)
end
+
+ else
+ local prev_item = get_prev_item(prev_values, prev_values_hash,
key)
+ if prev_item then
+ -- keep serving with the last valid configuration instead
of
+ -- silently dropping the whole item on a full reload, see
the
+ -- incremental path in sync_data for the same semantics
+ log.warn("failed to check item data of [", self.key, "/",
key,
+ "], keep the previous configuration, err: ", err)
+ insert_tab(self.values, prev_item)
+ self.values_hash[key] = #self.values
+ carried[prev_item] = true
+ end
end
self:upgrade_version(item.modifiedIndex)
end
end
+ -- fire the clean handlers of the previous items that were not carried
+ -- over: they were either replaced by a new value or deleted from etcd
+ if prev_values then
+ for _, item in ipairs(prev_values) do
+ if item and not carried[item] then
+ config_util.fire_all_clean_handlers(item)
+ end
+ end
+ end
Review Comment:
You're right that clean handlers have no registrant after #12426 (health
checks moved to `healthcheck_manager`), so this is effectively a no-op.
I've kept it here for consistency: `config_yaml`/`config_xds` full reloads
fire on replaced/deleted items the same way, and dropping it only in the etcd
full-reload path would leave a confusing asymmetry. Removing the whole
`clean_handlers` mechanism uniformly (the `config_util` API plus all fire sites
across the config backends) is broader than this bugfix, so I'll do it in a
dedicated follow-up PR rather than entangle it with a backportable fix.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
