You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "xiaozaiyuji (via GitHub)" <gi...@apache.org> on 2023/03/18 10:38:09 UTC

[GitHub] [skywalking] xiaozaiyuji opened a new pull request, #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

xiaozaiyuji opened a new pull request, #10557:
URL: https://github.com/apache/skywalking/pull/10557

   <!--
       ⚠️ Please make sure to read this template first, pull requests that don't accord with this template
       maybe closed without notice.
       Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
       Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
   -->
   
   <!-- ==== 🐛 Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist 👇 ====
   ### Fix <bug description or the bug issue number or bug issue link>
   - [ ] Add a unit test to verify that the fix works.
   - [ ] Explain briefly why the bug exists and how to fix it.
        ==== 🐛 Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist 👆 ==== -->
   
   <!-- ==== 📈 Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist 👇 ====
   ### Improve the performance of <class or module or ...>
   - [ ] Add a benchmark for the improvement, refer to [the existing ones](https://github.com/apache/skywalking/blob/master/apm-commons/apm-datacarrier/src/test/java/org/apache/skywalking/apm/commons/datacarrier/LinkedArrayBenchmark.java)
   - [ ] The benchmark result.
   ```text
   <Paste the benchmark results here>
   ```
   - [ ] Links/URLs to the theory proof or discussion articles/blogs. <links/URLs here>
        ==== 📈 Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist 👆 ==== -->
   
   <!-- ==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👇 ====
   ### <Feature description>
   - [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
   - [ ] Update the documentation to include this new feature.
   - [ ] Tests(including UT, IT, E2E) are added to verify the new feature.
   - [ ] If it's UI related, attach the screenshots below.
        ==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👆 ==== -->
   
   - [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #<issue number>.
   - [ ] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/docs/en/changes/changes.md).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141981700


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   I did not just guessing, I am a coder,I think this problem is obviously to find by a test.I will do an other pull request and give the errorlog and the compare.Sorry for confusion you.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141018655


##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/query/MetadataQueryEsDAO.java:
##########
@@ -76,17 +78,19 @@ public MetadataQueryEsDAO(
         ElasticSearchClient client,
         StorageModuleElasticsearchConfig config) {
         super(client);
-        this.queryMaxSize = config.getMetadataQueryMaxSize();
+        this.metadataMaxSize = config.getMetadataQueryMaxSize();
+        this.resultWindowMaxSize = config.getResultWindowMaxSize();
         this.scrollingBatchSize = config.getScrollingBatchSize();
         this.layerSize = Layer.values().length;
+        this.queryMaxSize = Math.min(metadataMaxSize, resultWindowMaxSize);

Review Comment:
   If it is a hardcode case, then we could fix it. But AFAIK, all these are exposed to you. Make them right, rather than adding more complex logic to cause further confusing.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141014285


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   > Where is the approval of this new query API? I can't find this.
   
   I add the group and layer search condition,because the performance for oap server is very sufficient.In product,I as the user with 10000 agent and use the group classify the search result will be limit by the query max size with min(metadataSize and scrollsize) with 5000,but the scroll query will help the user to search as many as the user want service to to search by the agent. In order to let the user to find the service sufficient,I want to modify the code.
   1.I give the user to find the service by group and layer when the user have classify the service by group.
   2.I give the user to query the service as they want the number they want but not limit by the elasticsearch maxWindowSize.
   So I modify the word by change the two place,I think carefully and without to change the other,I think this will very flexible and will more sufficient.



##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   In product,If the user change the metadataSize 、scrollsize over the elasticserch maxWindowSize.  ,It also will be limit by  the elasticserch maxWindowSize and give an exception. So I change the limit logical to use the scrollSize and metadataSize .This change will not cause the exception.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng commented on pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#issuecomment-1474813530

   And what is the reason of ElasticSearch change?
   A pull request must include the explanation. One pull request is about one thing.
   Please be clear about the change, otherwise, we don't know what do you mean, and how could we follow?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141022924


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   This param resultWindowMaxSize is correspond the elasticsearch max limit size 10000. What I think is wrong?I am sorry for this.But this name is very confusion.
   But the aim of sroll query is to query data not limit by elasticserach  limit 10000, In fact,If I didnot to change this to 20000.The other query will cause an exception. So I split them and can help the scroll query can query 20000 result,and other not scroll query will also query the data they want by the elasticsearch index max result limit.
   Did I should define a othe static param?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#issuecomment-1474842976

   > And what is the reason of ElasticSearch change? A pull request must include the explanation. One pull request is about one thing. Please be clear about the change, otherwise, we don't know what do you mean, and how could we follow?
   
   Sorry,I will explain it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#issuecomment-1474854304

   > > And what is the reason of ElasticSearch change? A pull request must include the explanation. One pull request is about one thing. Please be clear about the change, otherwise, we don't know what do you mean, and how could we follow?
   > 
   > Sorry,I will explain it.
   
   
   
   > > And what is the reason of ElasticSearch change? A pull request must include the explanation. One pull request is about one thing. Please be clear about the change, otherwise, we don't know what do you mean, and how could we follow?
   > 
   > Sorry,I will explain it.
   
   The metadataSize will effect queryMaxSize and will effect other query and the sroll query.But when user not change the elasticsearch max result limit size.The sroll query will not cause an exception,but the other query will fail for the exception.
   So I split the two size.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#issuecomment-1474859258

   > I can't follow you are randomly replying on different threads. If you want to discuss it, do it in a more formal w
   
   > I can't follow you are randomly replying on different threads. If you want to discuss it, do it in a more formal way.
   
   I will change the pull request and submit it soon.Sorry for this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141012243


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   > And what is the reason of ElasticSearch change? A pull request must include the explanation. One pull request is about one thing. Please be clear about the change, otherwise, we don't know what do you mean, and how could we follow?
   
   Sorry this the first to pull request for open source ,I explain will explain it carefully.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141020340


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   > Where is the approval of this new query API? I can't find this.
   
   
   
   > > > And what is the reason of ElasticSearch change? A pull request must include the explanation. One pull request is about one thing. Please be clear about the change, otherwise, we don't know what do you mean, and how could we follow?
   > > 
   > > 
   > > Sorry this the first to pull request for open source ,I explain will explain it carefully.
   > 
   > You are replying on another context. Please the keep thread clear.
   
   1.This will help the query to service by group which will classify the service by group,which had many services.This will more sufficient and flexible.
   2.Split the metadataSize and resultWindowMaxSize and aviod an exception when the user did not change elasticserch max size limit. And will find the query result as many as the user want to search,without reduce performance。



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141018448


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   > > .In product,I as the user with 10000 agent and use the group classify the search result will be limit by the query max size with min(metadataSize and scrollsize) with 5000,
   > 
   > I can't follow this. 10k agents only mean 10k instances. Service must be much less than that.
   
   In product, the metrics-all index will generate the data by day ,the service with the same name is added,does I setting something wrong for oap server.I  query the elasticsearch for index metrics-all and find the data is that.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1140994045


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   Where is the approval of this new query API? I can't find this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141016315


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   In fact the skywalking ui include  many different agent, kubenetes,istio and many, but the user may by just use the java agent service or the python agent, so they will write a ui by they use,the new query API will helpfully for this.This will help them to query the service sufficient and flexible.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141019327


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   > > metadataSize 、scrollsize over the elasticserch maxWindowSize
   > 
   > All these are open for you to change. Why don't you change them by your own?
   
   I change the metadataSize 20000 but this param is also use by the other query ,this cause an exception.This is over the elasticserch limit 10000. I also try to change the elasticsearch maxWindowSize to 30000,but this is very low performance for the query,So I think to split the metadataSize  and maxWindowSize to by this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141018448


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   > > .In product,I as the user with 10000 agent and use the group classify the search result will be limit by the query max size with min(metadataSize and scrollsize) with 5000,
   > 
   > I can't follow this. 10k agents only mean 10k instances. Service must be much less than that.
   
   In product, the metrics-all index will generate the data by day ,the service with the same name is added,does I setting something wrong for oap server.I  query the elasticsearch for index metrics-all and find the data is that.
   In fact this will cause the service query will limit by the size,many service will can not the query ,the service will only show part of the service.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141016163


##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/query/MetadataQueryEsDAO.java:
##########
@@ -76,17 +78,19 @@ public MetadataQueryEsDAO(
         ElasticSearchClient client,
         StorageModuleElasticsearchConfig config) {
         super(client);
-        this.queryMaxSize = config.getMetadataQueryMaxSize();
+        this.metadataMaxSize = config.getMetadataQueryMaxSize();
+        this.resultWindowMaxSize = config.getResultWindowMaxSize();
         this.scrollingBatchSize = config.getScrollingBatchSize();
         this.layerSize = Layer.values().length;
+        this.queryMaxSize = Math.min(metadataMaxSize, resultWindowMaxSize);

Review Comment:
   Why do you have to run `min` between these two configurations? `metadataQueryMaxSize` is already open for your manual setup. I can't see your point of changing this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141018341


##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/query/MetadataQueryEsDAO.java:
##########
@@ -76,17 +78,19 @@ public MetadataQueryEsDAO(
         ElasticSearchClient client,
         StorageModuleElasticsearchConfig config) {
         super(client);
-        this.queryMaxSize = config.getMetadataQueryMaxSize();
+        this.metadataMaxSize = config.getMetadataQueryMaxSize();
+        this.resultWindowMaxSize = config.getResultWindowMaxSize();
         this.scrollingBatchSize = config.getScrollingBatchSize();
         this.layerSize = Layer.values().length;
+        this.queryMaxSize = Math.min(metadataMaxSize, resultWindowMaxSize);

Review Comment:
   > I change this because this is also limit by the elasticsearch resultWindowMaxSize
   
   Why don't you change the value to fix the expectation? The server setup matters too.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141019875


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   > In product, the metrics-all index will generate the data by day ,the service with the same name is added
   
   No, they are not. Service would be only added once. Check the data before guessing. That is not a fact.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141023217


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   Are you kidding? You are contributing to an open-source project, but understand a variable from the name rather than from codes.
   
   I am going to leave this PR, with no further steps. The conclusion is very clear, you even don't read the codes and debug the codes. Just guessing.
   
   I am out. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141017807


##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/query/MetadataQueryEsDAO.java:
##########
@@ -76,17 +78,19 @@ public MetadataQueryEsDAO(
         ElasticSearchClient client,
         StorageModuleElasticsearchConfig config) {
         super(client);
-        this.queryMaxSize = config.getMetadataQueryMaxSize();
+        this.metadataMaxSize = config.getMetadataQueryMaxSize();
+        this.resultWindowMaxSize = config.getResultWindowMaxSize();
         this.scrollingBatchSize = config.getScrollingBatchSize();
         this.layerSize = Layer.values().length;
+        this.queryMaxSize = Math.min(metadataMaxSize, resultWindowMaxSize);

Review Comment:
   I change this because this is also limit by the elasticsearch resultWindowMaxSize, if the metadataQueryMaxSize  setting by 20000 over the default limit by elasticserch it will cause an exception. The metadataQueryMaxSize is add to help the user to query  with elasticserch sroll query to avoid the limit 10000 of elasticserch.In order to fix this problem,I did this by not affect the other query,but support the  elasticserch sroll query, this will separate the queryMaxSize and metadataSize,when the metadataSize is litte than the resultWindowMaxSize, will query the data as the user setting, when the user setting the metadataSize over the resultWindowMaxSize, the elastic scroll query will give the metadataSize result as the user want, the other query will also limit by the resultWindowMaxSize and will not cause an exception.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141016315


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   In fact the skywalking ui include  many different agent, kubenetes,istio and many, but the user may by just use the java agent service or the python agent, so they will write a ui by they use,the new query API will helpfully for this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141016321


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   > metadataSize 、scrollsize over the elasticserch maxWindowSize
   
   All these are open for you to change. Why don't you change them by your own?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141016087


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   > .In product,I as the user with 10000 agent and use the group classify the search result will be limit by the query max size with min(metadataSize and scrollsize) with 5000,
   
   I can't follow this. 10k agents only mean 10k instances. Service must be much less than that. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141023276


##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/query/MetadataQueryEsDAO.java:
##########
@@ -76,17 +78,19 @@ public MetadataQueryEsDAO(
         ElasticSearchClient client,
         StorageModuleElasticsearchConfig config) {
         super(client);
-        this.queryMaxSize = config.getMetadataQueryMaxSize();
+        this.metadataMaxSize = config.getMetadataQueryMaxSize();
+        this.resultWindowMaxSize = config.getResultWindowMaxSize();
         this.scrollingBatchSize = config.getScrollingBatchSize();
         this.layerSize = Layer.values().length;
+        this.queryMaxSize = Math.min(metadataMaxSize, resultWindowMaxSize);

Review Comment:
   Thanks for dear wu,I just want to help skywalking to become better and offer an effort.I will remember that not to adding more complex logic and further confusion. Thanks a lot.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng commented on pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#issuecomment-1474855111

   I can't follow you are randomly replying on different threads. If you want to discuss it, do it in a more formal way. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141981700


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   I  am sorry for this.I will do it as you ask,I think what you see is right.As a coder,must to give the evidence carefully.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141016645


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   > In fact the skywalking ui include many different agent, kubenetes,istio and many, but the user may by just use the java agent service or the python agent, so they will write a ui by they use,the new query API wll helpfull for this.
   
   So what? I can't see in any cases, you are limited by this. Set them as the number you want , 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141016315


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   In fact the skywalking ui include  many different agent, kubenetes,istio and many, but the user may by just use the java agent service or the python agent, so they will write a ui by they use,the new query API wll helpfull for this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141020461


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   And your explanation doesn't make the sense even for your codes. 
   You added `resultWindowMaxSize`, but only `Math.min(metadataMaxSize, resultWindowMaxSize)`. Why just set a larger `metadataMaxSize`? `resultWindowMaxSize` isn't used in any place of this DAO. 
   Still in all other places, `queryMaxSize`(`metadataMaxSize`) is still used. What is the point? What is changed?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141020692


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   > 2.Split the metadataSize and resultWindowMaxSize and aviod an exception when the user did not change elasticserch max size limit. And will find the query result as many as the user want to search,without reduce performance。
   
   I don't know how you get this conclusion. There is no `resultWindowMaxSize` used in any place other than NetworkAddressAliasEsDAO. 
   
   <img width="1072" alt="image" src="https://user-images.githubusercontent.com/5441976/226109506-e70dcb62-865e-4770-8217-2d30ba174d1b.png">
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141018655


##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/query/MetadataQueryEsDAO.java:
##########
@@ -76,17 +78,19 @@ public MetadataQueryEsDAO(
         ElasticSearchClient client,
         StorageModuleElasticsearchConfig config) {
         super(client);
-        this.queryMaxSize = config.getMetadataQueryMaxSize();
+        this.metadataMaxSize = config.getMetadataQueryMaxSize();
+        this.resultWindowMaxSize = config.getResultWindowMaxSize();
         this.scrollingBatchSize = config.getScrollingBatchSize();
         this.layerSize = Layer.values().length;
+        this.queryMaxSize = Math.min(metadataMaxSize, resultWindowMaxSize);

Review Comment:
   If it is a hardcode case, then we could fix it. But AFAIK, all these are exposed to you. Make them right, rather than adding more complex logic to cause further confusion.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng closed pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng closed pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize
URL: https://github.com/apache/skywalking/pull/10557


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] wu-sheng commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141012557


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   > > And what is the reason of ElasticSearch change? A pull request must include the explanation. One pull request is about one thing. Please be clear about the change, otherwise, we don't know what do you mean, and how could we follow?
   > 
   > Sorry this the first to pull request for open source ,I explain will explain it carefully.
   
   You are replying on another context. Please the keep thread clear.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141012243


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   > And what is the reason of ElasticSearch change? A pull request must include the explanation. One pull request is about one thing. Please be clear about the change, otherwise, we don't know what do you mean, and how could we follow?
   
   Sorry this the first to pull request for open source ,I will explain it carefully.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141014285


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   > Where is the approval of this new query API? I can't find this.
   
   I add the group and layer search condition,because the performance for oap server is very sufficient.In product,I as the user with 10000 agent and use the group classify the search result will be limit by the query max size with min(metadataSize and scrollsize) with 5000,but the scroll query will help the user to search as many as the user want service to to serarch by the agent. In order to let the user to find the service sufficient,I want to modify the code.
   1.I give the user to find the service by group and layer when the user have classify the service by group.
   2.I give the user to query the service as they want the number they want but not limit by the elasticsearch maxWindowSize.
   So I modify the word by change the two place,I think carefully and without to change the other,I think this will very flexible and will more sufficient.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking] xiaozaiyuji commented on a diff in pull request #10557: modify the service query and query by group and layer and modify the elasticsearch scroll queryMaxSize

Posted by "xiaozaiyuji (via GitHub)" <gi...@apache.org>.
xiaozaiyuji commented on code in PR #10557:
URL: https://github.com/apache/skywalking/pull/10557#discussion_r1141015746


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/MetadataQueryV2.java:
##########
@@ -67,6 +67,10 @@ public List<Service> listServices(final String layer) throws IOException {
         return getMetadataQueryService().listServices(layer, null);
     }
 
+    public List<Service> listGroupServices(final String layer, final String group) throws IOException {

Review Comment:
   In product,If the user change the metadataSize 、scrollsize over the elasticserch maxWindowSize.  ,It also will be limit by  the elasticserch maxWindowSize and give an exception. So I change the limit logical to use the scrollSize and metadataSize .This change will not cause the exception.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org