From 8e3f67c8ccb82a02f2608f0d24157a8c9ab57ca0 Mon Sep 17 00:00:00 2001 From: Javier Guerra Date: Mon, 11 Apr 2022 21:27:25 -0500 Subject: [PATCH 1/4] perf(clustering) do granular hashes like #8519 does for old protocol. --- kong/clustering/wrpc_control_plane.lua | 5 +++-- kong/clustering/wrpc_data_plane.lua | 7 +++++-- kong/include/kong/services/config/v1/config.proto | 12 +++++++++++- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/kong/clustering/wrpc_control_plane.lua b/kong/clustering/wrpc_control_plane.lua index 0ccbfcca8e6..9ef85eb2774 100644 --- a/kong/clustering/wrpc_control_plane.lua +++ b/kong/clustering/wrpc_control_plane.lua @@ -151,7 +151,7 @@ function _M:export_deflated_reconfigure_payload() end end - local config_hash = self:calculate_config_hash(config_table) + local config_hash, hashes = self:calculate_config_hash(config_table) config_version = config_version + 1 -- store serialized plugins map for troubleshooting purposes @@ -162,7 +162,8 @@ function _M:export_deflated_reconfigure_payload() self.config_call_rpc, self.config_call_args = assert(service:encode_args("ConfigService.SyncConfig", { config = config_table, version = config_version, - hash = config_hash, + config_hash = config_hash, + hashes = hashes, })) return config_table, nil diff --git a/kong/clustering/wrpc_data_plane.lua b/kong/clustering/wrpc_data_plane.lua index b7cb80f7c55..cda078b9982 100644 --- a/kong/clustering/wrpc_data_plane.lua +++ b/kong/clustering/wrpc_data_plane.lua @@ -84,7 +84,8 @@ local function get_config_service() data.config.format_version = nil peer.config_obj.next_config = data.config - peer.config_obj.next_hash = data.hash + peer.config_obj.next_hash = data.config_hash + peer.config_obj.next_hashes = data.hashes peer.config_obj.next_config_version = tonumber(data.version) if peer.config_semaphore:count() <= 0 then -- the following line always executes immediately after the `if` check @@ -196,11 +197,12 @@ function _M:communicate(premature) end local config_table = self.next_config local config_hash = self.next_hash + local hashes = self.next_hashes if config_table and self.next_config_version > last_config_version then ngx_log(ngx_INFO, _log_prefix, "received config #", self.next_config_version, log_suffix) local pok, res - pok, res, err = xpcall(self.update_config, debug.traceback, self, config_table, config_hash, true) + pok, res, err = xpcall(self.update_config, debug.traceback, self, config_table, config_hash, true, hashes) if pok then last_config_version = self.next_config_version if not res then @@ -214,6 +216,7 @@ function _M:communicate(premature) if self.next_config == config_table then self.next_config = nil self.next_hash = nil + self.next_hashes = nil end end diff --git a/kong/include/kong/services/config/v1/config.proto b/kong/include/kong/services/config/v1/config.proto index e2f6d44b999..8ecc79ff5c9 100644 --- a/kong/include/kong/services/config/v1/config.proto +++ b/kong/include/kong/services/config/v1/config.proto @@ -85,6 +85,15 @@ message PluginVersion { string version = 2; } +message GranularHashes { + string config = 1; + string routes = 2; + string services = 3; + string plugins = 4; + string upstreams = 5; + string targets = 6; +} + message SyncConfigRequest { // Config represents a configuration of Kong Gateway. // This is same as the declarative configuration of Kong. @@ -101,7 +110,8 @@ message SyncConfigRequest { uint64 version = 2; // raw binary hash of the config data. - string hash = 3; + string config_hash = 3; + GranularHashes hashes = 4; } message SyncConfigResponse { From 56a3dd30a5678e72b137945918d158969be53593 Mon Sep 17 00:00:00 2001 From: Javier Guerra Date: Tue, 12 Apr 2022 22:48:45 -0500 Subject: [PATCH 2/4] perf(clustering) pass the `hashes` parameter through. --- kong/db/declarative/init.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kong/db/declarative/init.lua b/kong/db/declarative/init.lua index fee3e72b199..5c06604959f 100644 --- a/kong/db/declarative/init.lua +++ b/kong/db/declarative/init.lua @@ -993,7 +993,7 @@ do local DECLARATIVE_LOCK_KEY = "declarative:lock" -- make sure no matter which path it exits, we released the lock. - function declarative.load_into_cache_with_events(entities, meta, hash) + function declarative.load_into_cache_with_events(entities, meta, hash, hashes) local kong_shm = ngx.shared.kong local ok, err = kong_shm:add(DECLARATIVE_LOCK_KEY, 0, DECLARATIVE_LOCK_TTL) @@ -1007,7 +1007,7 @@ do return nil, err end - ok, err = load_into_cache_with_events_no_lock(entities, meta, hash) + ok, err = load_into_cache_with_events_no_lock(entities, meta, hash, hashes) kong_shm:delete(DECLARATIVE_LOCK_KEY) return ok, err From 1963bd91b14cfa5ba782bf08c39c6e4327722fa7 Mon Sep 17 00:00:00 2001 From: Javier Guerra Date: Tue, 12 Apr 2022 22:49:36 -0500 Subject: [PATCH 3/4] perf(clustering) ensure every hash field is non-nil. --- kong/clustering/init.lua | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/kong/clustering/init.lua b/kong/clustering/init.lua index 7de0f267895..923f2f819b5 100644 --- a/kong/clustering/init.lua +++ b/kong/clustering/init.lua @@ -192,6 +192,18 @@ function _M:calculate_config_hash(config_table) } end +local function fill_empty_hashes(hashes) + for _, field_name in ipairs{ + "config", + "routes", + "services", + "plugins", + "upstreams", + "targets", + } do + hashes[field_name] = hashes[field_name] or DECLARATIVE_EMPTY_CONFIG_HASH + end +end function _M:request_version_negotiation() local response_data, err = version_negotiation.request_version_handshake(self.conf, self.cert, self.cert_key) @@ -212,6 +224,10 @@ function _M:update_config(config_table, config_hash, update_cache, hashes) config_hash, hashes = self:calculate_config_hash(config_table) end + if hashes then + fill_empty_hashes(hashes) + end + local current_hash = declarative.get_current_hash() if current_hash == config_hash then ngx_log(ngx_DEBUG, _log_prefix, "same config received from control plane, ", @@ -234,8 +250,8 @@ function _M:update_config(config_table, config_hash, update_cache, hashes) -- NOTE: no worker mutex needed as this code can only be -- executed by worker 0 - local res, err = - declarative.load_into_cache_with_events(entities, meta, new_hash, hashes) + local res + res, err = declarative.load_into_cache_with_events(entities, meta, new_hash, hashes) if not res then return nil, err end From 0bbc2fc40d0051665aa2cf2130f97b430baa6ae3 Mon Sep 17 00:00:00 2001 From: Javier Guerra Date: Tue, 12 Apr 2022 22:50:00 -0500 Subject: [PATCH 4/4] perf(clustering) test hash fields. --- spec/01-unit/19-hybrid/02-clustering_spec.lua | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/spec/01-unit/19-hybrid/02-clustering_spec.lua b/spec/01-unit/19-hybrid/02-clustering_spec.lua index 8cb589b5bb0..6880944bfca 100644 --- a/spec/01-unit/19-hybrid/02-clustering_spec.lua +++ b/spec/01-unit/19-hybrid/02-clustering_spec.lua @@ -209,5 +209,71 @@ describe("kong.clustering", function() end end) + describe("granular hashes", function() + local DECLARATIVE_EMPTY_CONFIG_HASH = require("kong.constants").DECLARATIVE_EMPTY_CONFIG_HASH + + it("filled with empty hash values for missing config fields", function() + local value = {} + + for _ = 1, 10 do + local hash, hashes = clustering.calculate_config_hash(clustering, value) + assert.is_string(hash) + assert.equal("aaf38faf0b5851d711027bb4d812d50d", hash) + assert.is_table(hashes) + assert.same({ + config = "aaf38faf0b5851d711027bb4d812d50d", + routes = DECLARATIVE_EMPTY_CONFIG_HASH, + services = DECLARATIVE_EMPTY_CONFIG_HASH, + plugins = DECLARATIVE_EMPTY_CONFIG_HASH, + upstreams = DECLARATIVE_EMPTY_CONFIG_HASH, + targets = DECLARATIVE_EMPTY_CONFIG_HASH, + }, hashes) + end + end) + + it("has sensible values for existing fields", function() + local value = { + routes = {}, + services = {}, + plugins = {}, + } + + for _ = 1, 10 do + local hash, hashes = clustering.calculate_config_hash(clustering, value) + assert.is_string(hash) + assert.equal("768533baebe6e0d46de8d5f8a0c05bf0", hash) + assert.is_table(hashes) + assert.same({ + config = "768533baebe6e0d46de8d5f8a0c05bf0", + routes = "99914b932bd37a50b983c5e7c90ae93b", + services = "99914b932bd37a50b983c5e7c90ae93b", + plugins = "99914b932bd37a50b983c5e7c90ae93b", + upstreams = DECLARATIVE_EMPTY_CONFIG_HASH, + targets = DECLARATIVE_EMPTY_CONFIG_HASH, + }, hashes) + end + + value = { + upstreams = {}, + targets = {}, + } + + for _ = 1, 10 do + local hash, hashes = clustering.calculate_config_hash(clustering, value) + assert.is_string(hash) + assert.equal("6c5fb69169a0fabb24dcfa3a5d7a14b0", hash) + assert.is_table(hashes) + assert.same({ + config = "6c5fb69169a0fabb24dcfa3a5d7a14b0", + routes = DECLARATIVE_EMPTY_CONFIG_HASH, + services = DECLARATIVE_EMPTY_CONFIG_HASH, + plugins = DECLARATIVE_EMPTY_CONFIG_HASH, + upstreams = "99914b932bd37a50b983c5e7c90ae93b", + targets = "99914b932bd37a50b983c5e7c90ae93b", + }, hashes) + end + end) + end) + end) end)