-
Notifications
You must be signed in to change notification settings - Fork 8.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
feature: seata namingserver #5800
Conversation
implement domain model data structure provide register,unregister,discovery,watch restful api to client
add tests
Disables disabled disabled tests
converted 'node' to a singleton unified RaftCluster and DefaultCluster
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## summer-code_namingServer #5800 +/- ##
==============================================================
- Coverage 49.78% 47.96% -1.82%
+ Complexity 4545 4476 -69
==============================================================
Files 854 837 -17
Lines 29883 29287 -596
Branches 3660 3630 -30
==============================================================
- Hits 14877 14048 -829
- Misses 13516 13781 +265
+ Partials 1490 1458 -32
|
private List<Unit> unitData; | ||
|
||
public Cluster() { | ||
unitData = new ArrayList<>(); |
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.
Suggest deleting this line of code and assigning the value in line 24.
|
||
private Map<String, Object> metadata = new HashMap<>(); | ||
|
||
private static Instance instance; |
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.
Changing to a singleton pattern for static inner classes
} | ||
|
||
public void addMetadata(String key, Object value) { | ||
if (this.metadata == null) { |
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.
The value is already assigned when the instance is created, so why is it necessary to make a judgment here?
&& Objects.equals(unit, instance.unit); | ||
} | ||
|
||
public String mapToJsonString(Map<String, Object> map) { |
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.
should be reused in CollectionUtils.
|
||
|
||
// convert String to Object | ||
public static Node fromJsonString(String jsonString) { |
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.
Delete useless code
namingserver/.gitignore
Outdated
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.
No need for this file.
namingserver/mvnw
Outdated
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.
No need for this file.
namingserver/mvnw.cmd
Outdated
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.
No need for this file.
namingserver/pom.xml
Outdated
</dependency> | ||
<dependency> | ||
<groupId>io.seata</groupId> | ||
<artifactId>seata-core</artifactId> |
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.
Should not rely on the seata-core module
public class NamingController { | ||
private static final Logger LOGGER = LoggerFactory.getLogger(NamingController.class); | ||
|
||
private static final String OK = "200"; |
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.
Define a status code enumeration
@ConfigurationProperties(prefix = REGISTRY_NAMINGSERVER_PREFIX) | ||
public class RegistryNamingserverProperties { | ||
private String namespace = "public"; | ||
private String serverAddr = "127.0.0.1:8500"; |
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.
Why is the port 8500?
@@ -75,6 +92,47 @@ public void initialize(ConfigurableApplicationContext applicationContext) { | |||
// Load by priority | |||
System.setProperty("sessionMode", StoreConfig.getSessionMode().getName()); | |||
System.setProperty("lockMode", StoreConfig.getLockMode().getName()); | |||
|
|||
// load node properties | |||
Instance instance = Instance.getInstance(); |
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.
Don't have too much content in one method. Separate methods by role type.
} | ||
|
||
// load vgroup mapping relationship | ||
String storeType = ConfigurationFactory.getInstance().getConfig("store.mode", "db"); |
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.
Read from SessionMode
} | ||
|
||
@Override | ||
public HashMap<String, Object> load() { |
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.
change method name to loadVGroups
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.
This class has not been modified, please roll it back.
HashMap<String, Object> vGroupMapping = new HashMap<>(); | ||
try { | ||
File fileToLoad = new File(path); | ||
if (!fileToLoad.exists()) { |
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.
Did the file exist when you first started it?
public boolean addVGroup(MappingDO mappingDO) { | ||
String vGroup = mappingDO.getVGroup(); | ||
try (Jedis jedis = JedisPooledFactory.getJedisInstance(); Pipeline pipelined = jedis.pipelined()) { | ||
pipelined.hmset(vGroup, BeanUtils.objectToMap(mappingDO)); |
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.
Is there a thread safety issue?
# support: nacos 、 eureka 、 redis 、 zk 、 consul 、 etcd3 、 sofa | ||
type: file | ||
# support: nacos 、 eureka 、 redis 、 zk 、 consul 、 etcd3 、 sofa 、 namingserver | ||
type: namingserver |
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.
Change to file
|
||
namingserver: | ||
server-addr: | ||
- 127.0.0.1:8889 |
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.
Why don't you use a string, but a list?
store: | ||
# support: file 、 db 、 redis | ||
mode: file | ||
# db: |
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.
Please remove these test codes
console/pom.xml
Outdated
<dependency> | ||
<groupId>io.seata</groupId> | ||
<artifactId>seata-common</artifactId> | ||
<version>2.0.0-SNAPSHOT</version> |
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.
I don't think you need to write the version number, do you?
core/pom.xml
Outdated
@@ -67,6 +67,10 @@ | |||
<artifactId>fastjson</artifactId> | |||
<scope>test</scope> | |||
</dependency> | |||
<dependency> | |||
<groupId>org.apache.httpcomponents</groupId> | |||
<artifactId>httpclient</artifactId> |
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.
Why does the core module depend on httpclient?
<dependency> | ||
<groupId>io.seata</groupId> | ||
<artifactId>seata-common</artifactId> | ||
<version>2.0.0-SNAPSHOT</version> |
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.
No need to write the version number
script/config-center/config.txt
Outdated
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.
How do you remove all the original configurations here?
script/server/db/mysql.sql
Outdated
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.
Other databases should also be supplemented with relevant table structures
server/pom.xml
Outdated
<dependency> | ||
<groupId>org.codehaus.jackson</groupId> | ||
<artifactId>jackson-mapper-asl</artifactId> | ||
<version>1.9.13</version> |
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.
No version number should be added, all version dependencies should be handled in the dependencies module.
server/pom.xml
Outdated
@@ -344,11 +337,6 @@ | |||
</ports> | |||
<labels> | |||
<name>seata-server</name> | |||
<git.commit.message.full>${git.commit.message.full}</git.commit.message.full> |
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.
why remove?
@@ -40,6 +53,60 @@ | |||
* @author slievrly | |||
*/ | |||
public class Server { | |||
|
|||
private static final String SEATA_ROOT_KEY = "seata"; |
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.
Why is he a static variable and not a local variable?
weight: 1 | ||
namingserver: | ||
server-addr: 127.0.0.1:8080,127.0.0.1:8081 | ||
cluster: cluster1 |
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.
cluster1 change to default
metadata: | ||
weight: 1 | ||
namingserver: | ||
server-addr: 127.0.0.1:8080,127.0.0.1:8081 |
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.
why port is 8080?It is possible to write only one address here, because it is only an example.
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
Ⅰ. Describe what this PR did
1.实现了注册中心的基本功能,包括服务注册,服务发现,心跳检测等
2.设计并实现了领域模型的数据结构,包括集群,节点等
3.添加了部分单元测试/集成测试,并在seata-samples上进行验证
namingserver提供的HTTP接口如下:
clusterName 节点所属集群名称
unitName 节点所属unit名称
node 节点实体类
node 节点实体类
vGroup 事务分组
clusterName 集群名称
unitName unit名称
vGroup 事务分组
vGroup 事务分组
timeout 超时时间
request 客户端HTTP请求
接口详情请查看[namingserver (getpostman.com)](https://documenter.getpostman.com/view/26098677/2s9YC31uAW)
Ⅱ. Does this pull request fix one issue?
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
1.执行mvn clean install "-Dmaven.test.skip=true"打包项目
2.将seata-samples中的seata-all版本换成新打包的版本
3.在seata-samples中修改registry配置,修改成
4.启动NamingserverApplication
5.修改server端的配置
6.启动server端的ServerApplication
7.启动DubboStockServiceStarter
,
DubboAccountServiceStarter,
DubboOrderServiceStarter,
DubboBusinessTesterⅤ. Special notes for reviews