You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2020/02/27 15:01:11 UTC
[GitHub] [rocketmq] Cczzzz opened a new pull request #1801: add client
custom nameserver TopAddressing
Cczzzz opened a new pull request #1801: add client custom nameserver TopAddressing
URL: https://github.com/apache/rocketmq/pull/1801
I change TopAddressing to interface.
```
public interface TopAddressing {
String fetchNSAddr();
void registerChangeCallBack(Consumer<String> changeCallBack);
}
```
Uesr can ues ServiceLoader to loading custom TopAddressing implementation. When there is a custom implementation,Custom implementations are preferred.
```
private TopAddressing findCustomTopAddressing() {
ServiceLoader<TopAddressing> serviceLoader = ServiceLoader.load(TopAddressing.class);
Iterator<TopAddressing> iterator = serviceLoader.iterator();
if (iterator.hasNext()) {
return iterator.next();
}
return null;
}
```
I also provide a way to call back.this is a methods in interface TopAddressing. MQClientAPIImpl will register call back function to perception changes
```
this.topAddressing.registerChangeCallBack(this::fetchNameServerAddr);
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] lizhanhui commented on issue #1801: add client custom
nameserver TopAddressing
Posted by GitBox <gi...@apache.org>.
lizhanhui commented on issue #1801: add client custom nameserver TopAddressing
URL: https://github.com/apache/rocketmq/pull/1801#issuecomment-592276805
A few issues in this pull request.
1. Please read [how to contribue](http://rocketmq.apache.org/docs/how-to-contribute/) page. In particular, PR should be made on top of "develop" branch; Code style should be fixed; Client related code modules are assumed to be Java 1.6 compatible.
2. TopAddressing.java misses copyright info;
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] lizhanhui commented on issue #1801: add client custom
nameserver TopAddressing
Posted by GitBox <gi...@apache.org>.
lizhanhui commented on issue #1801: add client custom nameserver TopAddressing
URL: https://github.com/apache/rocketmq/pull/1801#issuecomment-592282484
> @Cczzzz something wrong about ci `/home/travis/build/apache/rocketmq/common/src/main/java/org/apache/rocketmq/common/namesrv/TopAddressing.java:1: error: Line does not match expected header line of '/\*'.`
See issue #2 of my previous comment.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] lizhanhui commented on issue #1801: add client custom
nameserver TopAddressing
Posted by GitBox <gi...@apache.org>.
lizhanhui commented on issue #1801: add client custom nameserver TopAddressing
URL: https://github.com/apache/rocketmq/pull/1801#issuecomment-592284513
#findCustomTopAddressing, Should we keep a chain of implementations instead of the first one? Sort them according to some criteria, say an annotation @Order(index).
In method #fetchNSAddr, call the impls in order until one impl returns address list that makes sense.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] Cczzzz commented on a change in pull request #1801: add
client custom nameserver TopAddressing
Posted by GitBox <gi...@apache.org>.
Cczzzz commented on a change in pull request #1801: add client custom nameserver TopAddressing
URL: https://github.com/apache/rocketmq/pull/1801#discussion_r385172242
##########
File path: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java
##########
@@ -212,20 +213,25 @@ public RemotingClient getRemotingClient() {
public String fetchNameServerAddr() {
try {
String addrs = this.topAddressing.fetchNSAddr();
- if (addrs != null) {
- if (!addrs.equals(this.nameSrvAddr)) {
- log.info("name server address changed, old=" + this.nameSrvAddr + ", new=" + addrs);
- this.updateNameServerAddressList(addrs);
- this.nameSrvAddr = addrs;
- return nameSrvAddr;
- }
- }
+ return fetchNameServerAddr(addrs);
} catch (Exception e) {
log.error("fetchNameServerAddr Exception", e);
}
return nameSrvAddr;
}
+ public String fetchNameServerAddr(String addrs){
Review comment:
I'm going to split up the method so it's easy to call
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] ShannonDing commented on issue #1801: add client custom
nameserver TopAddressing
Posted by GitBox <gi...@apache.org>.
ShannonDing commented on issue #1801: add client custom nameserver TopAddressing
URL: https://github.com/apache/rocketmq/pull/1801#issuecomment-606012260
Could ypu pls move the PR to the develop branch? @Cczzzz
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] lizhanhui commented on issue #1801: add client custom
nameserver TopAddressing
Posted by GitBox <gi...@apache.org>.
lizhanhui commented on issue #1801: add client custom nameserver TopAddressing
URL: https://github.com/apache/rocketmq/pull/1801#issuecomment-592282905
By the way, I prefer to rename TopAddressingImpl to DefaultTopAddressing because it provides a default implementation if no customized ones are loaded.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] Cczzzz commented on issue #1801: add client custom
nameserver TopAddressing
Posted by GitBox <gi...@apache.org>.
Cczzzz commented on issue #1801: add client custom nameserver TopAddressing
URL: https://github.com/apache/rocketmq/pull/1801#issuecomment-592390487
This is my first time to submit the request, thank you, he, it has a lot of problems and I will modify it.
@lizhanhui Your suggestion is very good
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] xujianhai666 commented on issue #1801: add client custom
nameserver TopAddressing
Posted by GitBox <gi...@apache.org>.
xujianhai666 commented on issue #1801: add client custom nameserver TopAddressing
URL: https://github.com/apache/rocketmq/pull/1801#issuecomment-592276844
@Cczzzz something wrong about ci
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] xujianhai666 edited a comment on issue #1801: add client
custom nameserver TopAddressing
Posted by GitBox <gi...@apache.org>.
xujianhai666 edited a comment on issue #1801: add client custom nameserver TopAddressing
URL: https://github.com/apache/rocketmq/pull/1801#issuecomment-592276844
@Cczzzz something wrong about ci `/home/travis/build/apache/rocketmq/common/src/main/java/org/apache/rocketmq/common/namesrv/TopAddressing.java:1: error: Line does not match expected header line of '/\*'.`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] xujianhai666 commented on issue #1801: add client custom
nameserver TopAddressing
Posted by GitBox <gi...@apache.org>.
xujianhai666 commented on issue #1801: add client custom nameserver TopAddressing
URL: https://github.com/apache/rocketmq/pull/1801#issuecomment-592269937
@Cczzzz Is there any issue about this pr?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] Cczzzz commented on a change in pull request #1801: add
client custom nameserver TopAddressing
Posted by GitBox <gi...@apache.org>.
Cczzzz commented on a change in pull request #1801: add client custom nameserver TopAddressing
URL: https://github.com/apache/rocketmq/pull/1801#discussion_r385172817
##########
File path: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java
##########
@@ -174,16 +174,17 @@
}
private final RemotingClient remotingClient;
- private final TopAddressing topAddressing;
+ private final TopAddressingImpl topAddressing;
private final ClientRemotingProcessor clientRemotingProcessor;
- private String nameSrvAddr = null;
+ private volatile String nameSrvAddr = null;
Review comment:
It could be another thread in the callback function
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] xujianhai666 commented on a change in pull request
#1801: add client custom nameserver TopAddressing
Posted by GitBox <gi...@apache.org>.
xujianhai666 commented on a change in pull request #1801: add client custom nameserver TopAddressing
URL: https://github.com/apache/rocketmq/pull/1801#discussion_r385475431
##########
File path: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java
##########
@@ -215,20 +216,25 @@ public RemotingClient getRemotingClient() {
public String fetchNameServerAddr() {
try {
String addrs = this.topAddressing.fetchNSAddr();
- if (addrs != null) {
- if (!addrs.equals(this.nameSrvAddr)) {
- log.info("name server address changed, old=" + this.nameSrvAddr + ", new=" + addrs);
- this.updateNameServerAddressList(addrs);
- this.nameSrvAddr = addrs;
- return nameSrvAddr;
- }
- }
+ return fetchNameServerAddr(addrs);
} catch (Exception e) {
log.error("fetchNameServerAddr Exception", e);
}
return nameSrvAddr;
}
+ public String fetchNameServerAddr(String addrs){
+ if (addrs != null) {
+ if (!addrs.equals(this.nameSrvAddr)) {
Review comment:
why not rename method `onNameServerChange` ? because is just deal with nameserver change and then update if necessary
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] xujianhai666 commented on a change in pull request
#1801: add client custom nameserver TopAddressing
Posted by GitBox <gi...@apache.org>.
xujianhai666 commented on a change in pull request #1801: add client custom nameserver TopAddressing
URL: https://github.com/apache/rocketmq/pull/1801#discussion_r385474451
##########
File path: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java
##########
@@ -215,20 +216,25 @@ public RemotingClient getRemotingClient() {
public String fetchNameServerAddr() {
try {
String addrs = this.topAddressing.fetchNSAddr();
- if (addrs != null) {
- if (!addrs.equals(this.nameSrvAddr)) {
- log.info("name server address changed, old=" + this.nameSrvAddr + ", new=" + addrs);
- this.updateNameServerAddressList(addrs);
- this.nameSrvAddr = addrs;
- return nameSrvAddr;
- }
- }
+ return fetchNameServerAddr(addrs);
} catch (Exception e) {
log.error("fetchNameServerAddr Exception", e);
}
return nameSrvAddr;
}
+ public String fetchNameServerAddr(String addrs){
+ if (addrs != null) {
+ if (!addrs.equals(this.nameSrvAddr)) {
Review comment:
maybe we could combine two statements info one.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services