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 2021/03/24 06:52:36 UTC
[GitHub] [iotdb] jt2594838 commented on a change in pull request #2872: [IOTDB-1236] Improve jdbc performance for creating timeseries in cluster module
jt2594838 commented on a change in pull request #2872:
URL: https://github.com/apache/iotdb/pull/2872#discussion_r600209938
##########
File path: server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java
##########
@@ -459,8 +537,11 @@ public TSStatus executeBatchStatement(TSExecuteBatchStatementReq req) {
return RpcUtils.getStatus(TSStatusCode.NOT_LOGIN_ERROR);
}
- InsertRowsPlan insertRowsPlan = new InsertRowsPlan();
+ InsertRowsPlan insertRowsPlan;
int index = 0;
+ ArrayList executeList = new ArrayList();
Review comment:
It would be better to define a variable using interface type.
##########
File path: server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java
##########
@@ -472,19 +553,47 @@ public TSStatus executeBatchStatement(TSExecuteBatchStatementReq req) {
}
if (physicalPlan.getOperatorType().equals(OperatorType.INSERT)) {
+ if (OperatorType.INSERT == lastOperatorType) {
+ insertRowsPlan = (InsertRowsPlan) executeList.get(executeList.size() - 1);
+ } else {
+ insertRowsPlan = new InsertRowsPlan();
+ executeList.add(insertRowsPlan);
+ index = 0;
+ }
+ lastOperatorType = OperatorType.INSERT;
insertRowsPlan.addOneInsertRowPlan((InsertRowPlan) physicalPlan, index);
index++;
- if (i == req.getStatements().size() - 1
- && !executeInsertRowsPlan(insertRowsPlan, result)) {
- isAllSuccessful = false;
+ if (i == req.getStatements().size() - 1) {
+ if (!executeBatchList(executeList, result)) {
+ isAllSuccessful = false;
+ }
+ executeList = new ArrayList();
Review comment:
Is it necessary to create a new list now that you have processed the last statement?
And maybe this branch can be extracted from the loop.
##########
File path: server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java
##########
@@ -472,19 +553,47 @@ public TSStatus executeBatchStatement(TSExecuteBatchStatementReq req) {
}
if (physicalPlan.getOperatorType().equals(OperatorType.INSERT)) {
+ if (OperatorType.INSERT == lastOperatorType) {
+ insertRowsPlan = (InsertRowsPlan) executeList.get(executeList.size() - 1);
+ } else {
+ insertRowsPlan = new InsertRowsPlan();
+ executeList.add(insertRowsPlan);
+ index = 0;
+ }
+ lastOperatorType = OperatorType.INSERT;
insertRowsPlan.addOneInsertRowPlan((InsertRowPlan) physicalPlan, index);
index++;
- if (i == req.getStatements().size() - 1
- && !executeInsertRowsPlan(insertRowsPlan, result)) {
- isAllSuccessful = false;
+ if (i == req.getStatements().size() - 1) {
+ if (!executeBatchList(executeList, result)) {
+ isAllSuccessful = false;
+ }
+ executeList = new ArrayList();
+ }
+ } else if (physicalPlan.getOperatorType().equals(OperatorType.CREATE_TIMESERIES)) {
+ if (OperatorType.CREATE_TIMESERIES == lastOperatorType) {
+ multiPlan = (CreateMultiTimeSeriesPlan) executeList.get(executeList.size() - 1);
+ } else {
+ multiPlan = new CreateMultiTimeSeriesPlan();
+ executeList.add(multiPlan);
+ }
+ lastOperatorType = OperatorType.CREATE_TIMESERIES;
+ initMultiTimeSeriesPlan(multiPlan);
+
+ CreateTimeSeriesPlan createTimeSeriesPlan = (CreateTimeSeriesPlan) physicalPlan;
+ setMultiTimeSeriesPlan(multiPlan, createTimeSeriesPlan);
+ if (i == req.getStatements().size() - 1) {
+ if (!executeBatchList(executeList, result)) {
+ isAllSuccessful = false;
+ }
+ executeList = new ArrayList();
}
} else {
- if (insertRowsPlan.getRowCount() > 0) {
- if (!executeInsertRowsPlan(insertRowsPlan, result)) {
+ lastOperatorType = physicalPlan.getOperatorType();
+ if (executeList.size() > 0) {
+ if (!executeBatchList(executeList, result)) {
isAllSuccessful = false;
}
- index = 0;
- insertRowsPlan = new InsertRowsPlan();
+ executeList = new ArrayList();
Review comment:
`List.clear` should be enough here.
##########
File path: server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java
##########
@@ -447,7 +447,85 @@ private boolean executeInsertRowsPlan(InsertRowsPlan insertRowsPlan, List<TSStat
result.set(startIndex + entry.getKey(), entry.getValue());
}
}
- return tsStatus.equals(RpcUtils.SUCCESS_STATUS);
+ return tsStatus.getCode() == RpcUtils.SUCCESS_STATUS.getCode();
+ }
+
+ private boolean executeMultiTimeSeriesPlan(
+ CreateMultiTimeSeriesPlan multiPlan, List<TSStatus> result) {
+ long t1 = System.currentTimeMillis();
+ TSStatus tsStatus = executeNonQueryPlan(multiPlan);
+ Measurement.INSTANCE.addOperationLatency(Operation.EXECUTE_ROWS_PLAN_IN_BATCH, t1);
+
+ int startIndex = result.size();
+ if (startIndex > 0) {
+ startIndex = startIndex - 1;
+ }
+ for (int k = 0; k < multiPlan.getPaths().size(); k++) {
+ result.add(RpcUtils.SUCCESS_STATUS);
+ }
+ if (tsStatus.subStatus != null) {
+ for (Entry<Integer, TSStatus> entry : multiPlan.getResults().entrySet()) {
+ result.set(startIndex + entry.getKey(), entry.getValue());
+ }
+ }
+ return tsStatus.getCode() == RpcUtils.SUCCESS_STATUS.getCode();
+ }
+
+ private void initMultiTimeSeriesPlan(CreateMultiTimeSeriesPlan multiPlan) {
+ if (multiPlan.getPaths() == null) {
+ List<PartialPath> paths = new ArrayList<>();
+ List<TSDataType> tsDataTypes = new ArrayList<>();
+ List<TSEncoding> tsEncodings = new ArrayList<>();
+ List<CompressionType> tsCompressionTypes = new ArrayList<>();
+ List<Map<String, String>> tagsList = new ArrayList<>();
+ List<Map<String, String>> attributesList = new ArrayList<>();
+ List<String> aliasList = new ArrayList<>();
+ multiPlan.setPaths(paths);
+ multiPlan.setDataTypes(tsDataTypes);
+ multiPlan.setEncodings(tsEncodings);
+ multiPlan.setCompressors(tsCompressionTypes);
+ multiPlan.setTags(tagsList);
+ multiPlan.setAttributes(attributesList);
+ multiPlan.setAlias(aliasList);
+ }
+ }
+
+ private void setMultiTimeSeriesPlan(
+ CreateMultiTimeSeriesPlan multiPlan, CreateTimeSeriesPlan createTimeSeriesPlan) {
+ PartialPath path = createTimeSeriesPlan.getPath();
+ TSDataType type = createTimeSeriesPlan.getDataType();
+ TSEncoding encoding = createTimeSeriesPlan.getEncoding();
+ CompressionType compressor = createTimeSeriesPlan.getCompressor();
+ Map<String, String> tags = createTimeSeriesPlan.getTags();
+ Map<String, String> attributes = createTimeSeriesPlan.getAttributes();
+ String alias = createTimeSeriesPlan.getAlias();
+
+ multiPlan.getPaths().add(path);
+ multiPlan.getDataTypes().add(type);
+ multiPlan.getEncodings().add(encoding);
+ multiPlan.getCompressors().add(compressor);
+ multiPlan.getTags().add(tags);
+ multiPlan.getAttributes().add(attributes);
+ multiPlan.getAlias().add(alias);
+ }
+
+ private boolean executeBatchList(ArrayList executeList, List<TSStatus> result) {
+ boolean isAllSuccessful = true;
+ if (executeList.size() > 0) {
+ for (int j = 0; j < executeList.size(); j++) {
Review comment:
`if (executeList.size() > 0) {` can be removed.
##########
File path: server/src/test/java/org/apache/iotdb/db/integration/IoTDBExecuteBatchIT.java
##########
@@ -26,11 +26,7 @@
import org.junit.Before;
import org.junit.Test;
-import java.sql.Connection;
-import java.sql.DriverManager;
-import java.sql.ResultSet;
-import java.sql.SQLException;
-import java.sql.Statement;
+import java.sql.*;
Review comment:
I think we prefer not to use wildcards in imports.
--
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