Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IOTDB-1163]optimize the insertRecords session interface for cluster version #2698

Merged
merged 6 commits into from
Feb 23, 2021

Conversation

neuyilan
Copy link
Member

@neuyilan neuyilan commented Feb 19, 2021

@HTHou HTHou added the Module - Cluster PRs for the cluster module label Feb 20, 2021
@neuyilan neuyilan changed the title optimize the insertRecords session interface for cluster version [IOTDB-1163]optimize the insertRecords session interface for cluster version Feb 20, 2021
@neuyilan neuyilan marked this pull request as ready for review February 20, 2021 04:03
Copy link
Member

@mychaow mychaow left a comment

Choose a reason for hiding this comment

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

Good Job!

plan.getResults().put(i, RpcUtils.getStatus(e.getErrorCode(), e.getMessage()));
allSuccess = false;
}
if (!allSuccess) {
Copy link
Member

Choose a reason for hiding this comment

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

do we need to insert the left records while one record is failed? @qiaojialin

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your review, in my opinion, I support inserting the remaining records in case of one record insertion failure. After all, we support partial insertion success. Moreover, this implementation is consistent with the previous semantics of composing each record into an InsertRowPlan.

@sonarcloud
Copy link

sonarcloud bot commented Feb 23, 2021

Map<Integer, TSStatus> checkTsStatus,
int totalRowCount) {

if (allSuccess) {
Copy link
Member

Choose a reason for hiding this comment

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

Changing allSuccess to allCheckSuccess is better.

@mychaow mychaow merged commit b4544d8 into master Feb 23, 2021
@qiaojialin qiaojialin deleted the optimize_insert_records branch June 16, 2021 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Module - Cluster PRs for the cluster module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants