Skip to content

fix: etcd data sync exception#8493

Merged
spacewander merged 2 commits into
apache:masterfrom
jobop:master
Mar 21, 2023
Merged

fix: etcd data sync exception#8493
spacewander merged 2 commits into
apache:masterfrom
jobop:master

Conversation

@jobop

@jobop jobop commented Dec 10, 2022

Copy link
Copy Markdown
Contributor

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

@jobop jobop left a comment

Copy link
Copy Markdown
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.


-- 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
Copy Markdown
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.

@spacewander spacewander left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@jobop

jobop commented Dec 12, 2022

Copy link
Copy Markdown
Contributor Author

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

Comment thread t/core/config_util.t
util.fire_all_clean_handlers(item)

local item = setup_to_false()
util.fire_all_clean_handlers(item)

Copy link
Copy Markdown
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
Copy Markdown

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

@spacewander

Copy link
Copy Markdown
Member

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

@basuotian

Copy link
Copy Markdown

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

@Ethan-Zhang-ZH

Copy link
Copy Markdown

How about this bug now?

@spacewander

Copy link
Copy Markdown
Member

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

@oyiadin

oyiadin commented Mar 19, 2023

Copy link
Copy Markdown

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