Skip to content

Commit

Permalink
[#14163] yql - Improve logging in case of duplicate index error
Browse files Browse the repository at this point in the history
Summary: We show the client the same error message indicating a unique constraint error any time a unique index operation failed and the response explicitly set applied=false. While these conditions would only be met in the case of a duplicate value error, the code here invited speculation that we might be masking the true error which may be different. This revision provides an improved explanation of why this branch is ok to assume this is a unique index constraint error and adds additional VLOGs in case the doubt ever rises in production.

Test Plan: Jenkins

Reviewers: pjain, amitanand

Reviewed By: amitanand

Subscribers: kannan

Differential Revision: https://phabricator.dev.yugabyte.com/D20063
  • Loading branch information
robertsami committed Oct 19, 2022
1 parent 6876a67 commit 2800531
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/yb/docdb/cql_operation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1458,7 +1458,7 @@ Result<QLWriteRequestPB*> CreateAndSetupIndexInsertRequest(
RETURN_NOT_OK(expr_executor->EvalCondition(
index->where_predicate_spec()->where_expr().condition(), new_row,
&new_row_satisfies_idx_pred));
VLOG(1) << "Eval condition on partial index " << new_row_satisfies_idx_pred;
VLOG(2) << "Eval condition on partial index " << new_row_satisfies_idx_pred;
if (index_pred_new_row) {
*index_pred_new_row = new_row_satisfies_idx_pred;
}
Expand Down
16 changes: 10 additions & 6 deletions src/yb/tablet/write_query.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ bool CheckSchemaVersion(
return true;
}

DVLOG(1) << " On " << table_info->table_name
VLOG(1) << " On " << table_info->table_name
<< " Setting status for write as YQL_STATUS_SCHEMA_VERSION_MISMATCH tserver's: "
<< table_info->schema_version << " vs req's : " << schema_version
<< " is req compatible with prev version: "
Expand Down Expand Up @@ -310,7 +310,7 @@ Result<bool> WriteQuery::CqlPrepareExecute() {
RETURN_NOT_OK(InitExecute(ExecuteMode::kCql));

auto& metadata = *tablet().metadata();
DVLOG(2) << "Schema version for " << metadata.table_name() << ": " << metadata.schema_version();
VLOG(2) << "Schema version for " << metadata.table_name() << ": " << metadata.schema_version();

if (!CqlCheckSchemaVersion()) {
return false;
Expand Down Expand Up @@ -694,12 +694,15 @@ void WriteQuery::CompleteQLWriteBatch(const Status& status) {
ql_write_op->request().type() == QLWriteRequestPB::QL_STMT_INSERT &&
ql_write_op->response()->has_applied() && !ql_write_op->response()->applied()) {
// If this is an insert into a unique index and it fails to apply, report duplicate value err.
ql_write_op->response()->set_status(QLResponsePB::YQL_STATUS_USAGE_ERROR);
// has_applied is only true if we have evaluated the requests if_expr and is only false if
// that if_expr ws not satisfied or if the remote index was unique and had a duplicate value
// to the one we're trying to insert here.
VLOG(1) << "Could not apply operation to remote index " << AsString(ql_write_op->request())
<< " due to " << AsString(ql_write_op->response());
ql_write_op->response()->set_error_message(
Format("Duplicate value disallowed by unique index $0",
tablet().metadata()->table_name()));
DVLOG(1) << "Could not apply the given operation " << AsString(ql_write_op->request())
<< " due to " << AsString(ql_write_op->response());
ql_write_op->response()->set_status(QLResponsePB::YQL_STATUS_USAGE_ERROR);
} else if (ql_write_op->rowblock() != nullptr) {
// If the QL write op returns a rowblock, move the op to the transaction state to return the
// rows data as a sidecar after the transaction completes.
Expand Down Expand Up @@ -821,7 +824,8 @@ void WriteQuery::UpdateQLIndexesFlushed(
auto* index_response = index_op->mutable_response();

if (index_response->status() != QLResponsePB::YQL_STATUS_OK) {
DVLOG(1) << "Got status " << index_response->status() << " for " << AsString(index_op);
VLOG(1) << "Got response " << index_response->ShortDebugString()
<< " for " << AsString(index_op);
response->set_status(index_response->status());
response->set_error_message(std::move(*index_response->mutable_error_message()));
}
Expand Down

0 comments on commit 2800531

Please sign in to comment.