-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
Initialize schemes before refreshing application context #5032
Initialize schemes before refreshing application context #5032
Conversation
Signed-off-by: John Niang <johnniang@foxmail.com>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #5032 +/- ##
=========================================
Coverage 55.94% 55.94%
+ Complexity 3034 3032 -2
=========================================
Files 525 524 -1
Lines 17816 17817 +1
Branches 1329 1328 -1
=========================================
+ Hits 9967 9968 +1
Misses 7300 7300
Partials 549 549 ☔ View full report in Codecov by Sentry. |
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.
Hi @guqing ,建议提供完整的错误日志。(发现错误日志前面有一个 Warning)。 |
几乎都是可以复现的,我尝试了两次不同的插件出现这个问题,可以尝试安装两到三个插件然后挨个卸载(不要等一个卸载完在卸载另一个) 日志如下
|
从日志中可以看到,卸载插件过程中出现了“Optimistic locking failure”错误,导致重复调用 当前 PR 的修改,应该是不会影响到插件的卸载流程。而且,我在本地多次测试插件(stop 方法中同样调用了 schemeManager#unregister 方法)卸载,都没有问题。 |
因插件中 stop 方法可能会被多次调用,建议通过以下方式 unregister scheme: - schemeManager.unregister(schemeManager.get(Synchronization.class));
+ schemeManager.unregister(Scheme.buildFromType(Synchronization.class)); |
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: guqing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind improvement
/area core
What this PR does / why we need it:
Prior to this proposal, we encountered an error requesting any page before Halo is in ready state. That is because the timing of schemes initialization is incorrect.
The current proposal is to advance schemes initialization before refreshing application and removes
SchemeInitializedEvent
because it cannot be listened by other beans.Which issue(s) this PR fixes:
Fixes #4946
Special notes for your reviewer:
Does this PR introduce a user-facing change?