[IOTDB-527] Refactor series reader #864

qiaojialin · 2020-03-01T13:52:23Z

remove hasOverlappedPage & isPageOverlapped methods in IAggregateReader

optimize logic in series reader

* fix valueFilterBug for nextOverlappedPage

* new multithread groupby and GroupByExecutor

JackieTien97

About the Group by parallel, it seems that it will create new future task and invoke future.get() each time nextWithoutConstraint() is called. This way is like the first version's raw data query parallel that I implements. It will slower the query instead of speeding up.

JackieTien97 · 2020-03-02T02:36:03Z

server/src/main/java/org/apache/iotdb/db/query/reader/series/SeriesReaderByTimestamp.java

-      if (!satisfyTimeFilter(seriesReader.currentChunkStatistics())) {
-        seriesReader.skipCurrentChunk();
-        continue;


Why here delete this, if we can know from the chunk statistics that the chunk is not satisfied, we can just skip that chunk

I've made up for it

JackieTien97 · 2020-03-02T03:07:51Z

server/src/main/java/org/apache/iotdb/db/query/reader/series/SeriesReader.java

  public boolean hasNextChunk() throws IOException {
-    if (hasCachedFirstChunkMetadata) {
-      return true;


It seems that if I keep calling hasNextChunk(), the firstChunkMetaData will move to next all the time.
you can add an if statement.
if (firstChunkMetaData != null)
return true;

JackieTien97 · 2020-03-02T03:35:22Z

...rc/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByWithoutValueFilterDataSet.java

-        if (remainingToCalculate == 0) {
-          return aggregateResultList;
+
+  private class GroupByExecutor {


It's better name it GroupByTask

no , i like executor

executor or task are all fine to me

JackieTien97 · 2020-03-02T03:42:10Z

...rc/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByWithoutValueFilterDataSet.java

+      QueryDataSource queryDataSource = QueryResourceManager.getInstance()
+          .getQueryDataSource(path, context, timeFilter);
+      // update filter by TTL
+      timeFilter = queryDataSource.updateFilterUsingTTL(timeFilter);


No need to construct these two, if pathExecutors already has the path

liutaohua · 2020-03-02T07:19:14Z

About the Group by parallel, it seems that it will create new future task and invoke future.get() each time nextWithoutConstraint() is called. This way is like the first version's raw data query parallel that I implements. It will slower the query instead of speeding up.

Thank you very much for your reminding. I will continue to study why is it slow

* del callable task * move querysource to GroupByExecutor * skip chunk

…imize_series_reader

...rc/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByWithoutValueFilterDataSet.java

server/src/main/java/org/apache/iotdb/db/query/reader/series/SeriesReader.java

server/src/test/java/org/apache/iotdb/db/integration/IoTDBCloseIT.java

server/src/test/java/org/apache/iotdb/db/integration/IoTDBEngineTimeGeneratorIT.java

server/src/test/java/org/apache/iotdb/db/integration/IoTDBLargeDataIT.java

server/src/test/java/org/apache/iotdb/db/integration/IoTDBMultiSeriesIT.java

…eIT.java Co-Authored-By: Zesong Sun <szs19@mails.tsinghua.edu.cn>

…imize_series_reader

JackieTien97

Some places still needs to be fixed

JackieTien97 · 2020-03-02T12:40:28Z

...rc/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByWithoutValueFilterDataSet.java

      }
+    } catch (QueryProcessException e) {
+      logger.error("GroupByWithoutValueFilterDataSet execute has error,{}", e);


no need to use {},just
logger.error("GroupByWithoutValueFilterDataSet execute has error ", e);

JackieTien97 · 2020-03-02T12:49:24Z

server/src/test/java/org/apache/iotdb/db/integration/IoTDBMultiStatementsIT.java

@@ -85,7 +86,7 @@ private static void insertData()
            .getConnection(Config.IOTDB_URL_PREFIX + "127.0.0.1:6667/", "root", "root");
         Statement statement = connection.createStatement()) {

-      for (String sql : Constant.create_sql) {
+      for (String sql : org.apache.iotdb.db.constant.TestConstant.create_sql) {


Suggested change

for (String sql : org.apache.iotdb.db.constant.TestConstant.create_sql) {

for (String sql : TestConstant.create_sql) {

JackieTien97 · 2020-03-02T12:49:55Z

server/src/test/java/org/apache/iotdb/db/integration/IoTDBMultiStatementsIT.java

@@ -138,7 +139,7 @@ public void selectAllTest() throws ClassNotFoundException {
      int cnt1 = 0;
      while (resultSet1.next() && cnt1 < 5) {
        StringBuilder builder = new StringBuilder();
-        builder.append(resultSet1.getString(Constant.TIMESTAMP_STR))
+        builder.append(resultSet1.getString(org.apache.iotdb.db.constant.TestConstant.TIMESTAMP_STR))


Suggested change

builder.append(resultSet1.getString(org.apache.iotdb.db.constant.TestConstant.TIMESTAMP_STR))

builder.append(resultSet1.getString(TestConstant.TIMESTAMP_STR))

JackieTien97 · 2020-03-02T12:50:11Z

server/src/test/java/org/apache/iotdb/db/integration/IoTDBMultiStatementsIT.java

@@ -154,7 +155,7 @@ public void selectAllTest() throws ClassNotFoundException {
      int cnt2 = 0;
      while (resultSet2.next()) {
        StringBuilder builder = new StringBuilder();
-        builder.append(resultSet2.getString(Constant.TIMESTAMP_STR))
+        builder.append(resultSet2.getString(org.apache.iotdb.db.constant.TestConstant.TIMESTAMP_STR))


Suggested change

builder.append(resultSet2.getString(org.apache.iotdb.db.constant.TestConstant.TIMESTAMP_STR))

builder.append(resultSet2.getString(TestConstant.TIMESTAMP_STR))

JackieTien97 · 2020-03-02T12:53:11Z

server/src/test/java/org/apache/iotdb/db/tools/IoTDBWatermarkTest.java

+  private static String filePath1 = org.apache.iotdb.db.constant.TestConstant.BASE_OUTPUT_PATH.concat("watermarked_query_result.csv");
+  private static String filePath2 = org.apache.iotdb.db.constant.TestConstant.BASE_OUTPUT_PATH.concat("notWatermarked_query_result.csv");


Suggested change

private static String filePath1 = org.apache.iotdb.db.constant.TestConstant.BASE_OUTPUT_PATH.concat("watermarked_query_result.csv");

private static String filePath2 = org.apache.iotdb.db.constant.TestConstant.BASE_OUTPUT_PATH.concat("notWatermarked_query_result.csv");

private static String filePath1 = TestConstant.BASE_OUTPUT_PATH.concat("watermarked_query_result.csv");

private static String filePath2 = TestConstant.BASE_OUTPUT_PATH.concat("notWatermarked_query_result.csv");

sonarcloud · 2020-03-02T14:20:59Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
9 Code Smells

No Coverage information
0.0% Duplication

JackieTien97

LGTM

qiaojialin and others added 9 commits February 29, 2020 18:02

reorganize series reader and aggregate reader

9aa6176

fix point reader

d229e98

fix bytimestamp

18068a9

combine nextPage (#861)

1dadf07

* fix valueFilterBug for nextOverlappedPage

remove set chunkmetadata=null in hasNextChunk

fe759e8

remove hasOverlappedPage

17365a7

new groupby (#862)

34476e5

* new multithread groupby and GroupByExecutor

fix hasNextPage in SeriesReader

986f758

remove unused interfaces in IAggregateReader

b7c10da

jixuan1989 requested a review from JackieTien97 March 2, 2020 01:06

qiaojialin changed the title ~~[IOTDB-527] Optimize series reader~~ [IOTDB-527] Refactor series reader Mar 2, 2020

qiaojialin added 5 commits March 2, 2020 09:39

push down value filter and add log

3b33fbb

fix nextPage non-stop

e51e07e

make hasNextPage and nextPage more robust

0114481

add stack

d9a19dc

add mergeReaer check in isPageOverlapped

6ad42e9

JackieTien97 requested changes Mar 2, 2020

View reviewed changes

qiaojialin added 5 commits March 2, 2020 13:12

fix hasNextOverlappedPage when no data in this page under value filter

b725c72

Merge remote-tracking branch 'origin/master' into optimize_series_reader

1c39133

add log

9e85181

add filter overlapped data test and add log

f06043b

add log for point 100492

3e6a61f

qiaojialin and others added 7 commits March 2, 2020 15:55

add log in hasNextOverlappedPage

46a133b

Optimize_series_reader_fix (#867)

6ba2969

* del callable task * move querysource to GroupByExecutor * skip chunk

add log

80456bd

Merge remote-tracking branch 'origin/optimize_series_reader' into opt…

23d620a

…imize_series_reader

fix tmp bug

3954860

remove debug log

2d6e8fc

add test and fix review

60c0f5e

fix code smell

ad50583

samperson1997 requested changes Mar 2, 2020

View reviewed changes

JackieTien97 and others added 3 commits March 2, 2020 20:46

Update server/src/test/java/org/apache/iotdb/db/integration/IoTDBClos…

8f5cf8b

…eIT.java Co-Authored-By: Zesong Sun <szs19@mails.tsinghua.edu.cn>

fix review

e455a96

Merge remote-tracking branch 'origin/optimize_series_reader' into opt…

2843e8c

…imize_series_reader

JackieTien97 requested changes Mar 2, 2020

View reviewed changes

qiaojialin added 2 commits March 2, 2020 20:54

fix import

7a28519

fix review

9101bcf

JackieTien97 self-requested a review March 2, 2020 15:26

JackieTien97 approved these changes Mar 2, 2020

View reviewed changes

samperson1997 approved these changes Mar 2, 2020

View reviewed changes

qiaojialin merged commit c00b63f into master Mar 3, 2020

qiaojialin deleted the optimize_series_reader branch March 3, 2020 01:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IOTDB-527] Refactor series reader #864

[IOTDB-527] Refactor series reader #864

qiaojialin commented Mar 1, 2020

JackieTien97 left a comment

JackieTien97 Mar 2, 2020

liutaohua Mar 2, 2020

JackieTien97 Mar 2, 2020

qiaojialin Mar 2, 2020

JackieTien97 Mar 2, 2020

liutaohua Mar 2, 2020

qiaojialin Mar 2, 2020

JackieTien97 Mar 2, 2020

liutaohua Mar 2, 2020

liutaohua commented Mar 2, 2020

JackieTien97 left a comment

JackieTien97 Mar 2, 2020

qiaojialin Mar 2, 2020

JackieTien97 Mar 2, 2020

qiaojialin Mar 2, 2020

JackieTien97 Mar 2, 2020

JackieTien97 Mar 2, 2020

JackieTien97 Mar 2, 2020

sonarcloud bot commented Mar 2, 2020

JackieTien97 left a comment

	for (String sql : org.apache.iotdb.db.constant.TestConstant.create_sql) {
	for (String sql : TestConstant.create_sql) {

	builder.append(resultSet1.getString(org.apache.iotdb.db.constant.TestConstant.TIMESTAMP_STR))
	builder.append(resultSet1.getString(TestConstant.TIMESTAMP_STR))

	builder.append(resultSet2.getString(org.apache.iotdb.db.constant.TestConstant.TIMESTAMP_STR))
	builder.append(resultSet2.getString(TestConstant.TIMESTAMP_STR))

		private static String filePath1 = org.apache.iotdb.db.constant.TestConstant.BASE_OUTPUT_PATH.concat("watermarked_query_result.csv");
		private static String filePath2 = org.apache.iotdb.db.constant.TestConstant.BASE_OUTPUT_PATH.concat("notWatermarked_query_result.csv");

[IOTDB-527] Refactor series reader #864

[IOTDB-527] Refactor series reader #864

Conversation

qiaojialin commented Mar 1, 2020

JackieTien97 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liutaohua commented Mar 2, 2020

JackieTien97 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Mar 2, 2020

JackieTien97 left a comment

Choose a reason for hiding this comment