Add cluster-etcd-plugin #2725

wayilau · 2019-05-21T06:30:50Z

Please answer these questions before submitting pull request

Why submit this pull request?
Bug fix
New feature provided
Improve performance

Add cluster etcd plugin.

wu-sheng

Please ref #2640. We need you to provide the full list of dependency libs.

oap-server/server-cluster-plugin/cluster-etcd-plugin/pom.xml

wayilau · 2019-05-21T08:34:16Z

Please ref #2640. We need you to provide the full list of dependency libs.

Group ID: org.mousio
Artifact ID: etcd4j
Version: 2.17.0
Apache License 2.0.

Group ID: com.github.wnameless
Artifact ID: json-flattener
Version: 0.6.0
Apache License 2.0.

Group ID: org.mousio
Artifact ID: etcd4j
Version: 2.17.0
Apache License 2.0.

Group ID: org.fasterxml.jackson.core
Artifact ID: jackson-core
Version: 2.9.5
Apache License 2.0.

Group ID: org.fasterxml.jackson.core
Artifact ID: jackson-databind
Version: 2.9.5
Apache License 2.0.

Group ID: org.fasterxml.jackson.core
Artifact ID: jackson-annotations
Version: 2.9.5
Apache License 2.0.

Group ID: org.fasterxml.jackson.module
Artifact ID: jackson-module-afterburner
Version: 2.8.11
Apache License 2.0.

Group ID: io.netty
Artifact ID: netty-codec-dns:4.1.32.Final
Version: 4.1.27.Final
Apache License 2.0.

Group ID: io.netty
Artifact ID: netty-codec-http:4.1.32.Final
Version: 4.1.27.Final
Apache License 2.0.

Group ID: io.netty
Artifact ID: netty-handler:4.1.32.Final
Version: 4.1.27.Final
Apache License 2.0.

Group ID: io.netty
Artifact ID: netty-resolver-dns:4.1.32.Final
Version: 4.1.27.Final
Apache License 2.0.

all netty jars uses from dist tar, version: 4.0.27.Final

wayilau · 2019-05-21T08:39:31Z

@wu-sheng is this check netty's license failed ?

wu-sheng · 2019-05-21T08:40:12Z

I have a little concern about the 2.x netty version. Right now, if you check our dist tar, there are 4.1.27 netty jar(s) there.

wayilau · 2019-05-21T09:38:42Z

I have a little concern about the 2.x netty version. Right now, if you check our dist tar, there are 4.1.27 netty jar(s) there.

there's not enough jars from the oap-libs of dist tar. we need provide version of 4.1.32.Final ? or i have a try to change the version of etcd4j

wu-sheng · 2019-05-21T09:46:40Z

We provide because grpc needs that. And our tar is not small, over 100m. Cluster management usually includes a lot of dependency libs. So, I want to make sure don't add many news.

wayilau · 2019-05-22T02:27:09Z

Local compile passed. Is there some differences between local and the travis-ci ? @wu-sheng jar's versionis: 4.1.27.Final which i was using.

wu-sheng · 2019-05-22T03:05:09Z

From the CI log, it means, you have files without ASF header. https://travis-ci.org/apache/skywalking/jobs/535203203

wu-sheng · 2019-05-22T03:05:52Z

And I am still waiting for a committer to review and test this feature.

wayilau · 2019-05-22T03:11:20Z

From the CI log, it means, you have files without ASF header. https://travis-ci.org/apache/skywalking/jobs/535203203

ok, i check the license again.

wayilau · 2019-05-22T03:21:02Z

From the CI log, it means, you have files without ASF header. https://travis-ci.org/apache/skywalking/jobs/535203203

ok, i check the license again.

this tell me my own files may have not ASF header?

wayilau · 2019-05-22T03:32:40Z

From the CI log, it means, you have files without ASF header. https://travis-ci.org/apache/skywalking/jobs/535203203

ok, i check the license again.

this tell me my own files may have not ASF header?

I am sorry for my pom.xml that is not added ASF header. I've added.

wu-sheng · 2019-05-22T03:42:03Z

this tell me my own files may have not ASF header?

You could run mvn package locally, it should check the license header by using rat.

coveralls · 2019-05-22T04:03:05Z

Coverage increased (+0.1%) to 16.933% when pulling c2b8aecaf46d2c24f27fd854c0b689e7a96362c8 on wayilau:etcd_cluster into 48bca24 on apache:master.

coveralls · 2019-05-22T04:03:05Z

Coverage increased (+0.1%) to 17.373% when pulling dfd5724 on wayilau:etcd_cluster into dbb8bd8 on apache:master.

wayilau · 2019-05-22T05:30:27Z

this tell me my own files may have not ASF header?

You could run mvn package locally, it should check the license header by using rat.

resolved.

wu-sheng · 2019-05-23T01:48:54Z

You are still including UI changes in this pull request.

wayilau · 2019-05-23T02:00:28Z

You are still including UI changes in this pull request.

I will check this.

wayilau · 2019-05-23T02:19:08Z

You are still including UI changes in this pull request.

I will check this.

I need not merge the master into this branch, as it is not conflict with master. is that so ?

…d_cluster

JaredTan95

LGTM, etcd cluster plugin works. @wayilau Thanks for your contribution.

wayilau · 2019-06-14T08:02:35Z

@wu-sheng @JaredTan95 I have fix the ci. And I will open a new PR to achieve the configuration-etcd module. this pr only for server-cluster-etcd. And the etcd testcases also under "CI-with-IT".

wu-sheng · 2019-06-14T13:10:35Z

@JaredTan95 Please review. We don't have a good e2e test today, so even have IT test, I think you have to do local test again.

JaredTan95

Tested again, and going to merge this.
CC @wu-sheng

wu-sheng

Where is the etcd4j in LICENSE?

wu-sheng · 2019-06-17T02:12:39Z

...ain/java/org/apache/skywalking/oap/server/cluster/plugin/etcd/ClusterModuleEtcdProvider.java

+
+        List<URI> uris = EtcdUtils.parse(config);
+
+        //TODO check isSSL


SSL is going to be supported or not?

Where is the etcd4j in LICENSE?

Sorry for missing the etcd4j's license.

I think SSL should be added in a new pr.

wu-sheng · 2019-06-17T02:19:32Z

.../test/java/org/apache/skywalking/oap/server/cluster/plugin/etcd/ITClusterEtcdPluginTest.java

+    public void before() throws Exception {
+        etcdConfig = new ClusterModuleEtcdConfig();
+        etcdConfig.setServiceName(SERVICE_NAME);
+        client = new EtcdClient(URI.create("http://127.0.0.1:2379"));


I think this could cause CI failure randomly because of port conflict. Please take a good example from here, @kezhenxu94 's PR of IT test. cc @JaredTan95

skywalking/oap-server/server-configuration/configuration-apollo/src/test/java/org/apache/skywalking/oap/server/configuration/apollo/ITApolloConfigurationTest.java

Line 70 in fac940c

baseUrl = "http://" + host + ":" + port;

You mean that i should change the port to a parameter which given from the start running to avoid port conflict?

Apollo test failed.

Apollo test failure partly because there is no official image, and not because of this. Port need to be assigned randomly. Because in ASF Jenkins, we are sharing OS with other tasks of other ASF projects.

For Apollo, I have been working with @kezhenxu94 to check and fix it. But that is another thread somehow.

@wu-sheng I have make the host & port randomly as what you suggest. Also all test cases passed.

wu-sheng

LGTM

wu-sheng requested changes May 21, 2019

View reviewed changes

oap-server/server-cluster-plugin/cluster-etcd-plugin/pom.xml Outdated Show resolved Hide resolved

oap-server/server-cluster-plugin/cluster-etcd-plugin/pom.xml Outdated Show resolved Hide resolved

wu-sheng requested a review from peng-yongsheng May 21, 2019 06:43

wu-sheng added feature New feature backend OAP backend related. plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor. labels May 21, 2019

wayilau added 2 commits May 21, 2019 15:01

Add cluster-etcd-plugin

9b9f3a6

format code

57732ab

wayilau added 2 commits May 21, 2019 16:45

remove netty's dependency

92ef67f

add netty 4.1.27.Final.

25a9e3c

Add ASF header.

013bb47

Merge branch 'master' into etcd_cluster

46cd66c

wu-sheng requested a review from JaredTan95 May 24, 2019 13:58

JaredTan95 added 3 commits June 13, 2019 22:27

Merge branch 'master' into etcd_cluster

0fa2f7f

Merge branch 'etcd_cluster' of github.com:wayilau/skywalking into etc…

acd003c

…d_cluster

remove typo.

d1cdea2

JaredTan95 self-requested a review June 13, 2019 14:58

JaredTan95 previously approved these changes Jun 13, 2019

View reviewed changes

Fix configuration ci

5ee559a

wayilau dismissed JaredTan95’s stale review via 5ee559a June 14, 2019 02:29

wayilau and others added 5 commits June 14, 2019 11:21

Fix jackson conflict.

9aab648

Fix It etcd test cases.

a663e61

Merge branch 'master' into etcd_cluster

5fb257b

Fix IT test cases.

e1796b2

Merge branch 'master' into etcd_cluster

d87ddbf

JaredTan95 added 2 commits June 16, 2019 22:12

Merge branch 'master' into etcd_cluster

ea2488b

Merge branch 'master' into etcd_cluster

8ad036a

JaredTan95 self-requested a review June 17, 2019 01:51

JaredTan95 previously approved these changes Jun 17, 2019

View reviewed changes

wu-sheng reviewed Jun 17, 2019

View reviewed changes

Add etcd4j license.

13e6700

wayilau dismissed JaredTan95’s stale review via 13e6700 June 17, 2019 02:16

wu-sheng reviewed Jun 17, 2019

View reviewed changes

Fix port & host from system property.

1cc8e99

wu-sheng removed the TBD To be decided later, need more discussion or input. label Jun 17, 2019

wu-sheng added this to the 6.2.0 milestone Jun 17, 2019

wu-sheng approved these changes Jun 17, 2019

View reviewed changes

wu-sheng merged commit a70a2b2 into apache:master Jun 17, 2019

wayilau deleted the etcd_cluster branch June 17, 2019 07:51

wu-sheng mentioned this pull request Jul 1, 2019

[Revert]etcd configuration is not implemented #2974

Merged

Add cluster-etcd-plugin #2725

Add cluster-etcd-plugin #2725

Conversation

wayilau commented May 21, 2019

wu-sheng left a comment

Choose a reason for hiding this comment

wayilau commented May 21, 2019 • edited Loading

wayilau commented May 21, 2019

wu-sheng commented May 21, 2019

wayilau commented May 21, 2019

wu-sheng commented May 21, 2019

wayilau commented May 22, 2019 • edited Loading

wu-sheng commented May 22, 2019

wu-sheng commented May 22, 2019

wayilau commented May 22, 2019

wayilau commented May 22, 2019 • edited Loading

wayilau commented May 22, 2019

wu-sheng commented May 22, 2019

coveralls commented May 22, 2019

coveralls commented May 22, 2019 • edited Loading

wayilau commented May 22, 2019

wu-sheng commented May 23, 2019

wayilau commented May 23, 2019

wayilau commented May 23, 2019

JaredTan95 left a comment • edited Loading

Choose a reason for hiding this comment

wayilau commented Jun 14, 2019

wu-sheng commented Jun 14, 2019

JaredTan95 left a comment • edited Loading

Choose a reason for hiding this comment

wu-sheng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wayilau Jun 17, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wu-sheng left a comment

Choose a reason for hiding this comment

wayilau commented May 21, 2019 •

edited

Loading

wayilau commented May 22, 2019 •

edited

Loading

wayilau commented May 22, 2019 •

edited

Loading

coveralls commented May 22, 2019 •

edited

Loading

JaredTan95 left a comment •

edited

Loading

JaredTan95 left a comment •

edited

Loading

wayilau Jun 17, 2019 •

edited

Loading