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
Labels
kind/enhancement Category issues or prs related to enhancement.
Milestone

Comments

@horizonzy
Copy link
Collaborator

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

may be can use ConcurrentSkipListMap

@horizonzy
Copy link
Collaborator Author

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.

@chuntaojun
Copy link
Collaborator

chuntaojun commented Oct 27, 2020 via email

@horizonzy
Copy link
Collaborator Author

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"]

@KomachiSion KomachiSion added the kind/discussion Category issues related to discussion label Oct 27, 2020
@horizonzy
Copy link
Collaborator Author

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

@chuntaojun
Copy link
Collaborator

chuntaojun commented Oct 27, 2020 via email

@chuntaojun
Copy link
Collaborator

chuntaojun commented Oct 27, 2020 via email

@horizonzy
Copy link
Collaborator Author

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.

@horizonzy
Copy link
Collaborator Author

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.

@chuntaojun
Copy link
Collaborator

chuntaojun commented Oct 27, 2020 via email

@horizonzy
Copy link
Collaborator Author

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

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

* code format
@KomachiSion KomachiSion added kind/enhancement Category issues or prs related to enhancement. and removed kind/discussion Category issues related to discussion labels Nov 5, 2020
@KomachiSion KomachiSion added this to the 1.4.1 milestone Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Category issues or prs related to enhancement.
Projects
None yet
Development

No branches or pull requests

3 participants