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

com.alibaba.nacos.naming.controllers.ServiceController#list will out of order when serviceMap's value extent size #4066

Closed
horizonzy opened this issue Oct 26, 2020 · 11 comments
Assignees
Milestone

Comments

@horizonzy
Copy link
Collaborator

@horizonzy horizonzy commented Oct 26, 2020

Is your feature request related to a problem? Please describe.
From the openApi: curl -X GET '127.0.0.1:8848/nacos/v1/ns/service/list?pageNo=1&pageSize=2'.

    /**
     * Map(namespace, Map(group::serviceName, Service)).
     */
    private final Map<String, Map<String, Service>> serviceMap = new ConcurrentHashMap<>();

It can search service list, but when the serviceMap's value map extent size, the serviceMap's value map will out of order.
the request to server, same pageNo and pageSize, the result is different.

@chuntaojun
Copy link
Collaborator

@chuntaojun chuntaojun commented Oct 26, 2020

may be can use ConcurrentSkipListMap

Loading

@horizonzy
Copy link
Collaborator Author

@horizonzy horizonzy commented Oct 26, 2020

ConcurrentSkipListMap is not suit this case, the order should followed the order that it be added. the LinkedhashMap wrapped by SynchronizedMap is the choice, the degree of concurrency will not be high.

Loading

@chuntaojun
Copy link
Collaborator

@chuntaojun chuntaojun commented Oct 27, 2020

Loading

@horizonzy
Copy link
Collaborator Author

@horizonzy horizonzy commented Oct 27, 2020

Just make sure have a fixed order.

No, It's not enough. should follow the order that it be added, because we don't know the next service's position.
If add service as ["10","9","8","7","6","5","4","3","2","1","11","12"], the order should as ["10","9","8","7","6","5","4","3","2","1","11","12"], not ["1", "10", "11", "12", "2", "3", "4", "5", "6", "7", "8", "9"]

Loading

@horizonzy
Copy link
Collaborator Author

@horizonzy horizonzy commented Oct 27, 2020

If the service order followed it's order that it be added, user can feel the new servicesName just by add pageNo.

Loading

@chuntaojun
Copy link
Collaborator

@chuntaojun chuntaojun commented Oct 27, 2020

Loading

@chuntaojun
Copy link
Collaborator

@chuntaojun chuntaojun commented Oct 27, 2020

Loading

@horizonzy
Copy link
Collaborator Author

@horizonzy horizonzy commented Oct 27, 2020

Key can be a pair of objects, not necessarily a ServiceName, and what is the specific use difference between sorting and sorting with a fixed sort by the time of joining?

because the api searched by page, we suppose the page size is 10. Now server has service such as ['a','b','c','d','f','g','h','i','j','k'] 10 service without 'e'. Now I list service with param(pageNo = 1, pageSize = 10), the result is ['a','b','c','d','f','g','h','i','j','k'].And then add the service 'e'. request with param(pageNo = 1, pageSize = 10), the result is ['a','b','c','d','e','f','g','h','i','j'], request with param(pageNo = 2, pageSize = 10), the result is ['k'].
If user want to feel new service, the service order by the order that is's added, it's convenient.

Loading

@horizonzy
Copy link
Collaborator Author

@horizonzy horizonzy commented Oct 27, 2020

Sorting rules are diverse, and you can choose the simplest sort as the basic sort, and other functions can be implemented on the basis of the other functions.

yes, we also can use the timestamp that service be added to realize it.

Loading

@chuntaojun
Copy link
Collaborator

@chuntaojun chuntaojun commented Oct 27, 2020

Loading

@horizonzy
Copy link
Collaborator Author

@horizonzy horizonzy commented Oct 27, 2020

I think CurrentSkipListMap and custom comparator are fine. 发自我的iPhone 在 2020年10月27日,21:18,赵延 notifications@github.com 写道:  Sorting rules are diverse, and you can choose the simplest sort as the basic sort, and other functions can be implemented on the basis of the other functions. yes, we also can use the timestamp that service be added to realize it. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub<#4066 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AFS35NGBSHFG34K6G6YRJGDSM3CDDANCNFSM4S7F4QYA.

that's fine, next morning to solve it

Loading

KomachiSion pushed a commit that referenced this issue Nov 5, 2020
* make serviceNameList followed string order

* code format
@KomachiSion KomachiSion added this to the 1.4.1 milestone Nov 5, 2020
@KomachiSion KomachiSion closed this Nov 6, 2020
penghao03 added a commit to penghao03/nacos that referenced this issue Nov 14, 2020
…aba#4087)

* make serviceNameList followed string order

* code format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants