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

fix: etcd data sync exception #8493

Merged
merged 2 commits into from
Mar 21, 2023
Merged

fix: etcd data sync exception #8493

merged 2 commits into from
Mar 21, 2023

Conversation

jobop
Copy link
Contributor

@jobop jobop commented Dec 10, 2022

Description

Fixes #8492
Fix #8648

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 first)

@jobop jobop changed the title fix the bug:when the key is deleted is will throw a exception fix:when the key is deleted is will throw a exception Dec 10, 2022
@jobop jobop changed the title fix:when the key is deleted is will throw a exception fix: etcd data sync exception Dec 10, 2022
@jobop jobop marked this pull request as draft December 10, 2022 01:41
@jobop jobop marked this pull request as ready for review December 10, 2022 01:43
Copy link
Contributor Author

@jobop jobop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the key is deleted, the item will be set to false.

@@ -96,6 +96,10 @@ end

-- fire all clean handlers added by add_clean_handler.
function _M.fire_all_clean_handlers(item)
-- When the key is deleted, the item will be set to false.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the key is deleted, the item will be set to false.

Copy link
Member

@spacewander spacewander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to add a test case to avoid regression? For example, we can add a test in https://github.com/apache/apisix/blob/master/t/node/healthcheck-stop-checker.t

@jobop
Copy link
Contributor Author

jobop commented Dec 12, 2022

Is it possible to add a test case to avoid regression? For example, we can add a test in https://github.com/apache/apisix/blob/master/t/node/healthcheck-stop-checker.t

yes,i hava add some test cases

@@ -103,6 +108,9 @@ __DATA__
local item, idx1, idx2 = setup()
util.cancel_clean_handler(item, idx1)
util.fire_all_clean_handlers(item)

local item = setup_to_false()
util.fire_all_clean_handlers(item)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to test this bug by deleting a key as you described in the issue? It would be great if we could cover it via an integration test which is more readable.

@zhangliweixyz
Copy link

Hi, I had the same problem, and Is there any way to solve this problem?

@spacewander
Copy link
Member

@zhangliweixyz
Let's wait for @jobop to update the PR.

@basuotian
Copy link

we had the same problem, it occur especially in etcd_auto_compaction_mode=revision during we do a lot of update/delete operation to apisixroute

@zsmlinux
Copy link

zsmlinux commented Mar 3, 2023

How about this bug now?

@spacewander
Copy link
Member

Since people keep asking about it, we can merge it first.

@oyiadin
Copy link

oyiadin commented Mar 19, 2023

maybe related: #8648 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: attempt to index local 'item' (a boolean value) bug: when delete a route , it will trigger an exception
8 participants