You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2020/10/13 08:33:01 UTC
[GitHub] [iotdb] Alima777 opened a new pull request #1824: [IOTDB-942] Optimization of query with long time unsequence page
Alima777 opened a new pull request #1824:
URL: https://github.com/apache/iotdb/pull/1824
----------------------------------------------------------------
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
[GitHub] [iotdb] Alima777 removed a comment on pull request #1824: [IOTDB-942] Optimization of query with long time unsequence page
Posted by GitBox <gi...@apache.org>.
Alima777 removed a comment on pull request #1824:
URL: https://github.com/apache/iotdb/pull/1824#issuecomment-711478844
> Do not forget to add the experiment result in ur PR please : )
> Do not forget to add the experiment result in ur PR please : )
----------------------------------------------------------------
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
[GitHub] [iotdb] Alima777 commented on a change in pull request #1824: [IOTDB-942] Optimization of query with long time unsequence page
Posted by GitBox <gi...@apache.org>.
Alima777 commented on a change in pull request #1824:
URL: https://github.com/apache/iotdb/pull/1824#discussion_r507544732
##########
File path: server/src/main/java/org/apache/iotdb/db/query/reader/series/SeriesReader.java
##########
@@ -870,12 +963,31 @@ public TsFileResource getNextSeqFileResource(List<TsFileResource> seqResources,
(c1, c2) -> Long.compare(keyExtractor.applyAsLong(c2), keyExtractor.applyAsLong(c1));
}
+ @Override
+ public long getCurrentEndPoint(long time, Statistics<? extends Object> statistics) {
+ return Math.max(time, statistics.getStartTime());
+ }
+
+ @Override
+ public long getCurrentEndPoint(Statistics<? extends Object> seqStatistics,
+ Statistics<? extends Object> unseqStatistics) {
+ return Math.max(seqStatistics.getStartTime(), unseqStatistics.getStartTime());
+ }
@Override
public boolean isExcessEndpoint(long time, long endpointTime) {
return time < endpointTime;
}
+ @Override
+ public boolean isTakeSeqAsFirst(Statistics<? extends Object> seqStatistics,
+ Statistics<? extends Object> unseqStatistics) {
+ if (seqStatistics.getEndTime() > unseqStatistics.getEndTime()) {
+ return true;
+ }
+ return false;
Review comment:
Fixed.
----------------------------------------------------------------
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
[GitHub] [iotdb] HTHou merged pull request #1824: [IOTDB-942] Optimization of query with long time unsequence page
Posted by GitBox <gi...@apache.org>.
HTHou merged pull request #1824:
URL: https://github.com/apache/iotdb/pull/1824
----------------------------------------------------------------
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
[GitHub] [iotdb] samperson1997 commented on a change in pull request #1824: [IOTDB-942] Optimization of query with long time unsequence page
Posted by GitBox <gi...@apache.org>.
samperson1997 commented on a change in pull request #1824:
URL: https://github.com/apache/iotdb/pull/1824#discussion_r507483827
##########
File path: server/src/main/java/org/apache/iotdb/db/query/reader/series/SeriesReader.java
##########
@@ -931,11 +1043,31 @@ public TsFileResource getNextSeqFileResource(List<TsFileResource> seqResources,
(c1, c2) -> Long.compare(keyExtractor.applyAsLong(c1), keyExtractor.applyAsLong(c2));
}
+ @Override
+ public long getCurrentEndPoint(long time, Statistics<? extends Object> statistics) {
+ return Math.min(time, statistics.getEndTime());
+ }
+
+ @Override
+ public long getCurrentEndPoint(Statistics<? extends Object> seqStatistics,
+ Statistics<? extends Object> unseqStatistics) {
+ return Math.min(seqStatistics.getEndTime(), unseqStatistics.getEndTime());
+ }
+
@Override
public boolean isExcessEndpoint(long time, long endpointTime) {
return time > endpointTime;
}
+ @Override
+ public boolean isTakeSeqAsFirst(Statistics<? extends Object> seqStatistics,
+ Statistics<? extends Object> unseqStatistics) {
+ if (seqStatistics.getStartTime() < unseqStatistics.getStartTime()) {
+ return true;
+ }
+ return false;
Review comment:
```suggestion
return seqStatistics.getStartTime() < unseqStatistics.getStartTime());
```
##########
File path: server/src/main/java/org/apache/iotdb/db/query/reader/series/SeriesReader.java
##########
@@ -870,12 +963,31 @@ public TsFileResource getNextSeqFileResource(List<TsFileResource> seqResources,
(c1, c2) -> Long.compare(keyExtractor.applyAsLong(c2), keyExtractor.applyAsLong(c1));
}
+ @Override
+ public long getCurrentEndPoint(long time, Statistics<? extends Object> statistics) {
+ return Math.max(time, statistics.getStartTime());
+ }
+
+ @Override
+ public long getCurrentEndPoint(Statistics<? extends Object> seqStatistics,
+ Statistics<? extends Object> unseqStatistics) {
+ return Math.max(seqStatistics.getStartTime(), unseqStatistics.getStartTime());
+ }
@Override
public boolean isExcessEndpoint(long time, long endpointTime) {
return time < endpointTime;
}
+ @Override
+ public boolean isTakeSeqAsFirst(Statistics<? extends Object> seqStatistics,
+ Statistics<? extends Object> unseqStatistics) {
+ if (seqStatistics.getEndTime() > unseqStatistics.getEndTime()) {
+ return true;
+ }
+ return false;
Review comment:
```suggestion
return seqStatistics.getEndTime() > unseqStatistics.getEndTime();
```
##########
File path: server/src/main/java/org/apache/iotdb/db/query/reader/series/SeriesReader.java
##########
@@ -816,8 +901,16 @@ boolean isModified() {
<T> Comparator<T> comparingLong(ToLongFunction<? super T> keyExtractor);
+ long getCurrentEndPoint(long time, Statistics<? extends Object> statistics);
+
+ long getCurrentEndPoint(Statistics<? extends Object> seqStatistics,
+ Statistics<? extends Object> unseqStatistics);
+
boolean isExcessEndpoint(long time, long endpointTime);
+ boolean isTakeSeqAsFirst(Statistics<? extends Object> seqStatistics,
Review comment:
Add java doc for these new abstract methods
##########
File path: server/src/main/java/org/apache/iotdb/db/query/reader/series/SeriesReader.java
##########
@@ -407,11 +409,15 @@ boolean hasNextPage() throws IOException {
}
// make sure firstPageReader won't be null while cachedPageReaders has more cached page readers
- while (firstPageReader == null && !cachedPageReaders.isEmpty()) {
- firstPageReader = cachedPageReaders.poll();
- if (!cachedPageReaders.isEmpty()
- && orderUtils.isOverlapped(firstPageReader.getStatistics(),
- cachedPageReaders.peek().getStatistics())) {
+ while (firstPageReader == null && (!seqPageReaders.isEmpty() || !unSeqPageReaders.isEmpty())) {
+
+ initFirstPageReader();
+
+ if ((!seqPageReaders.isEmpty() && orderUtils
+ .isOverlapped(firstPageReader.getStatistics(), seqPageReaders.get(0).getStatistics()))
+ || (!unSeqPageReaders.isEmpty() && orderUtils
+ .isOverlapped(firstPageReader.getStatistics(),
+ unSeqPageReaders.peek().getStatistics()))) {
Review comment:
better to extract these complicate condition statements into a private method, since they appear not only once in this file (for example, in lines 395-398)
##########
File path: server/src/main/java/org/apache/iotdb/db/query/reader/series/SeriesReader.java
##########
@@ -610,18 +674,33 @@ private void tryToPutAllDirectlyOverlappedPageReadersIntoMergeReader() throws IO
}
/*
- * put all currently directly overlapped page reader to merge reader
+ * put all currently directly overlapped unseq page reader to merge reader
*/
- unpackAllOverlappedCachedPageReadersToMergeReader(currentPageEndpointTime);
+ unpackAllOverlappedUnseqPageReadersToMergeReader(currentPageEndpointTime);
}
- private void unpackAllOverlappedCachedPageReadersToMergeReader(long endpointTime)
+ private void initFirstPageReader() {
+ if (!seqPageReaders.isEmpty() && !unSeqPageReaders.isEmpty()) {
+ if (orderUtils.isTakeSeqAsFirst(seqPageReaders.get(0).getStatistics(), unSeqPageReaders.peek()
+ .getStatistics())) {
+ firstPageReader = seqPageReaders.remove(0);
+ } else {
+ firstPageReader = unSeqPageReaders.poll();
+ }
+ } else if (!seqPageReaders.isEmpty()) {
+ firstPageReader = seqPageReaders.remove(0);
+ } else if (!unSeqPageReaders.isEmpty()) {
+ firstPageReader = unSeqPageReaders.poll();
+ }
+ }
+
+ private void unpackAllOverlappedUnseqPageReadersToMergeReader(long endpointTime)
throws IOException {
- while (!cachedPageReaders.isEmpty()
- && orderUtils.isOverlapped(endpointTime, cachedPageReaders.peek().data.getStatistics())) {
- putPageReaderToMergeReader(cachedPageReaders.poll());
+ while (!unSeqPageReaders.isEmpty()
+ && orderUtils.isOverlapped(endpointTime, unSeqPageReaders.peek().data.getStatistics())) {
+ putPageReaderToMergeReader(unSeqPageReaders.poll());
}
- if (firstPageReader != null &&
+ if (firstPageReader != null && !firstPageReader.isSeq &&
Review comment:
```suggestion
if (firstPageReader != null && !firstPageReader.isSeq() &&
```
##########
File path: server/src/main/java/org/apache/iotdb/db/query/reader/series/SeriesReader.java
##########
@@ -381,19 +383,19 @@ boolean hasNextPage() throws IOException {
/*
* first chunk metadata is already unpacked, consume cached pages
*/
- if (!cachedPageReaders.isEmpty()) {
- firstPageReader = cachedPageReaders.poll();
+ initFirstPageReader();
+ if (firstPageReader != null) {
long endpointTime = orderUtils.getOverlapCheckTime(firstPageReader.getStatistics());
unpackAllOverlappedTsFilesToTimeSeriesMetadata(endpointTime);
unpackAllOverlappedTimeSeriesMetadataToCachedChunkMetadata(endpointTime, false);
unpackAllOverlappedChunkMetadataToCachedPageReaders(endpointTime, false);
Review comment:
Since you modified the field name of `cachedPageReaders`, maybe the name of this method should be updated as well.
----------------------------------------------------------------
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
[GitHub] [iotdb] Alima777 commented on a change in pull request #1824: [IOTDB-942] Optimization of query with long time unsequence page
Posted by GitBox <gi...@apache.org>.
Alima777 commented on a change in pull request #1824:
URL: https://github.com/apache/iotdb/pull/1824#discussion_r507544971
##########
File path: server/src/main/java/org/apache/iotdb/db/query/reader/series/SeriesReader.java
##########
@@ -407,11 +409,15 @@ boolean hasNextPage() throws IOException {
}
// make sure firstPageReader won't be null while cachedPageReaders has more cached page readers
- while (firstPageReader == null && !cachedPageReaders.isEmpty()) {
- firstPageReader = cachedPageReaders.poll();
- if (!cachedPageReaders.isEmpty()
- && orderUtils.isOverlapped(firstPageReader.getStatistics(),
- cachedPageReaders.peek().getStatistics())) {
+ while (firstPageReader == null && (!seqPageReaders.isEmpty() || !unSeqPageReaders.isEmpty())) {
+
+ initFirstPageReader();
+
+ if ((!seqPageReaders.isEmpty() && orderUtils
+ .isOverlapped(firstPageReader.getStatistics(), seqPageReaders.get(0).getStatistics()))
+ || (!unSeqPageReaders.isEmpty() && orderUtils
+ .isOverlapped(firstPageReader.getStatistics(),
+ unSeqPageReaders.peek().getStatistics()))) {
Review comment:
You are right, fixed~
----------------------------------------------------------------
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
[GitHub] [iotdb] Alima777 commented on a change in pull request #1824: [IOTDB-942] Optimization of query with long time unsequence page
Posted by GitBox <gi...@apache.org>.
Alima777 commented on a change in pull request #1824:
URL: https://github.com/apache/iotdb/pull/1824#discussion_r507684846
##########
File path: server/src/main/java/org/apache/iotdb/db/query/reader/series/SeriesReader.java
##########
@@ -816,8 +901,16 @@ boolean isModified() {
<T> Comparator<T> comparingLong(ToLongFunction<? super T> keyExtractor);
+ long getCurrentEndPoint(long time, Statistics<? extends Object> statistics);
+
+ long getCurrentEndPoint(Statistics<? extends Object> seqStatistics,
+ Statistics<? extends Object> unseqStatistics);
+
boolean isExcessEndpoint(long time, long endpointTime);
+ boolean isTakeSeqAsFirst(Statistics<? extends Object> seqStatistics,
Review comment:
OK~ Thank you
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] Alima777 commented on pull request #1824: [IOTDB-942] Optimization of query with long time unsequence page
Posted by GitBox <gi...@apache.org>.
Alima777 commented on pull request #1824:
URL: https://github.com/apache/iotdb/pull/1824#issuecomment-711478844
> Do not forget to add the experiment result in ur PR please : )
> Do not forget to add the experiment result in ur PR please : )
----------------------------------------------------------------
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
[GitHub] [iotdb] JackieTien97 commented on pull request #1824: [IOTDB-942] Optimization of query with long time unsequence page
Posted by GitBox <gi...@apache.org>.
JackieTien97 commented on pull request #1824:
URL: https://github.com/apache/iotdb/pull/1824#issuecomment-713249814
I think it's ok to merge this PR.
----------------------------------------------------------------
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
[GitHub] [iotdb] Alima777 commented on a change in pull request #1824: [IOTDB-942] Optimization of query with long time unsequence page
Posted by GitBox <gi...@apache.org>.
Alima777 commented on a change in pull request #1824:
URL: https://github.com/apache/iotdb/pull/1824#discussion_r507545531
##########
File path: server/src/main/java/org/apache/iotdb/db/query/reader/series/SeriesReader.java
##########
@@ -381,19 +383,19 @@ boolean hasNextPage() throws IOException {
/*
* first chunk metadata is already unpacked, consume cached pages
*/
- if (!cachedPageReaders.isEmpty()) {
- firstPageReader = cachedPageReaders.poll();
+ initFirstPageReader();
+ if (firstPageReader != null) {
long endpointTime = orderUtils.getOverlapCheckTime(firstPageReader.getStatistics());
unpackAllOverlappedTsFilesToTimeSeriesMetadata(endpointTime);
unpackAllOverlappedTimeSeriesMetadataToCachedChunkMetadata(endpointTime, false);
unpackAllOverlappedChunkMetadataToCachedPageReaders(endpointTime, false);
Review comment:
Remaining this name is still better I think.
##########
File path: server/src/main/java/org/apache/iotdb/db/query/reader/series/SeriesReader.java
##########
@@ -381,19 +383,19 @@ boolean hasNextPage() throws IOException {
/*
* first chunk metadata is already unpacked, consume cached pages
*/
- if (!cachedPageReaders.isEmpty()) {
- firstPageReader = cachedPageReaders.poll();
+ initFirstPageReader();
+ if (firstPageReader != null) {
long endpointTime = orderUtils.getOverlapCheckTime(firstPageReader.getStatistics());
unpackAllOverlappedTsFilesToTimeSeriesMetadata(endpointTime);
unpackAllOverlappedTimeSeriesMetadataToCachedChunkMetadata(endpointTime, false);
unpackAllOverlappedChunkMetadataToCachedPageReaders(endpointTime, false);
Review comment:
Remaining this name is still good I 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
[GitHub] [iotdb] samperson1997 commented on a change in pull request #1824: [IOTDB-942] Optimization of query with long time unsequence page
Posted by GitBox <gi...@apache.org>.
samperson1997 commented on a change in pull request #1824:
URL: https://github.com/apache/iotdb/pull/1824#discussion_r507679196
##########
File path: server/src/main/java/org/apache/iotdb/db/query/reader/series/SeriesReader.java
##########
@@ -816,8 +901,16 @@ boolean isModified() {
<T> Comparator<T> comparingLong(ToLongFunction<? super T> keyExtractor);
+ long getCurrentEndPoint(long time, Statistics<? extends Object> statistics);
+
+ long getCurrentEndPoint(Statistics<? extends Object> seqStatistics,
+ Statistics<? extends Object> unseqStatistics);
+
boolean isExcessEndpoint(long time, long endpointTime);
+ boolean isTakeSeqAsFirst(Statistics<? extends Object> seqStatistics,
Review comment:
@Alima777 Don't forget this 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
[GitHub] [iotdb] Alima777 closed pull request #1824: [IOTDB-942] Optimization of query with long time unsequence page
Posted by GitBox <gi...@apache.org>.
Alima777 closed pull request #1824:
URL: https://github.com/apache/iotdb/pull/1824
----------------------------------------------------------------
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
[GitHub] [iotdb] Alima777 commented on pull request #1824: [IOTDB-942] Optimization of query with long time unsequence page
Posted by GitBox <gi...@apache.org>.
Alima777 commented on pull request #1824:
URL: https://github.com/apache/iotdb/pull/1824#issuecomment-711479715
> Do not forget to add the experiment result in ur PR please : )
Added. Thank you ~
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org