You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "Sh-Zh-7 (via GitHub)" <gi...@apache.org> on 2023/10/25 12:42:37 UTC
[PR] Support batch processing in UDF interface [iotdb]
Sh-Zh-7 opened a new pull request, #11389:
URL: https://github.com/apache/iotdb/pull/11389
## Description
In this pr, we add a new batch processing UDF interface for scalar functions to avoid too much function calls and Java boxing/unboxing costs.
```java
void transform(Column[] columns, ColumnBuilder builder);
```
This modification has been observed to have huge performance benefit compared to old ones, which saves the end-to-end query time accordingly. More details can be seen in https://lz8h91usu0.feishu.cn/docx/OA9tdC8wXow1b0xu6kOcPZFqnMg
Now IoTDB only calls the batch interface internally when dealing with scalar functions. For those User Defined Functions which only implement single row method, IoTDB can still handle them correctly by calling them row by row in new method's default implementation.
While this new method use tsfile module members like `Column` and `ColumnBuilder`, and introducing such a large dependency is unacceptable for users, for the packaging time will dramatically increases, we extract a `common-api` module to expose minimum required members from tsfile. And it will be shared by other modules in the future.
<hr>
This PR has:
- [x] been self-reviewed.
- [x] added Javadocs for most classes and all non-trivial 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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support batch processing in UDF interface [iotdb]
Posted by "lancelly (via GitHub)" <gi...@apache.org>.
lancelly commented on code in PR #11389:
URL: https://github.com/apache/iotdb/pull/11389#discussion_r1372791509
##########
iotdb-api/common-api/pom.xml:
##########
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <artifactId>iotdb-api</artifactId>
+ <groupId>org.apache.iotdb</groupId>
+ <version>1.3.0-SNAPSHOT</version>
+ </parent>
+ <artifactId>common-api</artifactId>
+ <name>IoTDB: API: UDF API</name>
Review Comment:
common api？
##########
iotdb-api/common-api/pom.xml:
##########
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <artifactId>iotdb-api</artifactId>
+ <groupId>org.apache.iotdb</groupId>
+ <version>1.3.0-SNAPSHOT</version>
+ </parent>
+ <artifactId>common-api</artifactId>
+ <name>IoTDB: API: UDF API</name>
Review Comment:
common api？
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support batch processing in UDF interface [iotdb]
Posted by "Sh-Zh-7 (via GitHub)" <gi...@apache.org>.
Sh-Zh-7 commented on code in PR #11389:
URL: https://github.com/apache/iotdb/pull/11389#discussion_r1375637958
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/schema/SchemaQueryScanOperator.java:
##########
@@ -41,11 +42,13 @@
import java.util.stream.Collectors;
import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
-import static org.apache.iotdb.tsfile.read.common.block.TsBlockBuilderStatus.DEFAULT_MAX_TSBLOCK_SIZE_IN_BYTES;
public class SchemaQueryScanOperator<T extends ISchemaInfo> implements SourceOperator {
+ private static final long MAX_SIZE =
+ TSFileDescriptor.getInstance().getConfig().getMaxTsBlockSizeInBytes();
- private static final long MAX_SIZE = DEFAULT_MAX_TSBLOCK_SIZE_IN_BYTES;
+ private static final long DEFAULT_MAX_TSBLOCK_SIZE_IN_BYTES =
+ TSFileDescriptor.getInstance().getConfig().getMaxTsBlockSizeInBytes();
Review Comment:
Done.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support batch processing in UDF interface [iotdb]
Posted by "lancelly (via GitHub)" <gi...@apache.org>.
lancelly commented on code in PR #11389:
URL: https://github.com/apache/iotdb/pull/11389#discussion_r1375610620
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/schema/SchemaQueryScanOperator.java:
##########
@@ -41,11 +42,13 @@
import java.util.stream.Collectors;
import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
-import static org.apache.iotdb.tsfile.read.common.block.TsBlockBuilderStatus.DEFAULT_MAX_TSBLOCK_SIZE_IN_BYTES;
public class SchemaQueryScanOperator<T extends ISchemaInfo> implements SourceOperator {
+ private static final long MAX_SIZE =
+ TSFileDescriptor.getInstance().getConfig().getMaxTsBlockSizeInBytes();
- private static final long MAX_SIZE = DEFAULT_MAX_TSBLOCK_SIZE_IN_BYTES;
+ private static final long DEFAULT_MAX_TSBLOCK_SIZE_IN_BYTES =
+ TSFileDescriptor.getInstance().getConfig().getMaxTsBlockSizeInBytes();
Review Comment:
redundant variable?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support batch processing in UDF interface [iotdb]
Posted by "Sh-Zh-7 (via GitHub)" <gi...@apache.org>.
Sh-Zh-7 commented on code in PR #11389:
URL: https://github.com/apache/iotdb/pull/11389#discussion_r1375611578
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/schema/SchemaQueryScanOperator.java:
##########
@@ -41,11 +42,13 @@
import java.util.stream.Collectors;
import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
-import static org.apache.iotdb.tsfile.read.common.block.TsBlockBuilderStatus.DEFAULT_MAX_TSBLOCK_SIZE_IN_BYTES;
public class SchemaQueryScanOperator<T extends ISchemaInfo> implements SourceOperator {
+ private static final long MAX_SIZE =
+ TSFileDescriptor.getInstance().getConfig().getMaxTsBlockSizeInBytes();
- private static final long MAX_SIZE = DEFAULT_MAX_TSBLOCK_SIZE_IN_BYTES;
+ private static final long DEFAULT_MAX_TSBLOCK_SIZE_IN_BYTES =
+ TSFileDescriptor.getInstance().getConfig().getMaxTsBlockSizeInBytes();
Review Comment:
Both MAX_SIZE and DEFAULT_MAX_TSBLOCK_SIZE_IN_BYTES are 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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support batch processing in UDF interface [iotdb]
Posted by "Sh-Zh-7 (via GitHub)" <gi...@apache.org>.
Sh-Zh-7 commented on code in PR #11389:
URL: https://github.com/apache/iotdb/pull/11389#discussion_r1373214299
##########
iotdb-api/common-api/.gitignore:
##########
@@ -0,0 +1,38 @@
+target/
+!.mvn/wrapper/maven-wrapper.jar
+!**/src/main/**/target/
+!**/src/test/**/target/
Review Comment:
Done.
##########
iotdb-api/common-api/pom.xml:
##########
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <artifactId>iotdb-api</artifactId>
+ <groupId>org.apache.iotdb</groupId>
+ <version>1.3.0-SNAPSHOT</version>
+ </parent>
+ <artifactId>common-api</artifactId>
+ <name>IoTDB: API: UDF API</name>
Review Comment:
Done.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support batch processing in UDF interface [iotdb]
Posted by "lancelly (via GitHub)" <gi...@apache.org>.
lancelly commented on code in PR #11389:
URL: https://github.com/apache/iotdb/pull/11389#discussion_r1375616558
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/schema/SchemaQueryScanOperator.java:
##########
@@ -41,11 +42,13 @@
import java.util.stream.Collectors;
import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
-import static org.apache.iotdb.tsfile.read.common.block.TsBlockBuilderStatus.DEFAULT_MAX_TSBLOCK_SIZE_IN_BYTES;
public class SchemaQueryScanOperator<T extends ISchemaInfo> implements SourceOperator {
+ private static final long MAX_SIZE =
+ TSFileDescriptor.getInstance().getConfig().getMaxTsBlockSizeInBytes();
- private static final long MAX_SIZE = DEFAULT_MAX_TSBLOCK_SIZE_IN_BYTES;
+ private static final long DEFAULT_MAX_TSBLOCK_SIZE_IN_BYTES =
+ TSFileDescriptor.getInstance().getConfig().getMaxTsBlockSizeInBytes();
Review Comment:
Only one of them is necessary.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support batch processing in UDF interface [iotdb]
Posted by "lancelly (via GitHub)" <gi...@apache.org>.
lancelly commented on code in PR #11389:
URL: https://github.com/apache/iotdb/pull/11389#discussion_r1372834837
##########
iotdb-api/udf-api/src/main/java/org/apache/iotdb/udf/api/UDTF.java:
##########
@@ -112,6 +116,48 @@ default void transform(RowWindow rowWindow, PointCollector collector) throws Exc
// do nothing
}
+ /**
+ * When the user specifies {@link MappableRowByRowAccessStrategy} to access the original data in
+ * {@link UDTFConfigurations}, this method will be called to process the transformation. Compared
+ * to {@link #transform(Row)}, this method processes input data in batches. In a single UDF query,
+ * this method may be called multiple times.
+ *
+ * @param columns original input data columns (aligned by time)
+ * @param builder used to collect output data points
+ * @throws Exception the user can throw errors if necessary
+ * @throws UnsupportedOperationException if the user does not override this method
+ * @see MappableRowByRowAccessStrategy
+ */
+ default void transform(Column[] columns, ColumnBuilder builder) throws Exception {
+ int colCount = columns.length;
+ int rowCount = columns[0].getPositionCount();
+
+ // collect input data types from columns
+ TSDataType[] dataTypes = new TSDataType[colCount];
+ for (int i = 0; i < colCount; i++) {
+ dataTypes[i] = columns[i].getDataType();
+ }
Review Comment:
In my view, this interface, like other interfaces, should have a default empty implementation, allowing users to implement the specific interface. So I'm curious about the purpose of this implementation and RowImpl. Perhaps there are better ways to maintain compatibility with the old transform(Row) interface?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support batch processing in UDF interface [iotdb]
Posted by "lancelly (via GitHub)" <gi...@apache.org>.
lancelly commented on code in PR #11389:
URL: https://github.com/apache/iotdb/pull/11389#discussion_r1372834837
##########
iotdb-api/udf-api/src/main/java/org/apache/iotdb/udf/api/UDTF.java:
##########
@@ -112,6 +116,48 @@ default void transform(RowWindow rowWindow, PointCollector collector) throws Exc
// do nothing
}
+ /**
+ * When the user specifies {@link MappableRowByRowAccessStrategy} to access the original data in
+ * {@link UDTFConfigurations}, this method will be called to process the transformation. Compared
+ * to {@link #transform(Row)}, this method processes input data in batches. In a single UDF query,
+ * this method may be called multiple times.
+ *
+ * @param columns original input data columns (aligned by time)
+ * @param builder used to collect output data points
+ * @throws Exception the user can throw errors if necessary
+ * @throws UnsupportedOperationException if the user does not override this method
+ * @see MappableRowByRowAccessStrategy
+ */
+ default void transform(Column[] columns, ColumnBuilder builder) throws Exception {
+ int colCount = columns.length;
+ int rowCount = columns[0].getPositionCount();
+
+ // collect input data types from columns
+ TSDataType[] dataTypes = new TSDataType[colCount];
+ for (int i = 0; i < colCount; i++) {
+ dataTypes[i] = columns[i].getDataType();
+ }
Review Comment:
In my view, this interface, like other interfaces, should have a default empty implementation, allowing users to implement the specific interface. So I'm curious about the purpose of this implementation and RowImpl.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support batch processing in UDF interface [iotdb]
Posted by "lancelly (via GitHub)" <gi...@apache.org>.
lancelly commented on code in PR #11389:
URL: https://github.com/apache/iotdb/pull/11389#discussion_r1372789025
##########
iotdb-api/udf-api/pom.xml:
##########
@@ -28,6 +28,13 @@
</parent>
<artifactId>udf-api</artifactId>
<name>IoTDB: API: UDF API</name>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.iotdb</groupId>
+ <artifactId>common-api</artifactId>
Review Comment:
udf-api
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support batch processing in UDF interface [iotdb]
Posted by "Sh-Zh-7 (via GitHub)" <gi...@apache.org>.
Sh-Zh-7 commented on code in PR #11389:
URL: https://github.com/apache/iotdb/pull/11389#discussion_r1372882662
##########
iotdb-api/udf-api/src/main/java/org/apache/iotdb/udf/api/UDTF.java:
##########
@@ -112,6 +116,48 @@ default void transform(RowWindow rowWindow, PointCollector collector) throws Exc
// do nothing
}
+ /**
+ * When the user specifies {@link MappableRowByRowAccessStrategy} to access the original data in
+ * {@link UDTFConfigurations}, this method will be called to process the transformation. Compared
+ * to {@link #transform(Row)}, this method processes input data in batches. In a single UDF query,
+ * this method may be called multiple times.
+ *
+ * @param columns original input data columns (aligned by time)
+ * @param builder used to collect output data points
+ * @throws Exception the user can throw errors if necessary
+ * @throws UnsupportedOperationException if the user does not override this method
+ * @see MappableRowByRowAccessStrategy
+ */
+ default void transform(Column[] columns, ColumnBuilder builder) throws Exception {
+ int colCount = columns.length;
+ int rowCount = columns[0].getPositionCount();
+
+ // collect input data types from columns
+ TSDataType[] dataTypes = new TSDataType[colCount];
+ for (int i = 0; i < colCount; i++) {
+ dataTypes[i] = columns[i].getDataType();
+ }
Review Comment:
1. User can still customize their function logic by overriding this default implementation.
2. Currently IoTDB uses `AccessStrategy` to determine which method to execute. Since both the `transform(Row row)` and `transform(Column[] columns, ColumnBuilder builder)` are designed to deal with scalar functions, there's no need to introduce a new `AccessStrategy`. So the problem is, how does the IoTDB know which method the user has implemented, when the AccessStrategy is `MappableRowByRowAccessStrategy`. Our solution is adding a default implementation to the `transform(Column[] columns, ColumnBuilder builder)` and having it call the `transform(Row row)` row by row, and IoTDB internally only handles batch interfaces, which solves this problem.
3. As for `RowImpl`, the origin implementation constructs a row by using `ElasticSerializableRowRecordListBackedMultiColumnRow`, which belongs to `iotdb-server`. Moving this class to `common-api` would be troublesome. So I simply wrote my own `RowImpl` 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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support batch processing in UDF interface [iotdb]
Posted by "JackieTien97 (via GitHub)" <gi...@apache.org>.
JackieTien97 merged PR #11389:
URL: https://github.com/apache/iotdb/pull/11389
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support batch processing in UDF interface [iotdb]
Posted by "lancelly (via GitHub)" <gi...@apache.org>.
lancelly commented on code in PR #11389:
URL: https://github.com/apache/iotdb/pull/11389#discussion_r1373991330
##########
iotdb-api/udf-api/src/main/java/org/apache/iotdb/udf/api/UDTF.java:
##########
@@ -112,6 +116,48 @@ default void transform(RowWindow rowWindow, PointCollector collector) throws Exc
// do nothing
}
+ /**
+ * When the user specifies {@link MappableRowByRowAccessStrategy} to access the original data in
+ * {@link UDTFConfigurations}, this method will be called to process the transformation. Compared
+ * to {@link #transform(Row)}, this method processes input data in batches. In a single UDF query,
+ * this method may be called multiple times.
+ *
+ * @param columns original input data columns (aligned by time)
+ * @param builder used to collect output data points
+ * @throws Exception the user can throw errors if necessary
+ * @throws UnsupportedOperationException if the user does not override this method
+ * @see MappableRowByRowAccessStrategy
+ */
+ default void transform(Column[] columns, ColumnBuilder builder) throws Exception {
+ int colCount = columns.length;
+ int rowCount = columns[0].getPositionCount();
+
+ // collect input data types from columns
+ TSDataType[] dataTypes = new TSDataType[colCount];
+ for (int i = 0; i < colCount; i++) {
+ dataTypes[i] = columns[i].getDataType();
+ }
Review Comment:
I see👍
##########
iotdb-api/udf-api/src/main/java/org/apache/iotdb/udf/api/UDTF.java:
##########
@@ -112,6 +116,48 @@ default void transform(RowWindow rowWindow, PointCollector collector) throws Exc
// do nothing
}
+ /**
+ * When the user specifies {@link MappableRowByRowAccessStrategy} to access the original data in
+ * {@link UDTFConfigurations}, this method will be called to process the transformation. Compared
+ * to {@link #transform(Row)}, this method processes input data in batches. In a single UDF query,
+ * this method may be called multiple times.
+ *
+ * @param columns original input data columns (aligned by time)
+ * @param builder used to collect output data points
+ * @throws Exception the user can throw errors if necessary
+ * @throws UnsupportedOperationException if the user does not override this method
+ * @see MappableRowByRowAccessStrategy
+ */
+ default void transform(Column[] columns, ColumnBuilder builder) throws Exception {
+ int colCount = columns.length;
+ int rowCount = columns[0].getPositionCount();
+
+ // collect input data types from columns
+ TSDataType[] dataTypes = new TSDataType[colCount];
+ for (int i = 0; i < colCount; i++) {
+ dataTypes[i] = columns[i].getDataType();
+ }
Review Comment:
I see👍
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support batch processing in UDF interface [iotdb]
Posted by "lancelly (via GitHub)" <gi...@apache.org>.
lancelly commented on code in PR #11389:
URL: https://github.com/apache/iotdb/pull/11389#discussion_r1372787624
##########
iotdb-api/common-api/.gitignore:
##########
@@ -0,0 +1,38 @@
+target/
+!.mvn/wrapper/maven-wrapper.jar
+!**/src/main/**/target/
+!**/src/test/**/target/
Review Comment:
Delete this gitignore file？
##########
iotdb-api/common-api/.gitignore:
##########
@@ -0,0 +1,38 @@
+target/
+!.mvn/wrapper/maven-wrapper.jar
+!**/src/main/**/target/
+!**/src/test/**/target/
Review Comment:
Delete this gitignore file？
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support batch processing in UDF interface [iotdb]
Posted by "lancelly (via GitHub)" <gi...@apache.org>.
lancelly commented on code in PR #11389:
URL: https://github.com/apache/iotdb/pull/11389#discussion_r1372789025
##########
iotdb-api/udf-api/pom.xml:
##########
@@ -28,6 +28,13 @@
</parent>
<artifactId>udf-api</artifactId>
<name>IoTDB: API: UDF API</name>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.iotdb</groupId>
+ <artifactId>common-api</artifactId>
Review Comment:
udf-api
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org