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

naming selectInstances method doesn't subscribe when loadCacheAtStart=true #10585

Closed
nkorange opened this issue May 31, 2023 · 6 comments · Fixed by #10805
Closed

naming selectInstances method doesn't subscribe when loadCacheAtStart=true #10585

nkorange opened this issue May 31, 2023 · 6 comments · Fixed by #10805
Assignees
Labels
area/Client Related to Nacos Client SDK contribution welcome kind/bug Category issues or prs related to bug.
Milestone

Comments

@nkorange
Copy link
Collaborator

nkorange commented May 31, 2023

Describe the bug

naming selectInstances method doesn't subscribe when loadCacheAtStart=true

Expected behavior

naming selectInstances method does subscription when loadCacheAtStart=true

Actually behavior

naming selectInstances method doesn't subscribe when loadCacheAtStart=true

How to Reproduce
Steps to reproduce the behavior:

App1:

public class App1 {

    public static void main(String[] args) throws Exception {
        NamingService namingService = NamingFactory.createNamingService("127.0.0.1:8848");
        namingService.registerInstance("test.peterzhu.0", "2.2.2.2", 8080);
        Thread.sleep(100000000L);
    }
}

App2:

public class App2 {

    public static void main(String[] args) throws Exception {
        Properties properties = new Properties();
        properties.put("serverAddr", "127.0.0.1:8848");
        properties.put("namingLoadCacheAtStart", "true");
        NamingService namingService = NamingFactory.createNamingService(properties);
        namingService.registerInstance("test.peterzhu.0", "1.1.1.1", 8080);

        while (true) {
            System.out.println(namingService.selectInstances("test.peterzhu.0", true));
            Thread.sleep(3000L);
        }
    }
}
  1. Run App2
  2. Stop App2, and check the local cache file where there should be cached instance list containing 1.1.1.1.
  3. Run App2
  4. Run App1, you can find App2 always prints 1.1.1.1, without the new instance 2.2.2.2.

Desktop (please complete the following information):

  • OS: MacOS
  • Version Nacos Server 2.1.2, Nacos Client 2.2.2
  • Module naming
  • SDK original

Additional context

image

The bug is in selectInstances method that if local cache is not empty, the subscription would be skipped. Similar issue is also found in method selectOneHealthyInstance

@nkorange nkorange added the kind/bug Category issues or prs related to bug. label May 31, 2023
@KomachiSion KomachiSion added area/Client Related to Nacos Client SDK contribution welcome labels Jun 1, 2023
@KomachiSion KomachiSion added this to the 2.3.0 milestone Jun 1, 2023
@linzhihan
Copy link

i wanna take a look into this issue

@WindSearcher
Copy link

can i try it?

@KomachiSion
Copy link
Collaborator

Of cource, you can.

@WindSearcher
Copy link

I found that Mac OS can only reproduce the above situation in the same version environment, win11 not. I think the key is whether the service information changes are updated to the serviceInfoMap when the service instance is registered.

@stone-98
Copy link
Contributor

@WindSearcher 老哥,我再windows10复现出来了,这个和系统应该没关系,可以参考参考ISSUE#7550

Bo-Qiu added a commit to Bo-Qiu/nacos that referenced this issue Jul 16, 2023
…ods, if the parameter subscribe is true. Subscription is required when clientProxy.isSubscribe() is false.
@Bo-Qiu
Copy link
Contributor

Bo-Qiu commented Jul 16, 2023

Thank you all for the information provided. I happen to have some free time recently to deal with some issues, so I've submitted a PR to fix this problem.

KomachiSion pushed a commit that referenced this issue Jul 17, 2023
… the parameter subscribe is true. Subscription is required when clientProxy.isSubscribe() is false. (#10805)
realJackSun added a commit that referenced this issue Aug 10, 2023
…lop branch (#10942)

* [ISSUE #10734] Implement http request param check filter and http param extractors (#10758)

* For #10734,Implement grpc server interceptor and grpc param extractors

* For #10734,add unit test for grpc server interceptor and grpc param extractors

* For #10734,alter the test case

* For #10734,delete the ConnectionSetupRequestParamExtractor

* For #10734,add the naming http request param check filter and implement the naming http request param extractors

* For #10734,add unit test for naming http request param extractors

* For #10734,Implement grpc server interceptor and grpc param extractors

* For #10734,add unit test for grpc server interceptor and grpc param extractors

* For #10734,delete the ConnectionSetupRequestParamExtractor

* For #10734,add the naming http request param check filter and implement the naming http request param extractors

* For #10734,add unit test for naming http request param extractors

* For #10734,add the config http request param check filter and implement the config http request param extractors and unit test

* For #10734,add the console http request param check filter and implement the console http request param extractors and unit test

* For #10734,fix code style

* For #10734,alter the logic of exception handle in filter

* For #10734,fix code style

* dump change check task submit (#10755)

* dump change check task submit

* delete config nid convert error fix

* fix test case

* checkstyle

* Refactor grpc tls (#10759)

* Move Tls negotiator to GrpcSdkServer.

* use protocol negotiator builder replace directly create.

* use SPI load negotiator and set tls as default negotiator.

* Remove tlsconfig in BaseRpcServer.

* Add some ut.

* For checkstyle.

* fix word spelling in `AuthenticationManagerDelegator` (#10777)

* fix(#10427): When the execution of handleServerRequest() encounters an exception, record the log and throw an exception, then quickly response to the server errResponse (#10770)

* fix(#10585): selectInstances and selectOneHealthyInstance methods, if the parameter subscribe is true. Subscription is required when clientProxy.isSubscribe() is false. (#10805)

* disable check port input when use registered port (#10799)

* Add the handle to overload connection (#10783)

* add the handle to overload connection

* fast return

* [ISSUE #10662]Prometheus-sd add namespace and service api (#10663)

* add apis

* add tests

* fix checkstyle

* param namespace to namespaceId

* namespace to namespaceId

* fix test case

* fix test case

* Service instance should display related color when healthy or unhealthy (#10811)

* fix a couple of invaild props (#10810)

* feat(#10539): When the operation configuration fails, log. (#10804)

* [ISSUE #10744]feat:Add HealthControllerV2 and HealthControllerV2Test (#10786)

* [ISSUE #10744]feat:Add HealthControllerV2 and HealthControllerV2Test

* fix:V2 api return Result

* [ISSUE #10734] Refactor the ParamChecker and ParamExtractor (#10775)

* For #10734,refactor the paramextractor and ParamChecker

* For #10734,alter the rules of ParamCheck

* For #10734,alter the rules of ParamCheck

* For #10734,fix bug

* For #10734,fix bug and alter the ParamCheckRules.java

* For #10734,fix code style

* For #10734,fix the param check rules

* For #10734,implement the server param check config

* For #10734,optimize the logic

* For #10734,optimize the logic

* For #10734,optimize the logic

* Refactor Prometheus Module (#10827)

* Refactor Prometheus Module

* Complete Test Case

* format

* format

* For #10734,fix the param check rule (#10826)

* [ISSUE #10792]When nacos client use endpoint, the registration center should support configuring context-path and cluster-name like the configuration center (#10793)

* Reactor code in datasource-plugin (#10791)

* Reactor code in datasource-plugin

* Fix Abstract Mapper Test Case

* Add Empty Check

* Fix Checkstyle

* fix checkstyle

* fix check style

* fix check style

* Fix CheckStyle

* Fix SQL Blank

* bugfix for PersistentClientOperationServiceImpl log (#10825)

* For #10734,fix the param check rule (#10858)

* fix(#10831): When using the deregisterInstance method to remove one of multiple instances registered by batchRegisterInstance, all instances registered by batchRegisterInstance will be removed. (#10836)

* UnsupportedFeatureError (#10860)

* fix(distro): fix issue#10880. (#10881)

* feat(#5608 && #10223): When the custom instance id is empty, the id will be automatically generated. (#10812)

* fix: test-code branch (#10904)

* add nacos ci

* delete client version of nacos ci

* fix: test-code branch

* console-ui 新增 toml 语言支持,修复之前 properties 主题未生效的问题 (#10896)

* console-ui 新增 toml 语言支持,修复之前 properties 主题未生效的问题

* console-ui 新增 toml 语言支持,修复之前 properties 主题未生效的问题

* 老规矩,要编译一波

* feat(10891): Provide a configuration item for the maximum number of push retries, instead of directly hardcoding it to 50 times in the code. (#10895)

* [ISSUE #10824] Remove udp port param for v1-client (#10914)

* Remove UDP Param

* Fix gRPC client

* fix test case

* fix test case

* fix test case

* fix test case

* Fix login failed when close auth.

---------

Co-authored-by: Sunrisea <49605583+Sunrisea@users.noreply.github.com>
Co-authored-by: nov.lzf <liuzunfei@gmail.com>
Co-authored-by: 杨翊 SionYang <xiweng.yy@alibaba-inc.com>
Co-authored-by: ZhangShenao <15201440436@163.com>
Co-authored-by: blake.qiu <46370663+Bo-Qiu@users.noreply.github.com>
Co-authored-by: Joey777210 <53630996+Joey777210@users.noreply.github.com>
Co-authored-by: chenyiqin <83362909+Daydreamer-ia@users.noreply.github.com>
Co-authored-by: maoling <11016631+maoling@users.noreply.github.com>
Co-authored-by: DiligenceLai <96920907+DiligenceLai@users.noreply.github.com>
Co-authored-by: huhongjie2014 <139250842+huhongjie2014@users.noreply.github.com>
Co-authored-by: lu-xiaoshuang <121755080+lu-xiaoshuang@users.noreply.github.com>
Co-authored-by: zt9788 <zt9788@users.noreply.github.com>
Co-authored-by: 阿魁 <670569467@qq.com>
Co-authored-by: wuyfee <30968107+Wuyunfan-BUPT@users.noreply.github.com>
Co-authored-by: Darren Luo <jn296@qq.com>
Co-authored-by: xxc <xchaos8@126.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/Client Related to Nacos Client SDK contribution welcome kind/bug Category issues or prs related to bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants