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

Support dynamic admin API && correct syncer && make interface cleaner #96

Merged
merged 1 commit into from
Jul 2, 2021

Conversation

xxx7xxxx
Copy link
Contributor

  • Support dynamic(register/unregister) mux in admin api
  • Don't send data if not change in syncer(leverage it in supervisor)
  • Make the interface Object&Filter cleaner
  • Fix some minor bugs

@benja-wu benja-wu added the enhancement New feature or request label Jun 30, 2021
@benja-wu benja-wu added this to In progress in Easegress Project via automation Jun 30, 2021
@benja-wu benja-wu added this to the v1.0.2 milestone Jun 30, 2021
@localvar
Copy link
Collaborator

localvar commented Jul 2, 2021

There's a potential issue with the new behavior of syncer in this PR, take the mesh ingress as an example:

When receiving the resource update event, the mesh ingress controller tasks it just as a notification and ignores data in the event, it then reloads all data itself and rebuilds all data structures, and it uses the old data if it fails to reload data or rebuild data structures for some reason. So, if the syncer only sends an event at a resource change, there's a possibility that the latest data cannot be applied in time.

@xxx7xxxx
Copy link
Contributor Author

xxx7xxxx commented Jul 2, 2021

it uses the old data if it fails to reload data or rebuild data structures for some reason.

Acceptable if the cluster failed is the reason.

So, if the syncer only sends an event at a resource change, there's a possibility that the latest data can be applied in time.

Do you mean not in time. If so, it's also acceptable.

@localvar
Copy link
Collaborator

localvar commented Jul 2, 2021

Do you mean not in time. If so, it's also acceptable.

It should be "cannot be applied in time", sorry for the typo.

Easegress Project automation moved this from In progress to Reviewer approved Jul 2, 2021
Copy link
Contributor

@benja-wu benja-wu left a comment

Choose a reason for hiding this comment

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

LGTM

@benja-wu benja-wu merged commit c04064c into easegress-io:main Jul 2, 2021
Easegress Project automation moved this from Reviewer approved to Done Jul 2, 2021
@xxx7xxxx xxx7xxxx deleted the refactoring branch July 2, 2021 09:58
@benja-wu benja-wu modified the milestones: v1.2.0, v.1.1.0 Jul 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants