wu-sheng commented on a change in pull request #5132:
URL: https://github.com/apache/skywalking/pull/5132#discussion_r468476315
##########
File path:
oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -331,9 +333,25 @@ public boolean deleteTemplate(String indexName) throws
IOException {
return response.getStatusLine().getStatusCode() == HttpStatus.SC_OK;
}
+ public SearchResponse search(IndexNameMaker indexNameMaker,
SearchSourceBuilder searchSourceBuilder) throws IOException {
+ String[] indexNames =
Arrays.stream(indexNameMaker.make()).map(this::formatIndexName).toArray(String[]::new);
+ return search(indexNames, searchSourceBuilder);
+ }
+
public SearchResponse search(String indexName, SearchSourceBuilder
searchSourceBuilder) throws IOException {
indexName = formatIndexName(indexName);
- SearchRequest searchRequest = new SearchRequest(indexName);
+ return doSearch(searchSourceBuilder, indexName);
+ }
+
+ public SearchResponse search(String[] indexNames, SearchSourceBuilder
searchSourceBuilder) throws IOException {
Review comment:
I think we don't need this method in **public**, right?
##########
File path:
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/TimeSeriesUtils.java
##########
@@ -60,6 +61,31 @@ public static String latestWriteIndexName(Model model) {
}
}
+ /**
+ * @return Concrete index name for super dataset index
+ */
+ public static String[] querySuperDatasetIndices(String indexName, long
startTimeBucket, long endTimeBucket) {
+ if (startTimeBucket == 0 || endTimeBucket == 0) {
+ return new String[] {indexName};
+ }
+ long startDay =
compressTimeBucket(convert2DayTimeBucket(startTimeBucket),
SUPER_DATASET_DAY_STEP);
+ long endDay = compressTimeBucket(convert2DayTimeBucket(endTimeBucket),
SUPER_DATASET_DAY_STEP);
+ DateTime startDateTime = TIME_BUCKET_FORMATTER.parseDateTime(startDay
+ "");
+ DateTime endDateTime = TIME_BUCKET_FORMATTER.parseDateTime(endDay +
"");
+
+ int steps = Math.max((Days.daysBetween(startDateTime,
endDateTime).getDays()) / SUPER_DATASET_DAY_STEP, 0);
+ return IntStream.rangeClosed(0, steps)
+ .mapToObj(step -> indexName + Const.LINE +
startDateTime.plusDays(Math.toIntExact(SUPER_DATASET_DAY_STEP *
step)).toString(TIME_BUCKET_FORMATTER))
+ .toArray(String[]::new);
+ }
+
+ private static long convert2DayTimeBucket(long timeBucket) {
Review comment:
I think basically, we don't need this method. Read Joda lib, such as
`LocalDateTime.now().truncatedTo(ChronoUnit.MINUTES)`
##########
File path:
oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##########
@@ -331,9 +333,25 @@ public boolean deleteTemplate(String indexName) throws
IOException {
return response.getStatusLine().getStatusCode() == HttpStatus.SC_OK;
}
+ public SearchResponse search(IndexNameMaker indexNameMaker,
SearchSourceBuilder searchSourceBuilder) throws IOException {
+ String[] indexNames =
Arrays.stream(indexNameMaker.make()).map(this::formatIndexName).toArray(String[]::new);
+ return search(indexNames, searchSourceBuilder);
+ }
+
public SearchResponse search(String indexName, SearchSourceBuilder
searchSourceBuilder) throws IOException {
indexName = formatIndexName(indexName);
- SearchRequest searchRequest = new SearchRequest(indexName);
+ return doSearch(searchSourceBuilder, indexName);
+ }
+
+ public SearchResponse search(String[] indexNames, SearchSourceBuilder
searchSourceBuilder) throws IOException {
+ indexNames =
Arrays.stream(indexNames).map(this::formatIndexName).toArray(String[]::new);
+ return doSearch(searchSourceBuilder, indexNames);
+ }
+
+ private SearchResponse doSearch(SearchSourceBuilder searchSourceBuilder,
+ String... indexNames) throws IOException {
+ SearchRequest searchRequest = new SearchRequest(indexNames);
+ searchRequest.indicesOptions(IndicesOptions.fromOptions(true, true,
true, false));
Review comment:
Could you explain a little more about why we need this?
##########
File path:
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/TimeSeriesUtils.java
##########
@@ -60,6 +61,31 @@ public static String latestWriteIndexName(Model model) {
}
}
+ /**
+ * @return Concrete index name for super dataset index
+ */
+ public static String[] querySuperDatasetIndices(String indexName, long
startTimeBucket, long endTimeBucket) {
Review comment:
I think this should be renamed to `superDatasetIndexName`, right?
##########
File path:
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/TimeSeriesUtils.java
##########
@@ -60,6 +61,31 @@ public static String latestWriteIndexName(Model model) {
}
}
+ /**
+ * @return Concrete index name for super dataset index
+ */
+ public static String[] querySuperDatasetIndices(String indexName, long
startTimeBucket, long endTimeBucket) {
+ if (startTimeBucket == 0 || endTimeBucket == 0) {
+ return new String[] {indexName};
+ }
+ long startDay =
compressTimeBucket(convert2DayTimeBucket(startTimeBucket),
SUPER_DATASET_DAY_STEP);
+ long endDay = compressTimeBucket(convert2DayTimeBucket(endTimeBucket),
SUPER_DATASET_DAY_STEP);
+ DateTime startDateTime = TIME_BUCKET_FORMATTER.parseDateTime(startDay
+ "");
+ DateTime endDateTime = TIME_BUCKET_FORMATTER.parseDateTime(endDay +
"");
+
+ int steps = Math.max((Days.daysBetween(startDateTime,
endDateTime).getDays()) / SUPER_DATASET_DAY_STEP, 0);
Review comment:
Max with `0`? Do you avoid negative value? What happens when `the steps
== 0`?
##########
File path:
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/TimeSeriesUtils.java
##########
@@ -60,6 +61,31 @@ public static String latestWriteIndexName(Model model) {
}
}
+ /**
+ * @return Concrete index name for super dataset index
+ */
+ public static String[] querySuperDatasetIndices(String indexName, long
startTimeBucket, long endTimeBucket) {
+ if (startTimeBucket == 0 || endTimeBucket == 0) {
+ return new String[] {indexName};
+ }
+ long startDay =
compressTimeBucket(convert2DayTimeBucket(startTimeBucket),
SUPER_DATASET_DAY_STEP);
+ long endDay = compressTimeBucket(convert2DayTimeBucket(endTimeBucket),
SUPER_DATASET_DAY_STEP);
+ DateTime startDateTime = TIME_BUCKET_FORMATTER.parseDateTime(startDay
+ "");
+ DateTime endDateTime = TIME_BUCKET_FORMATTER.parseDateTime(endDay +
"");
+
+ int steps = Math.max((Days.daysBetween(startDateTime,
endDateTime).getDays()) / SUPER_DATASET_DAY_STEP, 0);
Review comment:
The above processes may need a little refactor about the
`compressTimeBucket` method, I should be able to accept `DateTime` as one
parameter too.
##########
File path:
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/TimeSeriesUtils.java
##########
@@ -60,6 +61,31 @@ public static String latestWriteIndexName(Model model) {
}
}
+ /**
+ * @return Concrete index name for super dataset index
+ */
+ public static String[] querySuperDatasetIndices(String indexName, long
startTimeBucket, long endTimeBucket) {
+ if (startTimeBucket == 0 || endTimeBucket == 0) {
+ return new String[] {indexName};
+ }
+ long startDay =
compressTimeBucket(convert2DayTimeBucket(startTimeBucket),
SUPER_DATASET_DAY_STEP);
+ long endDay = compressTimeBucket(convert2DayTimeBucket(endTimeBucket),
SUPER_DATASET_DAY_STEP);
+ DateTime startDateTime = TIME_BUCKET_FORMATTER.parseDateTime(startDay
+ "");
+ DateTime endDateTime = TIME_BUCKET_FORMATTER.parseDateTime(endDay +
"");
+
+ int steps = Math.max((Days.daysBetween(startDateTime,
endDateTime).getDays()) / SUPER_DATASET_DAY_STEP, 0);
Review comment:
I think this method should simply follow this logic,
1. List all days between `startTime` and `endTime`.
1. Use `compressTimeBucket` to get all indexes(could have duplicated records)
1. Distinct the list.
----------------------------------------------------------------
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:
[email protected]
