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

startConfigCenter时,不会使用自己扩展的ZookeeperTransports #4511

Closed
2 tasks done
CodingSinger opened this issue Jul 9, 2019 · 4 comments
Closed
2 tasks done

Comments

@CodingSinger
Copy link
Member

CodingSinger commented Jul 9, 2019

  • I have searched the issues of this repository and believe that this is not a duplicate.
  • I have checked the FAQ of this repository and believe that this is not a duplicate.

Environment

  • Dubbo version: 2.7.2
  • Operating System version: macOS 10.12.6
  • Java version: 1.8

Steps to reproduce this issue

Pls. provide [GitHub address] to reproduce this issue.

Expected Result

What do you expected from the above steps?

    ZookeeperDynamicConfiguration(URL url, ZookeeperTransporter zookeeperTransporter) {
        this.url = url;
        rootPath = "/" + url.getParameter(CONFIG_NAMESPACE_KEY, DEFAULT_GROUP) + "/config";

        initializedLatch = new CountDownLatch(1);
        this.cacheListener = new CacheListener(rootPath, initializedLatch);
        this.executor = Executors.newFixedThreadPool(1, new NamedThreadFactory(this.getClass().getSimpleName(), true));

        zkClient = zookeeperTransporter.connect(url);
        zkClient.addDataListener(rootPath, cacheListener, executor);
        try {
            // Wait for connection
            this.initializedLatch.await();
        } catch (InterruptedException e) {
            logger.warn("Failed to build local cache for config center (zookeeper)." + url);
        }
    }

zkClient = zookeeperTransporter.connect(url);会通过URL中的client参数来正确选择自适应扩展来初始化zk客户端。但在初始化配置中心的时候,URL是由ConfigCenterConfig得到的,此时URL中无client的信息,所以这个地方创建zk还是用的默认的zkClient。希望和注册中心时候一样获取自己扩展的zkClient

Actual Result

What actually happens?

If there is an exception, please attach the exception trace:

Just put your stack trace here!
@CodingSinger
Copy link
Member Author

CodingSinger commented Jul 9, 2019

需要在

   registries.stream().filter(RegistryConfig::isZookeeperProtocol).findFirst().ifPresent(rc -> {
            // we use the loading status of DynamicConfiguration to decide whether ConfigCenter has been initiated.
            Environment.getInstance().getDynamicConfiguration().orElseGet(() -> {
                ConfigManager configManager = ConfigManager.getInstance();
                ConfigCenterConfig cc = configManager.getConfigCenter().orElse(new ConfigCenterConfig());
                cc.setProtocol(rc.getProtocol());
                cc.setAddress(rc.getAddress());
                cc.setHighestPriority(false);
                cc.setClient(rc.getClient)); // 改进
                setConfigCenter(cc);
                startConfigCenter();
                return null;
            });
        });

ConfigCenterConfig中应该增加client属性,以将zk的ZookeeperTransporter方式传递给ConfigCenterConfig,保证此处zk客户端初始化和ZookeeperRegistry初始化zk客户端的方式一样。

@lexburner
Copy link
Contributor

看起来是 configcenter 加载 ZookeeperTransporter 的逻辑跟 registry 那块的逻辑不一致,没有根据 client 字段去加载 SPI 扩展。有兴趣的话可以提交一个 pr 帮忙修复一下

@CodingSinger
Copy link
Member Author

好的,我试试提pr

CodingSinger added a commit to CodingSinger/dubbo that referenced this issue Jul 9, 2019
CodingSinger added a commit to CodingSinger/dubbo that referenced this issue Jul 9, 2019
CodingSinger added a commit to CodingSinger/dubbo that referenced this issue Jul 10, 2019
CodingSinger added a commit to CodingSinger/dubbo that referenced this issue Jul 11, 2019
CodingSinger added a commit to CodingSinger/dubbo that referenced this issue Jul 11, 2019
lexburner pushed a commit that referenced this issue Jul 15, 2019
CodingSinger added a commit to CodingSinger/dubbo that referenced this issue Jul 18, 2019
CodingSinger added a commit to CodingSinger/dubbo that referenced this issue Jul 20, 2019
CodingSinger added a commit to CodingSinger/dubbo that referenced this issue Jul 20, 2019
@chickenlj
Copy link
Contributor

Fixed by #4513

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

No branches or pull requests

3 participants