Support data of configs and overrides from zookeeper registry. #100

SteNicholas · 2018-05-08T19:23:28Z

Motivation:

Zookeeper配置节点观察者未实现具体动态调整Provider/Consumer自定义配置逻辑

Modification:

Zookeeper配置节点观察者实现思路是Zookeeper注册中心修改订阅配置节点subscribeConfig方法，监听配置节点下子节点增加、子节点删除、子节点Data修改事件的实现即完善ZookeeperConfigObserver配置节点观察者实现，因为该配置节点观察者内部有configListenerMap绑定接口配置和配置监听者，所以实现updateConfig，updateConfigAll，removeConfig三个方法依赖子节点Data，之所以addConfig方法实现逻辑跟updateConfig方法完全一致，是因为ProviderAttributeListener，ConsumerAttributeListener更新属性逻辑决定的。另外removeConfig方法实现需要考虑删除子节点Data需要恢复接口级配置默认值。

Result:

Fixes #98 .
If there is no issue then describe the changes introduced by this PR.

codecov-io · 2018-05-08T19:55:42Z

Codecov Report

Merging #100 into master will decrease coverage by 0.77%.
The diff coverage is 21.78%.

@@             Coverage Diff              @@
##             master     #100      +/-   ##
============================================
- Coverage     65.85%   65.07%   -0.78%     
+ Complexity      648      647       -1     
============================================
  Files           270      271       +1     
  Lines         11407    11583     +176     
  Branches       1903     1951      +48     
============================================
+ Hits           7512     7538      +26     
- Misses         2933     3080     +147     
- Partials        962      965       +3

Impacted Files	Coverage Δ	Complexity Δ
.../sofa/rpc/registry/zk/ZookeeperConfigObserver.java	`15.71% <12%> (-13.7%)`	`0 <0> (ø)`
...ofa/rpc/registry/zk/ZookeeperOverrideObserver.java	`15.27% <15.27%> (ø)`	`0 <0> (?)`
.../sofa/rpc/registry/zk/ZookeeperRegistryHelper.java	`67.16% <2.77%> (-23.66%)`	`0 <0> (ø)`
...alipay/sofa/rpc/registry/zk/ZookeeperRegistry.java	`57.08% <59.09%> (+0.32%)`	`0 <0> (ø)`	⬇️
...sian/MultipleClassLoaderSofaSerializerFactory.java	`86.66% <0%> (-6.67%)`	`4% <0%> (-1%)`
...pay/sofa/rpc/transport/ClientTransportFactory.java	`60.6% <0%> (-4.55%)`	`0% <0%> (ø)`
...y/sofa/rpc/bootstrap/DefaultProviderBootstrap.java	`60.91% <0%> (-2.54%)`	`0% <0%> (ø)`
...ofa/rpc/registry/zk/ZookeeperProviderObserver.java	`68.51% <0%> (+1.85%)`	`0% <0%> (ø)`	⬇️
...ofa/rpc/registry/zk/AbstractZookeeperObserver.java	`100% <0%> (+25%)`	`0% <0%> (ø)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19cf47e...db84b44. Read the comment docs.

leizhiyuan · 2018-05-08T23:51:30Z

...-impl/registry-zk/src/main/java/com/alipay/sofa/rpc/registry/zk/ZookeeperRegistryHelper.java

+        String property = childData.getPath().substring(configPath.length() + 1);
+        //If event type is CHILD_REMOVED, attribute should return to default value
+        return Collections.singletonMap(property,
+            remove ? RpcConfigs.getStringValue(property) : new String(childData.getData()));


这个地方property 是一些服务相关的信息,比如 timeout,weight,其他自定义的动态属性这种 key
基本在这里
com.alipay.sofa.rpc.client.ProviderInfoAttrs 中,可以看下.
RpcConfigs 中是

// loadCustom loadCustom("sofa-rpc/rpc-config.json"); loadCustom("META-INF/sofa-rpc/rpc-config.json");

这两个文件中的一些默认配置,比如.
"consumer.connect.timeout": 3000

所以这时候这个取值会错误,因为配置文件中其实也是没有这个key的.

根据线下讨论，今天我尽快修复这块代码错误。

leizhiyuan · 2018-05-09T00:13:42Z

还有个问题,以com.alipay.sofa.rpc.bootstrap.DefaultProviderBootstrap为例

@Override
        public synchronized void attrUpdated(Map newValueMap) {
            String appName = providerConfig.getAppName();
            // 可以改变的配置 例如tag concurrents等
            Map<String, String> newValues = (Map<String, String>) newValueMap;
            Map<String, String> oldValues = new HashMap<String, String>();
            boolean reexport = false;

            // TODO 可能需要处理ServerConfig的配置变化
            try { // 检查是否有变化
                  // 是否过滤map?
                for (Map.Entry<String, String> entry : newValues.entrySet()) {
                    String newValue = entry.getValue();
                    String oldValue = providerConfig.queryAttribute(entry.getKey());
                    boolean changed = oldValue == null ? newValue != null : !oldValue.equals(newValue);
                    if (changed) {
                        oldValues.put(entry.getKey(), oldValue);
                    }
                    reexport = reexport || changed;
                }
            } catch (Exception e) {
                LOGGER.errorWithApp(appName, "Catch exception when provider attribute compare", e);
                return;
            }

如果用户删除了一个自定义配置,这情况下,newValues少了一个值,就不会被更新.这个不太符合期望

现在内存中对于ProviderConfig/ConsumerConfig 这种,是没有保存原始 weight信息的,所以删除动态值的时候,这些值没办法恢复成用户之前设置的值. @ujjboy

ujjboy · 2018-05-09T02:32:57Z

首先需要讨论下恢复默认配置是否合适？如果用户原来配置的不是默认值，那是不是反而会有不好的影响？

如果的确要恢复默认配置，建议直接new一个空Config对象获取里面的值，而不是自己去取参数？

SteNicholas · 2018-05-09T02:39:04Z

@ujjboy 那关于恢复默认配置的话，假如用户原来配置的不是默认值，当删除掉这条配置的时候意味着这条配置用户其实就想用系统提供的默认值吧？出于这点考虑所以通过RpcConfigs.getStringValue(property)去捞取系统提供的默认值。

leizhiyuan · 2018-05-09T02:55:14Z

@SteNicholas 不可以,默认值只是用户没有设置的情况下的默认值,用户可以通过 xml 等设置来改掉的.也就是他的初始值并不是默认值.恢复是要恢复到他的初始值.

SteNicholas · 2018-05-09T03:10:21Z

@leizhiyuan 好的，我清楚啦，那我今天修改下这块默认配置的取值逻辑。那关于恢复用户的初始值的话就像你说的内存中对于ProviderConfig/ConsumerConfig没有保存初始的配置，这个时候放ThreadLocal里面保存下可以吗？ThreadLocal里面如果拿不到初始值就用系统默认的取值。

leizhiyuan · 2018-05-09T03:11:24Z

@SteNicholas 稍等下,等我我补充一下这部分整体的流程说明.之后你再修改

SteNicholas · 2018-05-12T12:43:45Z

@leizhiyuan 由于之前混淆Ip级配置跟接口级配置导致convert的地方有问题，已按照你提供的流程增加override目录新增修改删除事件的监听观察者ZookeeperOverrideObserver，因为服务发布暂时不支持动态配置，暂时支持订阅ConsumerConfig参数设置缘故，故subscribeOverride方法只支持服务调用参数设置，即获取服务注册InterfaceConfig的地方暂时只支持ConsumerConfig类型。原本的ZookeeperConfigObserver保持不变，就是观察处理/config目录的事件变化。ZookeeperRegistryHelper类convertOverrideToAttribute方法提取属性的时候我根据convertProviderToUrls/convertConsumerToUrls得到ip级涉及到的属性变化的key筛选。请您帮忙Review这次修改，谢谢。

leizhiyuan · 2018-05-14T02:31:57Z

...-impl/registry-zk/src/main/java/com/alipay/sofa/rpc/registry/zk/ZookeeperRegistryHelper.java

+            return attributes;
+        }
+
+        for (ChildData childData : currentData) {


这个方法里,如果传进来的数据是两条,其中一条的 ip 是当前机器,转换的时候会怎么样

我觉得应该在这里过滤掉非本 ip 的配置,然后转换.这样传给 ConfigLisenter 的时候,ConfigLisenter就可以直接进行处理

嗯，我加一层ip校验过滤，只有本ip的才能交给ConfigListener处理，除了这些还有其他问题吗？

leizhiyuan · 2018-05-14T02:44:57Z

...-impl/registry-zk/src/main/java/com/alipay/sofa/rpc/registry/zk/ZookeeperConfigObserver.java

+                Map<String, String> attribute = ZookeeperRegistryHelper.convertConfigToAttribute(configPath, data,
+                    false);
+                for (ConfigListener listener : configListeners) {
+                    listener.attrUpdated(attribute);


这里改成调用 listener.ConfigChanged , attrUpdated 是给你添加的 override 用的.

好的，我这就修改下，因为configChanged我看重写的方法没具体实现所以就用attrUpdated了。

ujjboy · 2018-05-14T02:49:40Z

...on-impl/registry-zk/src/test/java/com/alipay/sofa/rpc/registry/zk/ZookeeperRegistryTest.java

 import com.alipay.sofa.rpc.listener.ProviderInfoListener;
 import com.alipay.sofa.rpc.registry.RegistryFactory;
+import com.google.common.collect.Maps;


用到guava的类的地方修改下，减少第三方强制依赖。

就直接用原生的JDK集合类是吧？

ujjboy · 2018-05-14T02:50:09Z

...-impl/registry-zk/src/main/java/com/alipay/sofa/rpc/registry/zk/ZookeeperRegistryHelper.java

 import com.alipay.sofa.rpc.config.AbstractInterfaceConfig;
 import com.alipay.sofa.rpc.config.ConsumerConfig;
 import com.alipay.sofa.rpc.config.ProviderConfig;
 import com.alipay.sofa.rpc.config.ServerConfig;
 import com.alipay.sofa.rpc.context.RpcRuntimeContext;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;


用到guava的类的地方修改下，减少第三方强制依赖。

ujjboy · 2018-05-14T02:51:04Z

...mpl/registry-zk/src/main/java/com/alipay/sofa/rpc/registry/zk/ZookeeperOverrideObserver.java

+                             AbstractInterfaceConfig registerConfig) throws Exception {
+        if (data == null) {
+            if (LOGGER.isInfoEnabled(config.getAppName())) {
+                LOGGER.info("data is null");


应该是之前代码问题，修改为 LOGGER.infoWithApp，类似的统一修改下。

就只有这个地方用了info方法，我看之前的代码就是这么打印log的，没敢改动，我改下。

SteNicholas · 2018-05-14T15:53:28Z

@ujjboy @leizhiyuan 按照你们Code Review地方修改完成,改动点包括1.convertOverrideToAttributes增加对当前机器IP校验,能够过滤掉非本IP配置转换属性传给ConfigLisenter处理;2.ZookeeperConfigObserver修改对ConfigListener调用,改成调用configChanged方法;3.ZookeeperRegistryHelper/ZookeeperRegistryTest改用原生JDK集合类，去掉guaua包第三方依赖;4.ZookeeperConfigObserver/ZookeeperOverrideObserver打印日志改成调用infoWithApp方法。请再次帮忙Review下，谢谢。

ujjboy · 2018-05-15T06:15:35Z

...-impl/registry-zk/src/main/java/com/alipay/sofa/rpc/registry/zk/ZookeeperConfigObserver.java

 /**
- * ZookeeperObserver for config node.
+ * ZookeeperObserver for override node,subscribe interface level provider/consumer config.


这里应该是 config node

ujjboy · 2018-05-15T06:17:25Z

...mpl/registry-zk/src/main/java/com/alipay/sofa/rpc/registry/zk/ZookeeperOverrideObserver.java

+     * @param currentData  子节点Data列表
+     */
+    public void updateConfigAll(AbstractInterfaceConfig config, String overridePath, List<ChildData> currentData)
+        throws UnsupportedEncodingException {


这里会抛这个异常吗？确认下，没有抛的话删掉，会抛的话，注释里加上 @throws

好的。我现在检查下

ujjboy · 2018-05-15T06:18:56Z

...mpl/registry-zk/src/main/java/com/alipay/sofa/rpc/registry/zk/ZookeeperOverrideObserver.java

+     * @param overridePath 覆盖Path
+     * @param data         子节点Data
+     */
+    public void updateConfig(AbstractInterfaceConfig config, String overridePath, ChildData data) throws Exception {


这里会抛出异常吗？确认下，没有抛的话删掉，会抛的话，注释里加上 @throws，我看有不少这样的地方，一起检查下。

SteNicholas · 2018-05-15T08:10:35Z

@ujjboy @throws注释已修改，麻烦您再看下是否还有问题，非常感谢。

SteNicholas added 2 commits May 9, 2018 03:12

添加ZookeeperConfigObserver功能

af6aa37

添加ZookeeperConfigObserver功能

26c5945

leizhiyuan reviewed May 8, 2018

View reviewed changes

SteNicholas added 12 commits May 12, 2018 16:30

添加ZookeeperConfigObserver配置观察功能

e955092

添加ZookeeperConfigObserver配置观察功能

a1ad9c0

添加ZookeeperConfigObserver配置观察功能

c48527e

添加ZookeeperConfigObserver配置观察功能

c8fe75a

添加ZookeeperConfigObserver配置观察功能

efd74f7

添加ZookeeperConfigObserver配置观察功能

0a01ab8

添加ZookeeperConfigObserver配置观察功能

be41225

添加ZookeeperConfigObserver配置观察功能

df279a1

添加ZookeeperConfigObserver配置观察功能

efda3f8

Merge branch 'master' into add_zookeeper_config_feature

83196d3

添加ZookeeperConfigObserver配置观察功能

4ad2a28

添加ZookeeperConfigObserver配置观察功能

8d52885

leizhiyuan reviewed May 14, 2018

View reviewed changes

ujjboy requested changes May 14, 2018

View reviewed changes

SteNicholas added 4 commits May 14, 2018 23:23

添加ZookeeperConfigObserver配置观察功能

ab0a5b4

添加ZookeeperConfigObserver配置观察功能

4816b12

添加ZookeeperConfigObserver配置观察功能

651d080

添加ZookeeperConfigObserver配置观察功能

6ff5342

ujjboy requested changes May 15, 2018

View reviewed changes

SteNicholas added 4 commits May 15, 2018 14:47

添加ZookeeperConfigObserver配置观察功能

9189ab8

添加ZookeeperConfigObserver配置观察功能

3f4fd72

添加ZookeeperConfigObserver配置观察功能

34509c0

添加ZookeeperConfigObserver配置观察功能

a182a5d

Merge branch 'master' into add_zookeeper_config_feature

db84b44

ujjboy approved these changes May 15, 2018

View reviewed changes

ujjboy merged commit 22b4bc3 into sofastack:master May 15, 2018

ujjboy changed the title ~~添加ZookeeperConfigObserver功能~~ Support data of configs and overrides from zookeeper registry. May 15, 2018

ujjboy added the enhancement New feature or request label May 15, 2018

ujjboy added this to the 5.4.0 milestone May 15, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support data of configs and overrides from zookeeper registry. #100

Support data of configs and overrides from zookeeper registry. #100

SteNicholas commented May 8, 2018

codecov-io commented May 8, 2018 •

edited

Loading

leizhiyuan May 8, 2018 •

edited

Loading

SteNicholas May 9, 2018

leizhiyuan commented May 9, 2018

ujjboy commented May 9, 2018

SteNicholas commented May 9, 2018

leizhiyuan commented May 9, 2018

SteNicholas commented May 9, 2018

leizhiyuan commented May 9, 2018

SteNicholas commented May 12, 2018

leizhiyuan May 14, 2018

SteNicholas May 14, 2018

leizhiyuan May 14, 2018

SteNicholas May 14, 2018

ujjboy May 14, 2018 •

edited

Loading

SteNicholas May 14, 2018

ujjboy May 14, 2018

ujjboy May 14, 2018 •

edited

Loading

ujjboy May 14, 2018

SteNicholas May 14, 2018

SteNicholas commented May 14, 2018

ujjboy May 15, 2018

ujjboy May 15, 2018

SteNicholas May 15, 2018

ujjboy May 15, 2018

SteNicholas commented May 15, 2018

Support data of configs and overrides from zookeeper registry. #100

Support data of configs and overrides from zookeeper registry. #100

Conversation

SteNicholas commented May 8, 2018

Motivation:

Modification:

Result:

codecov-io commented May 8, 2018 • edited Loading

Codecov Report

leizhiyuan May 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leizhiyuan commented May 9, 2018

ujjboy commented May 9, 2018

SteNicholas commented May 9, 2018

leizhiyuan commented May 9, 2018

SteNicholas commented May 9, 2018

leizhiyuan commented May 9, 2018

SteNicholas commented May 12, 2018

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

ujjboy May 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ujjboy May 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SteNicholas commented May 14, 2018

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

SteNicholas commented May 15, 2018

codecov-io commented May 8, 2018 •

edited

Loading

leizhiyuan May 8, 2018 •

edited

Loading

ujjboy May 14, 2018 •

edited

Loading

ujjboy May 14, 2018 •

edited

Loading