Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18536 )
Change subject: IMPALA-10465: Use IGNORE variant of Kudu write operations ...................................................................... Patch Set 6: (14 comments) http://gerrit.cloudera.org:8080/#/c/18536/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18536/5//COMMIT_MSG@17 PS5, Line 17: Kud > nit: drop the duplicate 'the' Done http://gerrit.cloudera.org:8080/#/c/18536/5//COMMIT_MSG@21 PS5, Line 21: fied > nit: running Reworded. http://gerrit.cloudera.org:8080/#/c/18536/5//COMMIT_MSG@34 PS5, Line 34: - Pass core tests. : : Change-Id: I8da7c41d61b0888378b390b8b643238433eb3b52 : > Ah, maybe those changes then could go first before this patch, so this patc I decide to remove the benchmark workload from this patch. The reason is that kudu dml workload that I made is quite different from other targetted-perf workload. The query is not independent with each other. I will submit follow up patch for that benchmark along with the required script changes. http://gerrit.cloudera.org:8080/#/c/18536/5/be/src/exec/kudu-table-sink.h File be/src/exec/kudu-table-sink.h: http://gerrit.cloudera.org:8080/#/c/18536/5/be/src/exec/kudu-table-sink.h@145 PS5, Line 145: duplicate > nit: duplicate and absent key conflicts Done http://gerrit.cloudera.org:8080/#/c/18536/5/be/src/exec/kudu-table-sink.cc File be/src/exec/kudu-table-sink.cc: http://gerrit.cloudera.org:8080/#/c/18536/5/be/src/exec/kudu-table-sink.cc@61 PS5, Line 61: te and abse > I'm not sure that 'by default' here provides enough context: if there is an Reworded. http://gerrit.cloudera.org:8080/#/c/18536/5/be/src/exec/kudu-table-sink.cc@354 PS5, Line 354: } > Make sense. Will do. Done http://gerrit.cloudera.org:8080/#/c/18536/5/be/src/exec/kudu-table-sink.cc@356 PS5, Line 356: Status KuduTableSink::CheckForErrors(RuntimeState* state) { : RETURN_IF_ERROR(state->CheckQueryState()); > IIRC, there might be any mix of errors in a Kudu session. Deleted. http://gerrit.cloudera.org:8080/#/c/18536/5/be/src/exec/kudu-table-sink.cc@360 PS5, Line 360: const ResourceMetrics& metrics = session_->GetWriteOpMetrics(); : int64_t ignored_errors = metrics.GetMetric("insert_ignore_errors") : + metrics.GetMetric("update_ignore_errors") : > Ah, probably I see what you mean -- in the absence of XXX_IGNORE ops there Added kudu_ignore_conflicts_in_transaction flag for more fine grained control. http://gerrit.cloudera.org:8080/#/c/18536/5/be/src/exec/kudu-table-sink.cc@364 PS5, Line 364: DCHECK(ignored_errors > All right, UPSERT is out of question here but what about other session erro Removed. We're not returning early anymore to anticipate for other kind of Kudu error message. http://gerrit.cloudera.org:8080/#/c/18536/5/be/src/exec/kudu-table-sink.cc@433 PS5, Line 433: state->dml_exec_state()->SetKuduDmlStats( > Does it make sense to zero out total_ignored_errors_ at this point as well? Done http://gerrit.cloudera.org:8080/#/c/18536/5/fe/src/main/java/org/apache/impala/planner/KuduTableSink.java File fe/src/main/java/org/apache/impala/planner/KuduTableSink.java: http://gerrit.cloudera.org:8080/#/c/18536/5/fe/src/main/java/org/apache/impala/planner/KuduTableSink.java@59 PS5, Line 59: cluste > cluster Done http://gerrit.cloudera.org:8080/#/c/18536/5/fe/src/main/java/org/apache/impala/planner/KuduTableSink.java@70 PS5, Line 70: cluste > cluster Done http://gerrit.cloudera.org:8080/#/c/18536/5/fe/src/main/java/org/apache/impala/planner/KuduTableSink.java@75 PS5, Line 75: supportsIgnoreOperations_ = client.supportsIgnoreOperations(); > SGTM! Append "Log Kudu Conflicts" to ExecOption info in query profile if Impala is not using the XXX_IGNORE operation. http://gerrit.cloudera.org:8080/#/c/18536/5/fe/src/main/java/org/apache/impala/planner/KuduTableSink.java@77 PS5, Line 77: ignore > nit: ignore ? Done -- To view, visit http://gerrit.cloudera.org:8080/18536 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8da7c41d61b0888378b390b8b643238433eb3b52 Gerrit-Change-Number: 18536 Gerrit-PatchSet: 6 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Tue, 24 May 2022 16:46:32 +0000 Gerrit-HasComments: Yes
