-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
refactor(clustering/sync): improve readability and debugability #14170
base: master
Are you sure you want to change the base?
Conversation
7bd1142
to
00bb83b
Compare
00bb83b
to
1a34b5a
Compare
537d6f2
to
b18175b
Compare
ee07453
to
43975e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!! all my comments are included in its associated EE pr, which have been resolved by xuming already
if is_full_sync then | ||
local entity_not_found = err and err:match("MDB_NOTFOUND") | ||
-- full sync needs extra tolerance for errors | ||
if not res and not entity_not_found then | ||
return nil, err | ||
end | ||
else | ||
if not res then | ||
return nil, err | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a block, i know you're struggling with this PR.
if is_full_sync then | |
local entity_not_found = err and err:match("MDB_NOTFOUND") | |
-- full sync needs extra tolerance for errors | |
if not res and not entity_not_found then | |
return nil, err | |
end | |
else | |
if not res then | |
return nil, err | |
end | |
end | |
if is_full_sync then | |
local entity_not_found = err and err:match("MDB_NOTFOUND") | |
-- full sync needs extra tolerance for errors | |
if not res and not entity_not_found then | |
return nil, err | |
end | |
end | |
if not res then | |
return nil, err | |
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are different. The later is incorrect.
5092ad4
to
5e42c09
Compare
1. abstract functions and reshape the code, including error handling 2. extra call to notify CP the state of sync; 3. more debuging logs; 4. sync once retry will not count timeout as retry, allowing DP to sync deltas right after a full sync KAG-6177
5e42c09
to
2d55554
Compare
Summary
Checklist
changelog/unreleased/kong
orskip-changelog
label added on PR if changelog is unnecessary. README.mdIssue reference
KAG-6177