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

Simplify registry data and add a new service data store seperated from registry #2030 #2468

Merged
merged 17 commits into from
Sep 14, 2018

Conversation

cvictory
Copy link
Contributor

@cvictory cvictory commented Sep 7, 2018

What is the purpose of the change

For issue: #2030

URL works as the carrier for Dubbo internal data transfer at runtime, and is also works as a protocol integrating with the external system: registration center, service query, service management, etc., resulting in serious coupling between the configuration and the registration center, and the data load pushed by the registration center is too large.

Keep URL unchanged at runtime, and even enrich the runtime URL; Keep the immutability of the URL during a single invoke from start to end.
Simplify Registry url as much as possible.
Refactor the way of doing service configuration, rely on Registry currently, switch to third-party Config Center while keeping compatible with old Overrides protocol.
Some data in URL are for service query purpose, we should register this part to another place instead of Registry. And it should be realized as extensible and users can decide to enable it or not.

Brief changelog

Verifying this change

XXXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a GITHUB_issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • [x ] Format the pull request title like [Dubbo-XXX] Fix UnknownException when host config not exist #XXX. Each commit in the pull request should have a meaningful subject line and body.
  • [x ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
  • Run mvn clean install -DskipTests & mvn clean test-compile failsafe:integration-test to make sure unit-test and integration-test pass.
  • If this contribution is large, please follow the Software Donation Guide.




AbstractInterfaceConfig extends AbstractMethodConfig {
Copy link
Contributor

Choose a reason for hiding this comment

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

Extra new lines here.

Copy link
Contributor

Choose a reason for hiding this comment

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

}
Map<String, String> newMap = new HashMap<String,String>(reserveParams.length);
Map<String, String> oldMap = result.getParameters();
for(String reserveParam : reserveParams){

Choose a reason for hiding this comment

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

code format

@@ -0,0 +1,41 @@
<?xml version="1.0" encoding="UTF-8"?>

Choose a reason for hiding this comment

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

LICENSE header.

import java.util.Set;

/**
*/

Choose a reason for hiding this comment

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

non meaningful comment

@@ -0,0 +1,31 @@
<?xml version="1.0" encoding="UTF-8"?>

Choose a reason for hiding this comment

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

license info too.

if (getServiceStoreConfig() == null) {
Map<String, ServiceStoreConfig> serviceStoreConfigMap = applicationContext == null ? null : BeanFactoryUtils.beansOfTypeIncludingAncestors(applicationContext, ServiceStoreConfig.class, false, false);
if (serviceStoreConfigMap != null && serviceStoreConfigMap.size() == 1) {
// 第一个元素
Copy link
Member

Choose a reason for hiding this comment

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

Chinese comment

@chickenlj
Copy link
Contributor

  • ServiceStore can replace ServiceStoreService as the entrance in RegistryConfig/ServiceConfig.
  • Delete useless attributes in ServiceStoreConfig.
  • Enrich method signature information, generalized inclusion and realize methods in PojoUtils may help.
  • ServiceStore's HA strategy: if the server cannot be reached, ignore and try background. You can reference FailbackRegistry.
  • ZK nodes written by ServiceStore should be changed to persistent
  • OPS will need to retrieve the ServiceStore instance and I think you should provide the exact URL format used by ServiceStore.
  • To decide which keys need to be registered to Registry, we can make it configurable by adding an item in dubbo.properties.
  • Add a switch to control the URL registered to Registry: register the complete URL to the registry by default.
  • Is it necessary to keep cache files locally in AbstractServiceStore? I think it's too complex, just log the error information in case of write failing can be enough.

@diecui1202 diecui1202 added this to the 2.7.0 milestone Sep 10, 2018
@chickenlj chickenlj merged commit 577eb77 into apache:dev-metadata Sep 14, 2018
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.

4 participants