You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/02/12 05:27:25 UTC
[GitHub] [skywalking] aderm opened a new pull request #4353: Optimizing ES query performance reduces the scope of index queries th…
aderm opened a new pull request #4353: Optimizing ES query performance reduces the scope of index queries th…
URL: https://github.com/apache/skywalking/pull/4353
…rough precise queries
Please answer these questions before submitting pull request
- Why submit this pull request?
- [ ] Bug fix
- [ ] New feature provided
- [x] Improve performance
- Related issues
#4194
___
### Bug fix
- Bug description.
- How to fix?
___
### New feature or improvement
- Describe the details and related test reports.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request
#4353: Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379286512
##########
File path: oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/EsModelName.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.oap.server.storage.plugin.elasticsearch.base;
+
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import org.apache.skywalking.oap.server.core.analysis.Downsampling;
+import org.apache.skywalking.oap.server.core.query.DurationUtils;
+import org.apache.skywalking.oap.server.core.register.EndpointInventory;
+import org.apache.skywalking.oap.server.core.register.NetworkAddressInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.storage.model.ModelName;
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
+
+public class EsModelName extends ModelName {
+
+ private static final DateTimeFormatter YYYYMM = DateTimeFormat.forPattern("yyyyMM");
+ private static final DateTimeFormatter YYYYMMDD = DateTimeFormat.forPattern("yyyyMMdd");
+
+ public static final List<String> WHITE_INDEX_LIST = new ArrayList<String>() {
+ {
+ add(EndpointInventory.INDEX_NAME);
+ add(NetworkAddressInventory.INDEX_NAME);
+ add(ServiceInventory.INDEX_NAME);
+ add(ServiceInstanceInventory.INDEX_NAME);
+ }
+ };
Review comment:
This is not a good practice to initialize a list, it creates an unnecessary inner class. Based on your usage, please consider make it immutable by `com.google.common.collect.Sets#newHashSet(E...)`, because you're always using the `contains` method to determine the existence, `Set` should be faster IMHO
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378110934
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -291,6 +292,30 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB
return client.search(searchRequest);
}
+ public SearchResponse search(String[] indexNames, SearchSourceBuilder searchSourceBuilder) throws IOException {
Review comment:
Also relate to `String[] indexNames`, should we keep all in List and covert to array at last minute?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm edited a comment on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586860632
Option two. In the production operation and maintenance scene, it may be necessary to query the slow segments of a few days or even many tens days ago. When the number of instances and endpoints and concurrency are large, it is still necessary to quickly output results slow trace. In our producing, the data volume is still relatively large, sampling is configured, and the TTL time is only three day, but many developments need to find dozens of days ago, and it is impossible to get it.
#4364 #4368 both about reduce the number of indexes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on issue #4353: Optimizing performance
reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586719463
@wu-sheng agree with your suggestion
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378636611
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -291,6 +292,30 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB
return client.search(searchRequest);
}
+ public SearchResponse search(String[] indexNames, SearchSourceBuilder searchSourceBuilder) throws IOException {
+ String[] fullIndexNames = formatIndexNames(indexNames);
+ SearchRequest searchRequest = new SearchRequest(fullIndexNames);
+ searchRequest.types(TYPE);
+ searchRequest.source(searchSourceBuilder);
+ return client.search(searchRequest);
+ }
+
+ public String[] filterNotExistIndex(String[] fullIndexNames, String indName) throws IOException {
+ // if no wrap, it is impossible to remove elements
+ List<String> indexNameList = new ArrayList<>(Arrays.asList(fullIndexNames));
+ if (fullIndexNames.length > 0) {
+ List<String> existIndex = retrievalIndexByAliases(indName);
+ indexNameList.removeIf(indexName -> {
+ //only filter index name with xxxx-xxxx
+ if (indexName.contains("-")) {
Review comment:
server-core module depends on library-client module, so `Const#LINE` in server-core can not used in library-client module.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586151116
Before doing code level deep dive, please read my comments, we should set the target clear about this optimization.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378223460
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -291,6 +292,30 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB
return client.search(searchRequest);
}
+ public SearchResponse search(String[] indexNames, SearchSourceBuilder searchSourceBuilder) throws IOException {
+ String[] fullIndexNames = formatIndexNames(indexNames);
+ SearchRequest searchRequest = new SearchRequest(fullIndexNames);
+ searchRequest.types(TYPE);
+ searchRequest.source(searchSourceBuilder);
+ return client.search(searchRequest);
+ }
+
+ public String[] filterNotExistIndex(String[] fullIndexNames, String indName) throws IOException {
+ // if no wrap, it is impossible to remove elements
+ List<String> indexNameList = new ArrayList<>(Arrays.asList(fullIndexNames));
+ if (fullIndexNames.length > 0) {
+ List<String> existIndex = retrievalIndexByAliases(indName);
+ indexNameList.removeIf(indexName -> {
+ //only filter index name with xxxx-xxxx
+ if (indexName.contains("-")) {
Review comment:
Use `Const#LINE`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378257166
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -291,6 +292,30 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB
return client.search(searchRequest);
}
+ public SearchResponse search(String[] indexNames, SearchSourceBuilder searchSourceBuilder) throws IOException {
+ String[] fullIndexNames = formatIndexNames(indexNames);
+ SearchRequest searchRequest = new SearchRequest(fullIndexNames);
+ searchRequest.types(TYPE);
+ searchRequest.source(searchSourceBuilder);
+ return client.search(searchRequest);
+ }
+
+ public String[] filterNotExistIndex(String[] fullIndexNames, String indName) throws IOException {
+ // if no wrap, it is impossible to remove elements
+ List<String> indexNameList = new ArrayList<>(Arrays.asList(fullIndexNames));
+ if (fullIndexNames.length > 0) {
+ List<String> existIndex = retrievalIndexByAliases(indName);
+ indexNameList.removeIf(indexName -> {
+ //only filter index name with xxxx-xxxx
+ if (indexName.contains("-")) {
Review comment:
ACK
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on issue #4353: Optimizing performance
reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-589914761
@wu-sheng @kezhenxu94 `queryBasicTraces` query index of segment is located according to the date range, PTAL.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378083903
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java
##########
@@ -213,4 +213,36 @@ private DateTime parseToDateTime(Downsampling downsampling, long time) {
}
throw new UnexpectedException("Unexpected downsampling: " + downsampling.name());
}
+
+ public DateTime startTimeBucket2DateTime(Downsampling downsampling, long startTB) {
Review comment:
Indeed, it maybe faster to convert to long like 202002. When the timebucket scope in loop statement add one day or month will trouble. Or maybe convert to timestamps，comparing and adding will fast，but also has to convert to Date string.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on issue #4353: Optimizing performance
reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586860632
Option one
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379504306
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java
##########
@@ -213,4 +213,36 @@ private DateTime parseToDateTime(Downsampling downsampling, long time) {
}
throw new UnexpectedException("Unexpected downsampling: " + downsampling.name());
}
+
+ public DateTime startTimeBucket2DateTime(Downsampling downsampling, long startTB) {
+ switch (downsampling) {
+ case Month:
+ return YYYYMM.parseDateTime(String.valueOf(startTB));
+ case Day:
+ return YYYYMMDD.parseDateTime(String.valueOf(startTB));
+ case Hour:
+ return YYYYMMDDHH.parseDateTime(String.valueOf(startTB));
+ case Minute:
+ return YYYYMMDDHHMM.parseDateTime(String.valueOf(startTB));
+ case Second:
+ return YYYYMMDDHHMMSS.parseDateTime(String.valueOf(startTB));
+ }
+ throw new UnexpectedException("Unexpected downsampling: " + downsampling.name());
+ }
+
+ public DateTime endTimeBucket2DateTime(Downsampling downsampling, long endTB) {
+ switch (downsampling) {
+ case Month:
+ return YYYYMM.parseDateTime(String.valueOf(endTB));
+ case Day:
+ return YYYYMMDD.parseDateTime(String.valueOf(endTB));
+ case Hour:
+ return YYYYMMDDHH.parseDateTime(String.valueOf(endTB));
+ case Minute:
+ return YYYYMMDDHHMM.parseDateTime(String.valueOf(endTB));
+ case Second:
+ return YYYYMMDDHHMMSS.parseDateTime(String.valueOf(endTB));
+ }
+ throw new UnexpectedException("Unexpected downsampling: " + downsampling.name());
Review comment:
ack, for the day and month, different circulation methods are used. Considering that the monthly increase calculation should be time-consuming, it is still the original method. WDYT?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586718264
As an opposite direction of my previous two PRs about reducing the index number, this runs into another way. We have two choices AFAIK
1. Close this for now, or leave it for now.
1. Make this chance only affecting the trace query. Because it seems the only optimization we need. And because of that, it will only related to Segment Query DAOs. But still remain the question, we don't require time conditions in traceId query.
Personally, I prefer to don't chance, even we have put much energy on this. @apache/skywalking-committers please feedback what do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379381981
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -403,4 +421,11 @@ public String formatIndexName(String indexName) {
}
return indexName;
}
+
+ public List<String> formatIndexNames(List<String> indexNameList) {
+ if (StringUtil.isNotEmpty(namespace)) {
+ indexNameList.stream().map(indexName -> namespace + "_" + indexName);
Review comment:
oops, ack
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378091521
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/storage/model/ModelName.java
##########
@@ -30,10 +43,73 @@ public static String build(Downsampling downsampling, String modelName) {
return modelName + Const.ID_SPLIT + Downsampling.Day.getName();
case Hour:
return modelName + Const.ID_SPLIT + Downsampling.Hour.getName();
- // case Second:
- // return modelName + Const.ID_SPLIT + Downsampling.Second.getName();
default:
return modelName;
}
}
+
+ public static String[] build(Downsampling downsampling, String modelName, long startTB, long endTB) {
+ List<String> indexNameList = new ArrayList<>();
+
+ List<String> whiteIndexList = new ArrayList<String>() {
Review comment:
This should be a static list, rather than create everytime.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm edited a comment on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586900265
> Option 2 has the condition issue. I have no idea how to fix it.
condition issue can more detail？ From the front debug, the parameters such as `startTimeBucket endTimeBucket step` are passed, but the step param， current interface `queryBasicTraces ` is not used.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm edited a comment on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586860632
Option two. In the production operation and maintenance scene, it may be necessary to query the slow segments of a few days or even many tens days ago. When the number of instances and endpoints and concurrency are large, it is still necessary to quickly output results slow trace. In our producing, the data volume is still relatively large, sampling is configured, and the TTL time is only three day, but many developments need to find dozens of days ago, and it is impossible to get it, before In the pressure test environment, trace queries were still stuttered.
#4364 #4368 both about reduce the number of indexes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm edited a comment on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586144859
@wu-sheng Worried about the problem of error in log( `index_not_found` exception ), I checked the relevant documents. Found that ES has configuration parameters that can be set.
Refer this:[[https://www.elastic.co/guide/en/elasticsearch/reference/current/multi-index.html](url)](url)
- gap time near 00:00
- multi OAP node
- product error log
The above mentioned scenarios are all fine.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on issue #4353: Optimizing performance
reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586900265
> Option 2 has the condition issue. I have no idea how to fix it.
condition can more detail？ From the front debug, the parameters such as `startTimeBucket endTimeBucket step` are passed, but the step param， current interface `queryBasicTraces ` is not used.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379368436
##########
File path: oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/EsModelName.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.oap.server.storage.plugin.elasticsearch.base;
+
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import org.apache.skywalking.oap.server.core.analysis.Downsampling;
+import org.apache.skywalking.oap.server.core.query.DurationUtils;
+import org.apache.skywalking.oap.server.core.register.EndpointInventory;
+import org.apache.skywalking.oap.server.core.register.NetworkAddressInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.storage.model.ModelName;
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
+
+public class EsModelName extends ModelName {
+
+ private static final DateTimeFormatter YYYYMM = DateTimeFormat.forPattern("yyyyMM");
+ private static final DateTimeFormatter YYYYMMDD = DateTimeFormat.forPattern("yyyyMMdd");
+
+ public static final List<String> WHITE_INDEX_LIST = new ArrayList<String>() {
+ {
+ add(EndpointInventory.INDEX_NAME);
+ add(NetworkAddressInventory.INDEX_NAME);
+ add(ServiceInventory.INDEX_NAME);
+ add(ServiceInstanceInventory.INDEX_NAME);
+ }
+ };
+
+ public static List<String> build(Downsampling downsampling, String modelName, long startTB, long endTB) {
+ if (WHITE_INDEX_LIST.contains(modelName)) {
+ return new ArrayList<String>() {
+ {
+ add(modelName);
+ }
+ };
+ }
+
+ List<String> indexNameList = new LinkedList<>();
+ DateTime startDT = DurationUtils.INSTANCE.startTimeBucket2DateTime(downsampling, startTB);
+ DateTime endDT = DurationUtils.INSTANCE.endTimeBucket2DateTime(downsampling, endTB);
+ if (endDT.isAfter(startDT)) {
+ switch (downsampling) {
+ case Month:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMM.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusMonths(1);
+ }
+ break;
+ case Day:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Hour:
+ //current hour index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Minute:
+ //current minute index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Second:
+ //current second index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
Review comment:
ACK
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586657434
Hi @aderm , I want to hold this PR a little longer. The index calculation will face a little more challenge.
In PR #4364, I provide a new feature to reduce the index number, and I am thinking to provide TTL step to reduce the index number again, especially for metrics indexes.
@apache/skywalking-committers WHDT? To @hanahmily and @kezhenxu94, above is one being considered optimization of index number.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378636611
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -291,6 +292,30 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB
return client.search(searchRequest);
}
+ public SearchResponse search(String[] indexNames, SearchSourceBuilder searchSourceBuilder) throws IOException {
+ String[] fullIndexNames = formatIndexNames(indexNames);
+ SearchRequest searchRequest = new SearchRequest(fullIndexNames);
+ searchRequest.types(TYPE);
+ searchRequest.source(searchSourceBuilder);
+ return client.search(searchRequest);
+ }
+
+ public String[] filterNotExistIndex(String[] fullIndexNames, String indName) throws IOException {
+ // if no wrap, it is impossible to remove elements
+ List<String> indexNameList = new ArrayList<>(Arrays.asList(fullIndexNames));
+ if (fullIndexNames.length > 0) {
+ List<String> existIndex = retrievalIndexByAliases(indName);
+ indexNameList.removeIf(indexName -> {
+ //only filter index name with xxxx-xxxx
+ if (indexName.contains("-")) {
Review comment:
server-core module depends on server-client module, so `Const#LINE` can not used in server-client module.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586720338
Which one? There are two above
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on issue #4353: Optimizing performance
reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-590067901
attention to this, other modifications can be removed. There is a suggestion. For trace query, add a configuration parameter. It is not enabled by default and is enabled when needed. WDYT?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378255741
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/storage/model/ModelName.java
##########
@@ -30,10 +43,73 @@ public static String build(Downsampling downsampling, String modelName) {
return modelName + Const.ID_SPLIT + Downsampling.Day.getName();
case Hour:
return modelName + Const.ID_SPLIT + Downsampling.Hour.getName();
- // case Second:
- // return modelName + Const.ID_SPLIT + Downsampling.Second.getName();
default:
return modelName;
}
}
+
+ public static String[] build(Downsampling downsampling, String modelName, long startTB, long endTB) {
+ List<String> indexNameList = new ArrayList<>();
+
+ List<String> whiteIndexList = new ArrayList<String>() {
+ {
+ add(EndpointInventory.INDEX_NAME);
+ add(NetworkAddressInventory.INDEX_NAME);
+ add(ServiceInventory.INDEX_NAME);
+ add(ServiceInstanceInventory.INDEX_NAME);
+ }
+ };
+
+ if (whiteIndexList.contains(modelName)) {
+ return new String[] { modelName };
+ }
+
+ DateTime startDT = DurationUtils.INSTANCE.startTimeBucket2DateTime(downsampling, startTB);
+ DateTime endDT = DurationUtils.INSTANCE.endTimeBucket2DateTime(downsampling, endTB);
+ if (endDT.isAfter(startDT)) {
+ switch (downsampling) {
+ case Month:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMM.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusMonths(1);
+ }
+ break;
+ case Day:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Hour:
+ //current hour index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Minute:
+ //current minute index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Second:
+ //current second index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ default:
+ indexNameList.add(modelName);
+ }
+ }
+ return indexNameList.toArray(new String[0]);
Review comment:
> You covert to array at the first place, but then you covert to List again in `#filterNotExistIndex`. Seems unnecessary?
Oh, indeed i will fix it. Convert at last minitue.
> But based on the things you talked, I think you should establish a cache to `filterNotExistIndex` and cache the result for at least a day, considering the index could only be deleted day by day, right? Set up a guava cache with `#expireAfterWrite`?
Good advice, i will try.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request
#4353: Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379290950
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -291,6 +291,24 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB
return client.search(searchRequest);
}
+ /**
+ * Search results from ES search engine according to various search conditions,
+ * Note the method is usered for the list of index names is optimized based on
+ * the scope of startTimeBucket and endTimeBucket
+ * @param indexNameList full index names list base on timebucket scope.
+ * Except for endpoint_inventory network_address_inventory service_inventory service_instance_inventory
+ * @param searchSourceBuilder Various search query conditions
+ * @return ES search query results
+ * @throws IOException throw IOException
Review comment:
not a good java doc: `throw IOException`, better to describe **WHEN** the exception is thrown
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378198260
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -291,6 +292,30 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB
return client.search(searchRequest);
}
+ public SearchResponse search(String[] indexNames, SearchSourceBuilder searchSourceBuilder) throws IOException {
Review comment:
good idea
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586150036
`allow_no_indices` and `ignore_unavailable` should be good choices. Could you try to use these? And check whether there is error log in OAP and ES server?
Here is the report from the original issue author, in Chinese,
[skywalking查询优化基准测试.pdf](https://github.com/apache/skywalking/files/4203428/skywalking.pdf)
This only shows the necessity to support this in the segment query, also, it requires a change in the UI.
@kezhenxu94 @zhaoyuguang @JaredTan95 @wayilau @aderm @arugal Did any of you face this issue in the production environment? Should we continue on this PR? Or tag this as TBD later?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378094991
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/storage/model/ModelName.java
##########
@@ -30,10 +43,73 @@ public static String build(Downsampling downsampling, String modelName) {
return modelName + Const.ID_SPLIT + Downsampling.Day.getName();
case Hour:
return modelName + Const.ID_SPLIT + Downsampling.Hour.getName();
- // case Second:
- // return modelName + Const.ID_SPLIT + Downsampling.Second.getName();
default:
return modelName;
}
}
+
+ public static String[] build(Downsampling downsampling, String modelName, long startTB, long endTB) {
+ List<String> indexNameList = new ArrayList<>();
+
+ List<String> whiteIndexList = new ArrayList<String>() {
+ {
+ add(EndpointInventory.INDEX_NAME);
+ add(NetworkAddressInventory.INDEX_NAME);
+ add(ServiceInventory.INDEX_NAME);
+ add(ServiceInstanceInventory.INDEX_NAME);
+ }
+ };
+
+ if (whiteIndexList.contains(modelName)) {
+ return new String[] { modelName };
+ }
+
+ DateTime startDT = DurationUtils.INSTANCE.startTimeBucket2DateTime(downsampling, startTB);
+ DateTime endDT = DurationUtils.INSTANCE.endTimeBucket2DateTime(downsampling, endTB);
+ if (endDT.isAfter(startDT)) {
+ switch (downsampling) {
+ case Month:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMM.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusMonths(1);
+ }
+ break;
+ case Day:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Hour:
+ //current hour index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Minute:
+ //current minute index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Second:
+ //current second index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ default:
+ indexNameList.add(modelName);
+ }
+ }
+ return indexNameList.toArray(new String[0]);
Review comment:
You covert to array at the first place, but then you covert to List again in `#filterNotExistIndex`. Seems unnecessary?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-589928033
> condition issue can more detail？
When we do query based on trace_id, don't need time duration.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm edited a comment on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586144859
@wu-sheng Worried about the problem of error in log( `index_not_found` exception ), I checked the relevant documents. Found that ES has configuration parameters that can be set.
Refer this:
[https://www.elastic.co/guide/en/elasticsearch/reference/current/multi-index.html](url)
- gap time near 00:00
- multi OAP node
- product error log
The above mentioned scenarios are all fine.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm edited a comment on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586860632
Option one. In my option，Based on the two existing optimizations above, the performance problem should be basically solved. #4368 User experience should be better.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-590049834
@aderm Do we still need to keep this PR continued? This PR still seems changing a lot, but the report shows only trace query maybe need optimization for very few users.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm edited a comment on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586860632
Option one: leave it for now. In my option, performance should still have some room for optimization if necessary. #4364 #4368 both about reduce the number of indexes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378255741
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/storage/model/ModelName.java
##########
@@ -30,10 +43,73 @@ public static String build(Downsampling downsampling, String modelName) {
return modelName + Const.ID_SPLIT + Downsampling.Day.getName();
case Hour:
return modelName + Const.ID_SPLIT + Downsampling.Hour.getName();
- // case Second:
- // return modelName + Const.ID_SPLIT + Downsampling.Second.getName();
default:
return modelName;
}
}
+
+ public static String[] build(Downsampling downsampling, String modelName, long startTB, long endTB) {
+ List<String> indexNameList = new ArrayList<>();
+
+ List<String> whiteIndexList = new ArrayList<String>() {
+ {
+ add(EndpointInventory.INDEX_NAME);
+ add(NetworkAddressInventory.INDEX_NAME);
+ add(ServiceInventory.INDEX_NAME);
+ add(ServiceInstanceInventory.INDEX_NAME);
+ }
+ };
+
+ if (whiteIndexList.contains(modelName)) {
+ return new String[] { modelName };
+ }
+
+ DateTime startDT = DurationUtils.INSTANCE.startTimeBucket2DateTime(downsampling, startTB);
+ DateTime endDT = DurationUtils.INSTANCE.endTimeBucket2DateTime(downsampling, endTB);
+ if (endDT.isAfter(startDT)) {
+ switch (downsampling) {
+ case Month:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMM.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusMonths(1);
+ }
+ break;
+ case Day:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Hour:
+ //current hour index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Minute:
+ //current minute index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Second:
+ //current second index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ default:
+ indexNameList.add(modelName);
+ }
+ }
+ return indexNameList.toArray(new String[0]);
Review comment:
> Did you answer the question in the wrong context? I am not saying `filterNotExistIndex` is useless.
Oh, Let me perfect it. Convert at last minitue.
> But based on the things you talked, I think you should establish a cache to `filterNotExistIndex` and cache the result for at least a day, considering the index could only be deleted day by day, right? Set up a guava cache with `#expireAfterWrite`?
Good advice, i will try.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request
#4353: Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379290165
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -403,4 +421,11 @@ public String formatIndexName(String indexName) {
}
return indexName;
}
+
+ public List<String> formatIndexNames(List<String> indexNameList) {
+ if (StringUtil.isNotEmpty(namespace)) {
+ indexNameList.stream().map(indexName -> namespace + "_" + indexName);
Review comment:
Bug here, you map the `indexNameList` but ignore the result, the `indexNameList` itself is unchanged
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-585785562
> It will have correct result query again without affecting subsequent queries.
This may be an issue in the production environment. The error log outputs will trigger the alarm system, which is monitoring SkyWalking. We need to avoid them permanently, I suppose.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378089483
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java
##########
@@ -213,4 +213,36 @@ private DateTime parseToDateTime(Downsampling downsampling, long time) {
}
throw new UnexpectedException("Unexpected downsampling: " + downsampling.name());
}
+
+ public DateTime startTimeBucket2DateTime(Downsampling downsampling, long startTB) {
Review comment:
When crossing months, you need to consider the big and small months. In addition, you need to consider some leap years. like `20200227-20200305`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378094387
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -291,6 +292,30 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB
return client.search(searchRequest);
}
+ public SearchResponse search(String[] indexNames, SearchSourceBuilder searchSourceBuilder) throws IOException {
+ String[] fullIndexNames = formatIndexNames(indexNames);
+ SearchRequest searchRequest = new SearchRequest(fullIndexNames);
+ searchRequest.types(TYPE);
+ searchRequest.source(searchSourceBuilder);
+ return client.search(searchRequest);
+ }
+
+ public String[] filterNotExistIndex(String[] fullIndexNames, String indName) throws IOException {
+ // if no wrap, it is impossible to remove elements
+ List<String> indexNameList = new ArrayList<>(Arrays.asList(fullIndexNames));
+ if (fullIndexNames.length > 0) {
+ List<String> existIndex = retrievalIndexByAliases(indName);
+ indexNameList.removeIf(indexName -> {
+ //only filter index name with xxxx-xxxx
+ if (indexName.contains("-")) {
Review comment:
Which one doesn't contain `-`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379373679
##########
File path: oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/EsModelName.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.oap.server.storage.plugin.elasticsearch.base;
+
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import org.apache.skywalking.oap.server.core.analysis.Downsampling;
+import org.apache.skywalking.oap.server.core.query.DurationUtils;
+import org.apache.skywalking.oap.server.core.register.EndpointInventory;
+import org.apache.skywalking.oap.server.core.register.NetworkAddressInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.storage.model.ModelName;
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
+
+public class EsModelName extends ModelName {
+
+ private static final DateTimeFormatter YYYYMM = DateTimeFormat.forPattern("yyyyMM");
+ private static final DateTimeFormatter YYYYMMDD = DateTimeFormat.forPattern("yyyyMMdd");
+
+ public static final List<String> WHITE_INDEX_LIST = new ArrayList<String>() {
+ {
+ add(EndpointInventory.INDEX_NAME);
+ add(NetworkAddressInventory.INDEX_NAME);
+ add(ServiceInventory.INDEX_NAME);
+ add(ServiceInstanceInventory.INDEX_NAME);
+ }
+ };
+
+ public static List<String> build(Downsampling downsampling, String modelName, long startTB, long endTB) {
+ if (WHITE_INDEX_LIST.contains(modelName)) {
+ return new ArrayList<String>() {
+ {
+ add(modelName);
+ }
+ };
Review comment:
ack
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on issue #4353: Optimizing performance
reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-590070746
agree
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378636611
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -291,6 +292,30 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB
return client.search(searchRequest);
}
+ public SearchResponse search(String[] indexNames, SearchSourceBuilder searchSourceBuilder) throws IOException {
+ String[] fullIndexNames = formatIndexNames(indexNames);
+ SearchRequest searchRequest = new SearchRequest(fullIndexNames);
+ searchRequest.types(TYPE);
+ searchRequest.source(searchSourceBuilder);
+ return client.search(searchRequest);
+ }
+
+ public String[] filterNotExistIndex(String[] fullIndexNames, String indName) throws IOException {
+ // if no wrap, it is impossible to remove elements
+ List<String> indexNameList = new ArrayList<>(Arrays.asList(fullIndexNames));
+ if (fullIndexNames.length > 0) {
+ List<String> existIndex = retrievalIndexByAliases(indName);
+ indexNameList.removeIf(indexName -> {
+ //only filter index name with xxxx-xxxx
+ if (indexName.contains("-")) {
Review comment:
server-core depends on server-client, so `Const#LINE` can not used in server-client module.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379375965
##########
File path: oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/EsModelName.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.oap.server.storage.plugin.elasticsearch.base;
+
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import org.apache.skywalking.oap.server.core.analysis.Downsampling;
+import org.apache.skywalking.oap.server.core.query.DurationUtils;
+import org.apache.skywalking.oap.server.core.register.EndpointInventory;
+import org.apache.skywalking.oap.server.core.register.NetworkAddressInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.storage.model.ModelName;
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
+
+public class EsModelName extends ModelName {
+
+ private static final DateTimeFormatter YYYYMM = DateTimeFormat.forPattern("yyyyMM");
+ private static final DateTimeFormatter YYYYMMDD = DateTimeFormat.forPattern("yyyyMMdd");
+
+ public static final List<String> WHITE_INDEX_LIST = new ArrayList<String>() {
+ {
+ add(EndpointInventory.INDEX_NAME);
+ add(NetworkAddressInventory.INDEX_NAME);
+ add(ServiceInventory.INDEX_NAME);
+ add(ServiceInstanceInventory.INDEX_NAME);
+ }
+ };
Review comment:
ack
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request
#4353: Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379285619
##########
File path: oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/EsModelName.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.oap.server.storage.plugin.elasticsearch.base;
+
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import org.apache.skywalking.oap.server.core.analysis.Downsampling;
+import org.apache.skywalking.oap.server.core.query.DurationUtils;
+import org.apache.skywalking.oap.server.core.register.EndpointInventory;
+import org.apache.skywalking.oap.server.core.register.NetworkAddressInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.storage.model.ModelName;
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
+
+public class EsModelName extends ModelName {
+
+ private static final DateTimeFormatter YYYYMM = DateTimeFormat.forPattern("yyyyMM");
+ private static final DateTimeFormatter YYYYMMDD = DateTimeFormat.forPattern("yyyyMMdd");
+
+ public static final List<String> WHITE_INDEX_LIST = new ArrayList<String>() {
+ {
+ add(EndpointInventory.INDEX_NAME);
+ add(NetworkAddressInventory.INDEX_NAME);
+ add(ServiceInventory.INDEX_NAME);
+ add(ServiceInstanceInventory.INDEX_NAME);
+ }
+ };
+
+ public static List<String> build(Downsampling downsampling, String modelName, long startTB, long endTB) {
+ if (WHITE_INDEX_LIST.contains(modelName)) {
+ return new ArrayList<String>() {
+ {
+ add(modelName);
+ }
+ };
+ }
+
+ List<String> indexNameList = new LinkedList<>();
+ DateTime startDT = DurationUtils.INSTANCE.startTimeBucket2DateTime(downsampling, startTB);
+ DateTime endDT = DurationUtils.INSTANCE.endTimeBucket2DateTime(downsampling, endTB);
+ if (endDT.isAfter(startDT)) {
+ switch (downsampling) {
+ case Month:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMM.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusMonths(1);
+ }
+ break;
+ case Day:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Hour:
+ //current hour index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Minute:
+ //current minute index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Second:
+ //current second index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
Review comment:
These can be merged into the `case Day` path, to reduce code duplicates
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io commented on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-585057389
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=h1) Report
> Merging [#4353](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/efed74a79888f9bd72dd6ef6f4b5a6e6dfb2bf77?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4353 +/- ##
=======================================
Coverage 26.19% 26.19%
=======================================
Files 1177 1177
Lines 26821 26821
Branches 3703 3703
=======================================
Hits 7026 7026
Misses 19178 19178
Partials 617 617
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=footer). Last update [efed74a...8fe5532](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378225818
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/storage/model/ModelName.java
##########
@@ -30,10 +43,73 @@ public static String build(Downsampling downsampling, String modelName) {
return modelName + Const.ID_SPLIT + Downsampling.Day.getName();
case Hour:
return modelName + Const.ID_SPLIT + Downsampling.Hour.getName();
- // case Second:
- // return modelName + Const.ID_SPLIT + Downsampling.Second.getName();
default:
return modelName;
}
}
+
+ public static String[] build(Downsampling downsampling, String modelName, long startTB, long endTB) {
+ List<String> indexNameList = new ArrayList<>();
+
+ List<String> whiteIndexList = new ArrayList<String>() {
+ {
+ add(EndpointInventory.INDEX_NAME);
+ add(NetworkAddressInventory.INDEX_NAME);
+ add(ServiceInventory.INDEX_NAME);
+ add(ServiceInstanceInventory.INDEX_NAME);
+ }
+ };
+
+ if (whiteIndexList.contains(modelName)) {
+ return new String[] { modelName };
+ }
+
+ DateTime startDT = DurationUtils.INSTANCE.startTimeBucket2DateTime(downsampling, startTB);
+ DateTime endDT = DurationUtils.INSTANCE.endTimeBucket2DateTime(downsampling, endTB);
+ if (endDT.isAfter(startDT)) {
+ switch (downsampling) {
+ case Month:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMM.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusMonths(1);
+ }
+ break;
+ case Day:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Hour:
+ //current hour index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Minute:
+ //current minute index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Second:
+ //current second index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ default:
+ indexNameList.add(modelName);
+ }
+ }
+ return indexNameList.toArray(new String[0]);
Review comment:
Did you answer the question in the wrong context? I am not saying `filterNotExistIndex` is useless.
But based on the things you talked, I think you should establish a cache to `filterNotExistIndex` and cache the result for at least a day, considering the index could only be deleted day by day, right? Set up a guava cache with `#expireAfterWrite`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378636611
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -291,6 +292,30 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB
return client.search(searchRequest);
}
+ public SearchResponse search(String[] indexNames, SearchSourceBuilder searchSourceBuilder) throws IOException {
+ String[] fullIndexNames = formatIndexNames(indexNames);
+ SearchRequest searchRequest = new SearchRequest(fullIndexNames);
+ searchRequest.types(TYPE);
+ searchRequest.source(searchSourceBuilder);
+ return client.search(searchRequest);
+ }
+
+ public String[] filterNotExistIndex(String[] fullIndexNames, String indName) throws IOException {
+ // if no wrap, it is impossible to remove elements
+ List<String> indexNameList = new ArrayList<>(Arrays.asList(fullIndexNames));
+ if (fullIndexNames.length > 0) {
+ List<String> existIndex = retrievalIndexByAliases(indName);
+ indexNameList.removeIf(indexName -> {
+ //only filter index name with xxxx-xxxx
+ if (indexName.contains("-")) {
Review comment:
server-core module depends on server-client module, so `Const#LINE` in server-core can not used in server-client module.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378093441
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -291,6 +292,30 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB
return client.search(searchRequest);
}
+ public SearchResponse search(String[] indexNames, SearchSourceBuilder searchSourceBuilder) throws IOException {
Review comment:
Add comments for these methods.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586895445
Option 2 has the condition issue. I have no idea how to fix 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378083903
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java
##########
@@ -213,4 +213,36 @@ private DateTime parseToDateTime(Downsampling downsampling, long time) {
}
throw new UnexpectedException("Unexpected downsampling: " + downsampling.name());
}
+
+ public DateTime startTimeBucket2DateTime(Downsampling downsampling, long startTB) {
Review comment:
Indeed, it maybe faster to convert to long like 202002. When the timebucket scope in loop statement add one day or month will trouble.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request
#4353: Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379287530
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java
##########
@@ -213,4 +213,36 @@ private DateTime parseToDateTime(Downsampling downsampling, long time) {
}
throw new UnexpectedException("Unexpected downsampling: " + downsampling.name());
}
+
+ public DateTime startTimeBucket2DateTime(Downsampling downsampling, long startTB) {
+ switch (downsampling) {
+ case Month:
+ return YYYYMM.parseDateTime(String.valueOf(startTB));
+ case Day:
+ return YYYYMMDD.parseDateTime(String.valueOf(startTB));
+ case Hour:
+ return YYYYMMDDHH.parseDateTime(String.valueOf(startTB));
+ case Minute:
+ return YYYYMMDDHHMM.parseDateTime(String.valueOf(startTB));
+ case Second:
+ return YYYYMMDDHHMMSS.parseDateTime(String.valueOf(startTB));
+ }
+ throw new UnexpectedException("Unexpected downsampling: " + downsampling.name());
+ }
+
+ public DateTime endTimeBucket2DateTime(Downsampling downsampling, long endTB) {
+ switch (downsampling) {
+ case Month:
+ return YYYYMM.parseDateTime(String.valueOf(endTB));
+ case Day:
+ return YYYYMMDD.parseDateTime(String.valueOf(endTB));
+ case Hour:
+ return YYYYMMDDHH.parseDateTime(String.valueOf(endTB));
+ case Minute:
+ return YYYYMMDDHHMM.parseDateTime(String.valueOf(endTB));
+ case Second:
+ return YYYYMMDDHHMMSS.parseDateTime(String.valueOf(endTB));
+ }
+ throw new UnexpectedException("Unexpected downsampling: " + downsampling.name());
Review comment:
The logic seems not to be related to `startTime` or `endTime`, so I don't think it's a good method name `endTimeBucket2DateTime ` or `startTimeBucket2DateTime `, and the method body are exactly the same in `parseToDateTime`, if you meant it, simply use `parseToDateTime` is enough? Otherwise, you may forget to `.plusMonths(1)`, `plusDays(1)`, etc. like `org.apache.skywalking.oap.server.core.query.DurationUtils#endTimeToTimestamp`
```java
public long endTimeToTimestamp(Step step, String dateStr) {
switch (step) {
case MONTH:
return YYYY_MM.parseDateTime(dateStr).plusMonths(1).getMillis();
case DAY:
return YYYY_MM_DD.parseDateTime(dateStr).plusDays(1).getMillis();
case HOUR:
return YYYY_MM_DD_HH.parseDateTime(dateStr).plusHours(1).getMillis();
case MINUTE:
return YYYY_MM_DD_HHMM.parseDateTime(dateStr).plusMinutes(1).getMillis();
case SECOND:
return YYYY_MM_DD_HHMMSS.parseDateTime(dateStr).plusSeconds(1).getMillis();
}
throw new UnexpectedException("Unsupported step " + step.name());
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on issue #4353: Optimizing performance
reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-585783014
There is indeed a problem that needs to determine whether the index exists. The better way I thought at the time was if it was found that the query part of the index does not exist, if ES can query part of the index, and there is a configuration that can be set to not report an error.
Regarding the narrow gap time, this commit has optimization is that if it is not found in the cache, it will query ES again to ensure that the index name list is up to date. If you want to be more precise in fine-grained time, for example ms level, the index change after index query, it may be that the current query will report an error(currently this situation will only occur in the early morning). It will have correct result query again without affecting subsequent queries.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request
#4353: Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379294683
##########
File path: oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/EsModelName.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.oap.server.storage.plugin.elasticsearch.base;
+
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import org.apache.skywalking.oap.server.core.analysis.Downsampling;
+import org.apache.skywalking.oap.server.core.query.DurationUtils;
+import org.apache.skywalking.oap.server.core.register.EndpointInventory;
+import org.apache.skywalking.oap.server.core.register.NetworkAddressInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.storage.model.ModelName;
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
+
+public class EsModelName extends ModelName {
+
+ private static final DateTimeFormatter YYYYMM = DateTimeFormat.forPattern("yyyyMM");
+ private static final DateTimeFormatter YYYYMMDD = DateTimeFormat.forPattern("yyyyMMdd");
+
+ public static final List<String> WHITE_INDEX_LIST = new ArrayList<String>() {
+ {
+ add(EndpointInventory.INDEX_NAME);
+ add(NetworkAddressInventory.INDEX_NAME);
+ add(ServiceInventory.INDEX_NAME);
+ add(ServiceInstanceInventory.INDEX_NAME);
+ }
+ };
+
+ public static List<String> build(Downsampling downsampling, String modelName, long startTB, long endTB) {
+ if (WHITE_INDEX_LIST.contains(modelName)) {
+ return new ArrayList<String>() {
+ {
+ add(modelName);
+ }
+ };
Review comment:
Same here, please be careful when using this kind of initialization, it creates unnecessary inner class, if you always use this, there may be many many unnecessary inner classes, exploding the meta space of the JVM, and the problem is that we have much better method to do this, like `Arrays.asList`, Guava list, etc.:
```java
new ArrayList<String>() {
{
add(modelName);
}
};
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378091398
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/storage/model/ModelName.java
##########
@@ -30,10 +43,73 @@ public static String build(Downsampling downsampling, String modelName) {
return modelName + Const.ID_SPLIT + Downsampling.Day.getName();
case Hour:
return modelName + Const.ID_SPLIT + Downsampling.Hour.getName();
- // case Second:
- // return modelName + Const.ID_SPLIT + Downsampling.Second.getName();
default:
return modelName;
}
}
+
+ public static String[] build(Downsampling downsampling, String modelName, long startTB, long endTB) {
Review comment:
This build method is for ES storage only, please move inside the plugin.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io edited a comment on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-585057389
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=h1) Report
> Merging [#4353](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/3736a5cb5be1b10f926d2a055a2911d058197a48?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `0%`.
[](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4353 +/- ##
======================================
Coverage 25.4% 25.4%
======================================
Files 1212 1212
Lines 28082 28082
Branches 3883 3883
======================================
Hits 7134 7134
Misses 20297 20297
Partials 651 651
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../apm/network/trace/component/ComponentsDefine.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-YXBtLXByb3RvY29sL2FwbS1uZXR3b3JrL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9uZXR3b3JrL3RyYWNlL2NvbXBvbmVudC9Db21wb25lbnRzRGVmaW5lLmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=footer). Last update [3736a5c...04f2854](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378088298
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java
##########
@@ -213,4 +213,36 @@ private DateTime parseToDateTime(Downsampling downsampling, long time) {
}
throw new UnexpectedException("Unexpected downsampling: " + downsampling.name());
}
+
+ public DateTime startTimeBucket2DateTime(Downsampling downsampling, long startTB) {
Review comment:
Why do you need to add? The current prefix should be enough. `2020021015`-`2020021315` to days is 20200210`-`20200213`
Do I miss anything?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378090251
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java
##########
@@ -213,4 +213,36 @@ private DateTime parseToDateTime(Downsampling downsampling, long time) {
}
throw new UnexpectedException("Unexpected downsampling: " + downsampling.name());
}
+
+ public DateTime startTimeBucket2DateTime(Downsampling downsampling, long startTB) {
Review comment:
OK, got it, make sense.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on issue #4353: Optimizing performance
reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586574243
> `allow_no_indices` and `ignore_unavailable` should be good choices. Could you try to use these? And check whether there is error log in OAP and ES server?
Try to use without problems, in addition, the machine tested some queries that do not exist across the date index, the results are normal，no error in log.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378083903
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java
##########
@@ -213,4 +213,36 @@ private DateTime parseToDateTime(Downsampling downsampling, long time) {
}
throw new UnexpectedException("Unexpected downsampling: " + downsampling.name());
}
+
+ public DateTime startTimeBucket2DateTime(Downsampling downsampling, long startTB) {
Review comment:
Indeed, it maybe faster to convert to long. When the date while loop statement is added, it needs to be converted to the character format of the date.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io edited a comment on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-585057389
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=h1) Report
> Merging [#4353](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/04f28549276262a55fd3493ea9c6f785bfc0986e?src=pr&el=desc) will **decrease** coverage by `0.44%`.
> The diff coverage is `5.41%`.
[](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4353 +/- ##
==========================================
- Coverage 25.4% 24.95% -0.45%
==========================================
Files 1212 1214 +2
Lines 28082 28246 +164
Branches 3883 3899 +16
==========================================
- Hits 7134 7049 -85
- Misses 20297 20548 +251
+ Partials 651 649 -2
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...re/register/service/EndpointInventoryRegister.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcmVnaXN0ZXIvc2VydmljZS9FbmRwb2ludEludmVudG9yeVJlZ2lzdGVyLmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
| [...lking/oap/query/graphql/resolver/ProfileQuery.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcXVlcnktcGx1Z2luL3F1ZXJ5LWdyYXBocWwtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9xdWVyeS9ncmFwaHFsL3Jlc29sdmVyL1Byb2ZpbGVRdWVyeS5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
| [.../oap/server/core/query/entity/ProfiledSegment.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcXVlcnkvZW50aXR5L1Byb2ZpbGVkU2VnbWVudC5qYXZh) | `0% <0%> (ø)` | |
| [...oap/server/core/query/ProfileTaskQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcXVlcnkvUHJvZmlsZVRhc2tRdWVyeVNlcnZpY2UuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
| [...gin/influxdb/query/ProfileThreadSnapshotQuery.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1pbmZsdXhkYi1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9pbmZsdXhkYi9xdWVyeS9Qcm9maWxlVGhyZWFkU25hcHNob3RRdWVyeS5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
| [...storage/plugin/jdbc/h2/dao/H2TopologyQueryDAO.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1qZGJjLWhpa2FyaWNwLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2pkYmMvaDIvZGFvL0gyVG9wb2xvZ3lRdWVyeURBTy5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
| [...csearch/query/ProfileThreadSnapshotQueryEsDAO.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvcXVlcnkvUHJvZmlsZVRocmVhZFNuYXBzaG90UXVlcnlFc0RBTy5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
| [...ng/oap/server/core/query/TopologyQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcXVlcnkvVG9wb2xvZ3lRdWVyeVNlcnZpY2UuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
| [...ing/oap/server/core/query/entity/ProfiledSpan.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcXVlcnkvZW50aXR5L1Byb2ZpbGVkU3Bhbi5qYXZh) | `0% <0%> (ø)` | |
| [...r/parser/standardization/ReferenceIdExchanger.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctdHJhY2UtcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvdHJhY2UvcHJvdmlkZXIvcGFyc2VyL3N0YW5kYXJkaXphdGlvbi9SZWZlcmVuY2VJZEV4Y2hhbmdlci5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
| ... and [18 more](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=footer). Last update [04f2854...5fd4dff](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378153034
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/storage/model/ModelName.java
##########
@@ -30,10 +43,73 @@ public static String build(Downsampling downsampling, String modelName) {
return modelName + Const.ID_SPLIT + Downsampling.Day.getName();
case Hour:
return modelName + Const.ID_SPLIT + Downsampling.Hour.getName();
- // case Second:
- // return modelName + Const.ID_SPLIT + Downsampling.Second.getName();
default:
return modelName;
}
}
+
+ public static String[] build(Downsampling downsampling, String modelName, long startTB, long endTB) {
+ List<String> indexNameList = new ArrayList<>();
+
+ List<String> whiteIndexList = new ArrayList<String>() {
+ {
+ add(EndpointInventory.INDEX_NAME);
+ add(NetworkAddressInventory.INDEX_NAME);
+ add(ServiceInventory.INDEX_NAME);
+ add(ServiceInstanceInventory.INDEX_NAME);
+ }
+ };
+
+ if (whiteIndexList.contains(modelName)) {
+ return new String[] { modelName };
+ }
+
+ DateTime startDT = DurationUtils.INSTANCE.startTimeBucket2DateTime(downsampling, startTB);
+ DateTime endDT = DurationUtils.INSTANCE.endTimeBucket2DateTime(downsampling, endTB);
+ if (endDT.isAfter(startDT)) {
+ switch (downsampling) {
+ case Month:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMM.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusMonths(1);
+ }
+ break;
+ case Day:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Hour:
+ //current hour index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Minute:
+ //current minute index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Second:
+ //current second index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ default:
+ indexNameList.add(modelName);
+ }
+ }
+ return indexNameList.toArray(new String[0]);
Review comment:
If select a date with a long span, which dates are not in the middle, or the system has been stopped for a few days, there may be no index data for several days. If you query date contain those days directly, searching the index will appear `index not found exception`. so, Did a filter that the date index does not exist
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm edited a comment on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586144859
@wu-sheng Worried about the problem of error in log( `index_not_found` exception ), I checked the relevant documents. Found that ES has configuration parameters that can be set.
Refer this:[[https://www.elastic.co/guide/en/elasticsearch/reference/current/multi-index.html](url)](url)
- gap time near 00:00
- multi OAP node
- product error log
The above mentioned scenarios are all fine.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on issue #4353: Optimizing performance
reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586144859
@wu-sheng Worried about the problem of error in log( `index_not_found` exception ), I checked the relevant documents. Found that ES has configuration parameters that can be set.
Refer this:[[https://www.elastic.co/guide/en/elasticsearch/reference/current/multi-index.html](url)](url)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379502634
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -291,6 +291,24 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB
return client.search(searchRequest);
}
+ /**
+ * Search results from ES search engine according to various search conditions,
+ * Note the method is usered for the list of index names is optimized based on
+ * the scope of startTimeBucket and endTimeBucket
+ * @param indexNameList full index names list base on timebucket scope.
+ * Except for endpoint_inventory network_address_inventory service_inventory service_instance_inventory
+ * @param searchSourceBuilder Various search query conditions
+ * @return ES search query results
+ * @throws IOException throw IOException
Review comment:
ack
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm edited a comment on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586860632
Option two. But a little changed, not `TraceQueryEsDAO#queryByTraceId` is `TraceQueryEsDAO#queryBasicTraces` about segment index, There is a sort in the query. the production operation and maintenance scene, it may be necessary to query the slow segments of a few days or even many tens days ago. When the number of instances and endpoints and concurrency are large, it is still necessary to quickly output results slow trace. In our producing, the data volume is still relatively large, sampling is configured, and the TTL time is only three day, but many developments need to find dozens of days ago, and it is impossible to get it, before In the pressure test environment, trace queries were still stuttered.
#4364 #4368 both about reduce the number of indexes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm edited a comment on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586860632
Option two. But a little changed, not `TraceQueryEsDAO#queryByTraceId` is `TraceQueryEsDAO#queryBasicTraces` about segment index. the production operation and maintenance scene, it may be necessary to query the slow segments of a few days or even many tens days ago. When the number of instances and endpoints and concurrency are large, it is still necessary to quickly output results slow trace. In our producing, the data volume is still relatively large, sampling is configured, and the TTL time is only three day, but many developments need to find dozens of days ago, and it is impossible to get it, before In the pressure test environment, trace queries were still stuttered.
#4364 #4368 both about reduce the number of indexes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378255741
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/storage/model/ModelName.java
##########
@@ -30,10 +43,73 @@ public static String build(Downsampling downsampling, String modelName) {
return modelName + Const.ID_SPLIT + Downsampling.Day.getName();
case Hour:
return modelName + Const.ID_SPLIT + Downsampling.Hour.getName();
- // case Second:
- // return modelName + Const.ID_SPLIT + Downsampling.Second.getName();
default:
return modelName;
}
}
+
+ public static String[] build(Downsampling downsampling, String modelName, long startTB, long endTB) {
+ List<String> indexNameList = new ArrayList<>();
+
+ List<String> whiteIndexList = new ArrayList<String>() {
+ {
+ add(EndpointInventory.INDEX_NAME);
+ add(NetworkAddressInventory.INDEX_NAME);
+ add(ServiceInventory.INDEX_NAME);
+ add(ServiceInstanceInventory.INDEX_NAME);
+ }
+ };
+
+ if (whiteIndexList.contains(modelName)) {
+ return new String[] { modelName };
+ }
+
+ DateTime startDT = DurationUtils.INSTANCE.startTimeBucket2DateTime(downsampling, startTB);
+ DateTime endDT = DurationUtils.INSTANCE.endTimeBucket2DateTime(downsampling, endTB);
+ if (endDT.isAfter(startDT)) {
+ switch (downsampling) {
+ case Month:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMM.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusMonths(1);
+ }
+ break;
+ case Day:
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Hour:
+ //current hour index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Minute:
+ //current minute index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ case Second:
+ //current second index is also suffix with YYYYMMDD
+ while (endDT.isAfter(startDT)) {
+ String indexName = build(downsampling, modelName) + "-" + YYYYMMDD.print(startDT);
+ indexNameList.add(indexName);
+ startDT = startDT.plusDays(1);
+ }
+ break;
+ default:
+ indexNameList.add(modelName);
+ }
+ }
+ return indexNameList.toArray(new String[0]);
Review comment:
> You covert to array at the first place, but then you covert to List again in `#filterNotExistIndex`. Seems unnecessary?
Oh, indeed i will fix it. Convert at last minitue.
> But based on the things you talked, I think you should establish a cache to `filterNotExistIndex` and cache the result for at least a day, considering the index could only be deleted day by day, right? Set up a guava cache with `#expireAfterWrite`?
Good advice, i will try.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm edited a comment on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586860632
Option two. But a little changed, not `TraceQueryEsDAO#queryByTraceId` is `TraceQueryEsDAO#queryBasicTraces` about segment index, There is a sort in the query. This query interface is recommended to be optimized according to this scheme. the production operation and maintenance scene, it may be necessary to query the slow segments of a few days or even many tens days ago. When the number of instances and endpoints and concurrency are large, it is still necessary to quickly output results slow trace. In our producing, the data volume is still relatively large, sampling is configured, and the TTL time is only three day, but many developments need to find dozens of days ago, and it is impossible to get it, before In the pressure test environment, trace queries were still stuttered.
#4364 #4368 both about reduce the number of indexes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378179210
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -291,6 +292,30 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB
return client.search(searchRequest);
}
+ public SearchResponse search(String[] indexNames, SearchSourceBuilder searchSourceBuilder) throws IOException {
+ String[] fullIndexNames = formatIndexNames(indexNames);
+ SearchRequest searchRequest = new SearchRequest(fullIndexNames);
+ searchRequest.types(TYPE);
+ searchRequest.source(searchSourceBuilder);
+ return client.search(searchRequest);
+ }
+
+ public String[] filterNotExistIndex(String[] fullIndexNames, String indName) throws IOException {
+ // if no wrap, it is impossible to remove elements
+ List<String> indexNameList = new ArrayList<>(Arrays.asList(fullIndexNames));
+ if (fullIndexNames.length > 0) {
+ List<String> existIndex = retrievalIndexByAliases(indName);
+ indexNameList.removeIf(indexName -> {
+ //only filter index name with xxxx-xxxx
+ if (indexName.contains("-")) {
Review comment:
for example `segment alarm_record`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io edited a comment on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-585057389
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@be902e1`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `6.54%`.
[](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4353 +/- ##
========================================
Coverage ? 25.4%
========================================
Files ? 1212
Lines ? 28082
Branches ? 3883
========================================
Hits ? 7134
Misses ? 20297
Partials ? 651
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...lking/oap/server/core/query/TraceQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcXVlcnkvVHJhY2VRdWVyeVNlcnZpY2UuamF2YQ==) | `0% <ø> (ø)` | |
| [...storage/plugin/influxdb/installer/H2Installer.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1pbmZsdXhkYi1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9pbmZsdXhkYi9pbnN0YWxsZXIvSDJJbnN0YWxsZXIuamF2YQ==) | `0% <0%> (ø)` | |
| [.../server/core/alarm/provider/grpc/GRPCCallback.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9ncnBjL0dSUENDYWxsYmFjay5qYXZh) | `59.42% <0%> (ø)` | |
| [.../RestHighLevelClientClusterMethodsInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vZWxhc3RpY3NlYXJjaC02LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vZWxhc3RpY3NlYXJjaC92Ni9pbnRlcmNlcHRvci9SZXN0SGlnaExldmVsQ2xpZW50Q2x1c3Rlck1ldGhvZHNJbnRlcmNlcHRvci5qYXZh) | `0% <0%> (ø)` | |
| [...er/storage/plugin/influxdb/query/MetricsQuery.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1pbmZsdXhkYi1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9pbmZsdXhkYi9xdWVyeS9NZXRyaWNzUXVlcnkuamF2YQ==) | `0% <0%> (ø)` | |
| [...ge/plugin/elasticsearch/query/TraceQueryEsDAO.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvcXVlcnkvVHJhY2VRdWVyeUVzREFPLmphdmE=) | `0% <0%> (ø)` | |
| [.../plugin/elasticsearch/base/StorageEsInstaller.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvYmFzZS9TdG9yYWdlRXNJbnN0YWxsZXIuamF2YQ==) | `0% <0%> (ø)` | |
| [...gin/influxdb/query/ProfileThreadSnapshotQuery.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1pbmZsdXhkYi1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9pbmZsdXhkYi9xdWVyeS9Qcm9maWxlVGhyZWFkU25hcHNob3RRdWVyeS5qYXZh) | `0% <0%> (ø)` | |
| [...age/plugin/influxdb/query/ProfileTaskLogQuery.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1pbmZsdXhkYi1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9pbmZsdXhkYi9xdWVyeS9Qcm9maWxlVGFza0xvZ1F1ZXJ5LmphdmE=) | `0% <0%> (ø)` | |
| [...rver/storage/plugin/influxdb/query/TraceQuery.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1pbmZsdXhkYi1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9pbmZsdXhkYi9xdWVyeS9UcmFjZVF1ZXJ5LmphdmE=) | `0% <0%> (ø)` | |
| ... and [31 more](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=footer). Last update [be902e1...3736a5c](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378197273
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/storage/model/ModelName.java
##########
@@ -30,10 +43,73 @@ public static String build(Downsampling downsampling, String modelName) {
return modelName + Const.ID_SPLIT + Downsampling.Day.getName();
case Hour:
return modelName + Const.ID_SPLIT + Downsampling.Hour.getName();
- // case Second:
- // return modelName + Const.ID_SPLIT + Downsampling.Second.getName();
default:
return modelName;
}
}
+
+ public static String[] build(Downsampling downsampling, String modelName, long startTB, long endTB) {
+ List<String> indexNameList = new ArrayList<>();
+
+ List<String> whiteIndexList = new ArrayList<String>() {
Review comment:
ok
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378197175
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/storage/model/ModelName.java
##########
@@ -30,10 +43,73 @@ public static String build(Downsampling downsampling, String modelName) {
return modelName + Const.ID_SPLIT + Downsampling.Day.getName();
case Hour:
return modelName + Const.ID_SPLIT + Downsampling.Hour.getName();
- // case Second:
- // return modelName + Const.ID_SPLIT + Downsampling.Second.getName();
default:
return modelName;
}
}
+
+ public static String[] build(Downsampling downsampling, String modelName, long startTB, long endTB) {
Review comment:
ok
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378673889
##########
File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -291,6 +292,30 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB
return client.search(searchRequest);
}
+ public SearchResponse search(String[] indexNames, SearchSourceBuilder searchSourceBuilder) throws IOException {
+ String[] fullIndexNames = formatIndexNames(indexNames);
+ SearchRequest searchRequest = new SearchRequest(fullIndexNames);
+ searchRequest.types(TYPE);
+ searchRequest.source(searchSourceBuilder);
+ return client.search(searchRequest);
+ }
+
+ public String[] filterNotExistIndex(String[] fullIndexNames, String indName) throws IOException {
+ // if no wrap, it is impossible to remove elements
+ List<String> indexNameList = new ArrayList<>(Arrays.asList(fullIndexNames));
+ if (fullIndexNames.length > 0) {
+ List<String> existIndex = retrievalIndexByAliases(indName);
+ indexNameList.removeIf(indexName -> {
+ //only filter index name with xxxx-xxxx
+ if (indexName.contains("-")) {
Review comment:
Then you could provide a filter func as the parameter to do the filter works. This indicates this logic doesn't belong to this class.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io edited a comment on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-585057389
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=h1) Report
> Merging [#4353](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/1d589f17dab9cd6258a611bccd17fb31af469a84?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `0%`.
[](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4353 +/- ##
==========================================
- Coverage 26.02% 26.02% -0.01%
==========================================
Files 1185 1185
Lines 27048 27050 +2
Branches 3739 3740 +1
==========================================
Hits 7040 7040
- Misses 19374 19376 +2
Partials 634 634
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...pm/plugin/armeria/Armeria085ServerInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vYXJtZXJpYS0wLjg1LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vYXJtZXJpYS9Bcm1lcmlhMDg1U2VydmVySW50ZXJjZXB0b3IuamF2YQ==) | `0% <ø> (ø)` | :arrow_up: |
| [...pm/plugin/armeria/Armeria085ClientInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vYXJtZXJpYS0wLjg1LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vYXJtZXJpYS9Bcm1lcmlhMDg1Q2xpZW50SW50ZXJjZXB0b3IuamF2YQ==) | `0% <ø> (ø)` | :arrow_up: |
| [...pm/plugin/armeria/Armeria086ClientInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vYXJtZXJpYS0wLjg1LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vYXJtZXJpYS9Bcm1lcmlhMDg2Q2xpZW50SW50ZXJjZXB0b3IuamF2YQ==) | `0% <ø> (ø)` | :arrow_up: |
| [...g/apm/plugin/armeria/ArmeriaClientInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vYXJtZXJpYS0wLjg1LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vYXJtZXJpYS9Bcm1lcmlhQ2xpZW50SW50ZXJjZXB0b3IuamF2YQ==) | `0% <ø> (ø)` | :arrow_up: |
| [...pm/plugin/armeria/Armeria098ClientInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vYXJtZXJpYS0wLjg1LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vYXJtZXJpYS9Bcm1lcmlhMDk4Q2xpZW50SW50ZXJjZXB0b3IuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
| [...storage/plugin/elasticsearch/base/EsModelName.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvYmFzZS9Fc01vZGVsTmFtZS5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
| [...n/elasticsearch7/query/AggregationQueryEs7DAO.java](https://codecov.io/gh/apache/skywalking/pull/4353/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoNy1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9lbGFzdGljc2VhcmNoNy9xdWVyeS9BZ2dyZWdhdGlvblF1ZXJ5RXM3REFPLmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=footer). Last update [1d589f1...037d940](https://codecov.io/gh/apache/skywalking/pull/4353?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4353: Optimizing
performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#issuecomment-590068534
I would support removing other besides the trace. And I prefer to keep this open and not included in 7.0.0. We could be back to this if there is someone(s) pay attentions on this because of their production cases.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378070209
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java
##########
@@ -213,4 +213,36 @@ private DateTime parseToDateTime(Downsampling downsampling, long time) {
}
throw new UnexpectedException("Unexpected downsampling: " + downsampling.name());
}
+
+ public DateTime startTimeBucket2DateTime(Downsampling downsampling, long startTB) {
Review comment:
First, time covert should be inside `TimeBucket`, take a look at InfluxDB, https://github.com/apache/skywalking/pull/4239/files#diff-df3b7b3f55b25fb0e29e345e7e6cc078. Maybe good for you.
I think you don't need the `DateTime`? Directly convert to long as day/month makes more sense?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4353:
Optimizing performance reduces es index queries scope by timebucket
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r378071809
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java
##########
@@ -213,4 +213,36 @@ private DateTime parseToDateTime(Downsampling downsampling, long time) {
}
throw new UnexpectedException("Unexpected downsampling: " + downsampling.name());
}
+
+ public DateTime startTimeBucket2DateTime(Downsampling downsampling, long startTB) {
Review comment:
Currently, you covert to DateTime, then cover to long like `202002`, but actually, you could use divide to get `202002` directly and more quickly, right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services