Skip to content

[IOTDB-749] Handle select * from root OOM#1367

Merged
HTHou merged 20 commits intomasterfrom
OOMImprove
Oct 23, 2020
Merged

[IOTDB-749] Handle select * from root OOM#1367
HTHou merged 20 commits intomasterfrom
OOMImprove

Conversation

@JackieTien97
Copy link
Copy Markdown
Contributor

No description provided.


# allowed max numbers of deduplicated path in one query
# it's just an advised value, the real limitation will be the smaller one between this and the one we calculated
max_deduplicated_path_num=10000
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reduce to 1000?

@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

warning The version of Java (1.8.0_252) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

if (totalFreeMemoryForRead.addAndGet(-estimatedMemoryUsage) >= 0) {
queryIdEstimatedMemoryMap.put(queryId, estimatedMemoryUsage);
} else {
totalFreeMemoryForRead.addAndGet(estimatedMemoryUsage);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why add memory here..

private final AtomicLong totalFreeMemoryForRead;

// estimated size for one point memory size, the unit is byte
private static final long POINT_ESTIMATED_SIZE = 16L;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only works for non-string..

@jixuan1989
Copy link
Copy Markdown
Member

I register 10_000_000 timeseries by:

  public static void main(String[] args) throws ClassNotFoundException, SQLException {
    Class.forName("org.apache.iotdb.jdbc.IoTDBDriver");
    try (Connection connection = DriverManager
        .getConnection("jdbc:iotdb://127.0.0.1:6667/", "root", "root");
        Statement statement = connection.createStatement()) {
      int total = 10_000_000;
      try {
        statement.execute("SET STORAGE GROUP TO root.sg1");
        for (int i = 0; i < total; i++) {
          statement.execute(
              "CREATE TIMESERIES root.sg1.d1.s" + i
                  + " WITH DATATYPE=INT64, ENCODING=RLE, COMPRESSOR=SNAPPY");
          if (i % 10_000 == 0) {
            System.out.println((double) i / total * 100 + "%");
          }
        }
      } catch (IoTDBSQLException e) {
        System.out.println(e.getMessage());
      }
      
    } catch (IoTDBSQLException e) {
      System.out.println(e.getMessage());
    }
  }

And then call select * from root;
The CLI hangs..

Copy link
Copy Markdown
Contributor

@SilverNarcissus SilverNarcissus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look good to me~


# allowed max numbers of deduplicated path in one query
# it's just an advised value, the real limitation will be the smaller one between this and the one we calculated
max_deduplicated_path_num=10000
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my perspective, 1000 is better. XD

# Read memory Allocation Ratio: ChunkMetadataCache, ChunkCache, TimeSeriesMetadataCache and Free Memory Used in Query.
# The parameter form is a:b:c:d, where a, b, c and d are integers. for example: 1:1:1:1 , 6:10:5:15
chunkmeta_chunk_timeseriesmeta_free_memory_proportion=1:1:1:7
chunkmeta_chunk_timeseriesmeta_free_memory_proportion=1:1:1:3:4
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the value has 5 parameters, maybe the annotation and the attribute need to change?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with that. The comments need to be modified.

Copy link
Copy Markdown
Contributor

@Alima777 Alima777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are conflicts now, please fix it.

Copy link
Copy Markdown
Contributor

@Alima777 Alima777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Please fix some small issues.

# Read memory Allocation Ratio: ChunkMetadataCache, ChunkCache, TimeSeriesMetadataCache and Free Memory Used in Query.
# The parameter form is a:b:c:d, where a, b, c and d are integers. for example: 1:1:1:1 , 6:10:5:15
chunkmeta_chunk_timeseriesmeta_free_memory_proportion=1:1:1:7
chunkmeta_chunk_timeseriesmeta_free_memory_proportion=1:1:1:3:4
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with that. The comments need to be modified.

Comment on lines 198 to 205
/**
* Make 'SLIMIT&SOFFSET' take effect by trimming the suffixList and aggregations of the
* selectOperator.
*
* @param seriesLimit is ensured to be positive integer
* @param seriesOffset is ensured to be non-negative integer
*/
private void slimitTrim(SelectOperator select, int seriesLimit, int seriesOffset)
private void slimitTrim(SelectOperator select, int seriesOffset)
throws LogicalOptimizeException {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my perspective, limit is done in concatselect() while this method is only for trimming the offset of aggregations.
If I'm right, please modify the method name and java doc of it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix

* Is dynamic parameter adapter enable.
*/
private boolean enableParameterAdapter = true;
private boolean enableParameterAdapter = false;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why set it to false?

* @param prefixPath a prefix path or a full path, may contain '*'.
*/
List<PartialPath> getAllTimeseriesPathWithAlias(PartialPath prefixPath) throws MetadataException {
Pair<List<PartialPath>, Integer> getAllTimeseriesPathWithAlias(PartialPath prefixPath, int limit, int offset) throws MetadataException {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the Javadoc...

@JackieTien97
Copy link
Copy Markdown
Contributor Author

image

@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

warning The version of Java (1.8.0_252) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

@HTHou HTHou merged commit 2600bf8 into master Oct 23, 2020
@qiaojialin qiaojialin deleted the OOMImprove branch October 23, 2020 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants