From 17071ae23fa22d45839278da15abeb40a25959ff Mon Sep 17 00:00:00 2001 From: Yuansheng Date: Thu, 3 Sep 2020 17:33:56 +0800 Subject: [PATCH 1/3] bugfix: it is unsafe to get the length of array if it contains any `nil` object, and we may get wrong length if it contain any `nil`. https://stackoverflow.com/questions/25903170/operator-nil-values-at-end-of-array --- apisix/core/config_etcd.lua | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/apisix/core/config_etcd.lua b/apisix/core/config_etcd.lua index 6e616b08f906..1bd4991c0c33 100644 --- a/apisix/core/config_etcd.lua +++ b/apisix/core/config_etcd.lua @@ -14,6 +14,8 @@ -- See the License for the specific language governing permissions and -- limitations under the License. -- + +local table = require("apisix.core.table") local config_local = require("apisix.core.config_local") local log = require("apisix.core.log") local json = require("apisix.core.json") @@ -330,20 +332,23 @@ local function sync_data(self) -- avoid space waste -- todo: need to cover this path, it is important. if self.sync_times > 100 then - local count = 0 - for i = 1, #self.values do + local count_new = 0 + local count_org = #self.values + for i = 1, count_org do local val = self.values[i] self.values[i] = nil if val then - count = count + 1 - self.values[count] = val + count_new = count_new + 1 + self.values[count_new] = val end end - for i = 1, count do + table.clear(self.values_hash) + for i = 1, count_new do key = short_key(self, self.values[i].key) self.values_hash[key] = i end + self.sync_times = 0 end From 96a9c2043436d0309964b084e331ce06183bf7e2 Mon Sep 17 00:00:00 2001 From: Yuansheng Date: Fri, 4 Sep 2020 14:08:31 +0800 Subject: [PATCH 2/3] bugfix: clear the value which stored by hash way. --- apisix/core/config_etcd.lua | 1 + 1 file changed, 1 insertion(+) diff --git a/apisix/core/config_etcd.lua b/apisix/core/config_etcd.lua index 1bd4991c0c33..100ed464cb90 100644 --- a/apisix/core/config_etcd.lua +++ b/apisix/core/config_etcd.lua @@ -320,6 +320,7 @@ local function sync_data(self) else self.sync_times = self.sync_times + 1 self.values[pre_index] = false + self.values_hash[key] = nil end elseif res.value then From 12a79e15f2deee5999cd6f42551ec384f1591486 Mon Sep 17 00:00:00 2001 From: Yuansheng Date: Fri, 4 Sep 2020 14:27:49 +0800 Subject: [PATCH 3/3] change: code style, more easy for reading. --- apisix/core/config_etcd.lua | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/apisix/core/config_etcd.lua b/apisix/core/config_etcd.lua index 100ed464cb90..2b6ba24848e8 100644 --- a/apisix/core/config_etcd.lua +++ b/apisix/core/config_etcd.lua @@ -333,19 +333,18 @@ local function sync_data(self) -- avoid space waste -- todo: need to cover this path, it is important. if self.sync_times > 100 then - local count_new = 0 - local count_org = #self.values - for i = 1, count_org do - local val = self.values[i] - self.values[i] = nil + local values_org = table.clone(self.values) + table.clear(self.values) + + for i = 1, #values_org do + local val = values_org[i] if val then - count_new = count_new + 1 - self.values[count_new] = val + table.insert(self.values, val) end end table.clear(self.values_hash) - for i = 1, count_new do + for i = 1, #self.values do key = short_key(self, self.values[i].key) self.values_hash[key] = i end