Skip to content
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

bug: 'global_rule' is a boolean value #1473

Closed
membphis opened this issue Apr 19, 2020 · 4 comments · Fixed by #1517
Closed

bug: 'global_rule' is a boolean value #1473

membphis opened this issue Apr 19, 2020 · 4 comments · Fixed by #1517
Labels
bug Something isn't working

Comments

@membphis
Copy link
Member

membphis commented Apr 19, 2020

Steps to reproduce:

  1. Add global routes (such as prometheus)
  2. Delete some global routes
  3. User request, we can see the response 500 error.

One photo from community:

image

This patch should be fine, and we need to add test case for confirming.

[proxychains] DLL init
diff --git a/apisix/init.lua b/apisix/init.lua
index aa8598e8..1e6b1efb 100644
--- a/apisix/init.lua
+++ b/apisix/init.lua
@@ -264,14 +264,16 @@ function _M.http_access_phase()
        and #router.global_rules.values > 0 then
         local plugins = core.tablepool.fetch("plugins", 32, 0)
         for _, global_rule in ipairs(router.global_rules.values) do
-            api_ctx.conf_type = "global_rule"
-            api_ctx.conf_version = global_rule.modifiedIndex
-            api_ctx.conf_id = global_rule.value.id
-
-            core.table.clear(plugins)
-            api_ctx.plugins = plugin.filter(global_rule, plugins)
-            run_plugin("rewrite", plugins, api_ctx)
-            run_plugin("access", plugins, api_ctx)
+            if type(global_rule) == "table" then
+                api_ctx.conf_type = "global_rule"
+                api_ctx.conf_version = global_rule.modifiedIndex
+                api_ctx.conf_id = global_rule.value.id
+
+                core.table.clear(plugins)
+                api_ctx.plugins = plugin.filter(global_rule, plugins)
+                run_plugin("rewrite", plugins, api_ctx)
+                run_plugin("access", plugins, api_ctx)
+            end
         end

         core.tablepool.release("plugins", plugins)
@spacewander
Copy link
Member

I guess the problem happens there:
https://github.com/apache/incubator-apisix/blob/7440192f8fdf679dadc3e816767762e6047d0893/apisix/core/config_etcd.lua#L284

I am not quite understand the tombstone mechanism. Is the memory footprint so high that we need to use some trick? Can we implement an iterator to hide the detail?

@membphis
Copy link
Member Author

membphis commented Apr 21, 2020

@spacewander you are right.

If the original data is deleted, it is marked as "false".

This is why we need to check the data type first.

Can we implement an iterator to hide the detail?

can you provide an example? I can't imagine it.

@spacewander
Copy link
Member

In Lua, we can implement custom iterator to iterate table, for instance:

local t = {{"a"}, false, {"b"}}

local function _iterate_values(self, tab)
    while true do
        self.idx = self.idx + 1
        local v = tab[self.idx]
        if type(v) == "table" then
            return self.idx, v
        end
        if v == nil then
            return nil, nil
        end
        -- skip other type of values
    end
end

local function iterate_values(tab)
    local iter = setmetatable({idx = 0}, {__call = _iterate_values})
    return iter, tab, 0
end

local j = 0
for k = 1, 1e5 do
    for i, v in iterate_values(t) do
        j = j + i
    end
end

@membphis
Copy link
Member Author

Well done, a very good solution. We can use this method to iterate over all values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants