-
Notifications
You must be signed in to change notification settings - Fork 26.6k
Feature improvement, only register one url for port unification. #12528
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
Changes from all commits
96c0eb6
692ff67
b2969d0
4bbd6c7
92d1c26
68d2004
c0b0467
13b93aa
2e9f863
1165258
053d04a
973c236
d2793c8
ac7bbd2
012e7c4
6ce82a7
34c23d2
59370c7
ebe72f6
3688890
e825b84
8a9c1ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -183,13 +183,13 @@ private void unRegisterShutdownHook() { | |
| } | ||
|
|
||
| /** | ||
| * Close registration of instance for pure Consumer process by setting registerConsumer to 'false' | ||
| * by default is true. | ||
| * Enable registration of instance for pure Consumer process by setting registerConsumer to 'true' | ||
| * by default is false. | ||
| */ | ||
| private boolean isRegisterConsumerInstance() { | ||
| Boolean registerConsumer = getApplication().getRegisterConsumer(); | ||
| if (registerConsumer == null) { | ||
| return true; | ||
| return false; | ||
| } | ||
| return Boolean.TRUE.equals(registerConsumer); | ||
| } | ||
|
|
@@ -746,23 +746,27 @@ private void startModules() { | |
| } | ||
|
|
||
| @Override | ||
| public void prepareApplicationInstance() { | ||
| public void prepareApplicationInstance(ModuleModel moduleModel) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ApplicationDeployer depends on ModuleModel might not a good design.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DefaultApplicationDeployer.java public void notifyModuleChanged(ModuleModel moduleModel, DeployState state) {
checkState(moduleModel, state);
}
public void checkState(ModuleModel moduleModel, DeployState moduleState) {
synchronized (stateLock) {
if (!moduleModel.isInternal() && moduleState == DeployState.STARTED) {
prepareApplicationInstance(moduleModel);
}
}It's currently designed to work this way:
ApplicationDeployer depends on ModuleModel mainly querying for statuses such as 'STARTED', 'isInternal', and 'hasRegistryInteraction'. All these statuses can be wrapped into an Event or something, but I think this can be further optimized with this issue #12750 |
||
| if (hasPreparedApplicationInstance.get()) { | ||
| return; | ||
| } | ||
|
|
||
| // export MetricsService | ||
| exportMetricsService(); | ||
|
|
||
| if (isRegisterConsumerInstance()) { | ||
| exportMetadataService(); | ||
| if (isRegisterConsumerInstance() || moduleModel.getDeployer().hasRegistryInteraction()) { | ||
| if (hasPreparedApplicationInstance.compareAndSet(false, true)) { | ||
| // register the local ServiceInstance if required | ||
| registerServiceInstance(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public synchronized void exportMetadataService() { | ||
| doExportMetadataService(); | ||
| } | ||
|
|
||
| public void prepareInternalModule() { | ||
| if (hasPreparedInternalModule) { | ||
| return; | ||
|
|
@@ -1081,7 +1085,7 @@ public void notifyModuleChanged(ModuleModel moduleModel, DeployState state) { | |
| public void checkState(ModuleModel moduleModel, DeployState moduleState) { | ||
| synchronized (stateLock) { | ||
| if (!moduleModel.isInternal() && moduleState == DeployState.STARTED) { | ||
| prepareApplicationInstance(); | ||
| prepareApplicationInstance(moduleModel); | ||
| } | ||
| DeployState newState = calculateState(); | ||
| switch (newState) { | ||
|
|
@@ -1183,8 +1187,8 @@ private void onInitialize() { | |
| } | ||
| } | ||
|
|
||
| private void exportMetadataService() { | ||
| if (!isStarting()) { | ||
| private void doExportMetadataService() { | ||
| if (!isStarting() && !isStarted()) { | ||
| return; | ||
| } | ||
| for (DeployListener<ApplicationModel> listener : listeners) { | ||
|
|
||
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.
Depends on MetadataService here might not a good design
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.
Consider provide a solution for this issue with #12750
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.
It seems that the full service export process can move from ServiceConfig to a new ModuleLifecycle implement? This might solve this problem
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.
Yes, I think so, this might need some extra efforts to merge this change with Lifecycle mechanism.