Skip to content

Commit

Permalink
[fix](partial update) update error code when failed to fill the missi…
Browse files Browse the repository at this point in the history
…ng fields (apache#29103)

1. InternalError is not clear for such error, use InvalidSchema Error instead
2. avoid some useless stacktrace on InternalError when load failed
  • Loading branch information
zhannngchen authored Dec 28, 2023
1 parent 1284975 commit e5b6826
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 18 deletions.
3 changes: 2 additions & 1 deletion be/src/olap/memtable_flush_executor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ void FlushToken::_flush_memtable(MemTable* mem_table, int32_t segment_id,
}
if (!s.ok()) {
std::lock_guard wrlk(_flush_status_lock);
LOG(WARNING) << "Flush memtable failed with res = " << s;
LOG(WARNING) << "Flush memtable failed with res = " << s
<< ", load_id: " << print_id(_rowset_writer->load_id());
_flush_status = s;
return;
}
Expand Down
15 changes: 12 additions & 3 deletions be/src/olap/rowset/segment_v2/segment_writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -471,9 +471,18 @@ Status SegmentWriter::append_block_with_partial_content(const vectorized::Block*
}

if (!_opts.rowset_ctx->partial_update_info->can_insert_new_rows_in_partial_update) {
return Status::InternalError(
"the unmentioned columns should have default value or be nullable for "
"newly inserted rows in non-strict mode partial update");
std::string error_column;
for (auto cid : _opts.rowset_ctx->partial_update_info->missing_cids) {
const TabletColumn& col = _tablet_schema->column(cid);
if (!col.has_default_value() && !col.is_nullable()) {
error_column = col.name();
break;
}
}
return Status::Error<INVALID_SCHEMA, false>(
"the unmentioned column `{}` should have default value or be nullable for "
"newly inserted rows in non-strict mode partial update",
error_column);
}
has_default_or_nullable = true;
use_default_or_null_flag.emplace_back(true);
Expand Down
15 changes: 12 additions & 3 deletions be/src/olap/rowset/segment_v2/vertical_segment_writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -405,9 +405,18 @@ Status VerticalSegmentWriter::_append_block_with_partial_content(RowsInBlock& da
}

if (!_opts.rowset_ctx->partial_update_info->can_insert_new_rows_in_partial_update) {
return Status::InternalError(
"the unmentioned columns should have default value or be nullable for "
"newly inserted rows in non-strict mode partial update");
std::string error_column;
for (auto cid : _opts.rowset_ctx->partial_update_info->missing_cids) {
const TabletColumn& col = _tablet_schema->column(cid);
if (!col.has_default_value() && !col.is_nullable()) {
error_column = col.name();
break;
}
}
return Status::Error<INVALID_SCHEMA, false>(
"the unmentioned column `{}` should have default value or be nullable for "
"newly inserted rows in non-strict mode partial update",
error_column);
}
has_default_or_nullable = true;
use_default_or_null_flag.emplace_back(true);
Expand Down
20 changes: 11 additions & 9 deletions be/src/vec/sink/writer/vtablet_writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,16 +176,16 @@ void IndexChannel::mark_as_failed(const VNodeChannel* node_channel, const std::s
_failed_channels_msgs.emplace(the_tablet_id,
err + ", host: " + node_channel->host());
if (_failed_channels[the_tablet_id].size() >= ((_parent->_num_replicas + 1) / 2)) {
_intolerable_failure_status =
Status::InternalError(_failed_channels_msgs[the_tablet_id]);
_intolerable_failure_status = Status::Error<ErrorCode::INTERNAL_ERROR, false>(
_failed_channels_msgs[the_tablet_id]);
}
}
} else {
_failed_channels[tablet_id].insert(node_id);
_failed_channels_msgs.emplace(tablet_id, err + ", host: " + node_channel->host());
if (_failed_channels[tablet_id].size() >= ((_parent->_num_replicas + 1) / 2)) {
_intolerable_failure_status =
Status::InternalError(_failed_channels_msgs[tablet_id]);
_intolerable_failure_status = Status::Error<ErrorCode::INTERNAL_ERROR, false>(
_failed_channels_msgs[tablet_id]);
}
}
}
Expand Down Expand Up @@ -432,7 +432,7 @@ Status VNodeChannel::open_wait() {
_cancelled = true;
auto error_code = open_callback->cntl_->ErrorCode();
auto error_text = open_callback->cntl_->ErrorText();
return Status::InternalError(
return Status::Error<ErrorCode::INTERNAL_ERROR, false>(
"failed to open tablet writer, error={}, error_text={}, info={}",
berror(error_code), error_text, channel_info());
}
Expand Down Expand Up @@ -466,7 +466,8 @@ Status VNodeChannel::add_block(vectorized::Block* block, const Payload* payload,
if (!st.ok()) {
if (_cancelled) {
std::lock_guard<doris::SpinLock> l(_cancel_msg_lock);
return Status::InternalError("add row failed. {}", _cancel_msg);
return Status::Error<ErrorCode::INTERNAL_ERROR, false>("add row failed. {}",
_cancel_msg);
} else {
return std::move(st.prepend("already stopped, can't add row. cancelled/eos: "));
}
Expand Down Expand Up @@ -872,7 +873,8 @@ Status VNodeChannel::close_wait(RuntimeState* state) {
if (!st.ok()) {
if (_cancelled) {
std::lock_guard<doris::SpinLock> l(_cancel_msg_lock);
return Status::InternalError("wait close failed. {}", _cancel_msg);
return Status::Error<ErrorCode::INTERNAL_ERROR, false>("wait close failed. {}",
_cancel_msg);
} else {
return std::move(
st.prepend("already stopped, skip waiting for close. cancelled/!eos: "));
Expand Down Expand Up @@ -902,7 +904,7 @@ Status VNodeChannel::close_wait(RuntimeState* state) {
return Status::OK();
}

return Status::InternalError(get_cancel_msg());
return Status::Error<ErrorCode::INTERNAL_ERROR, false>(get_cancel_msg());
}

void VNodeChannel::_close_check() {
Expand Down Expand Up @@ -1049,7 +1051,7 @@ Status VTabletWriter::open(doris::RuntimeState* state, doris::RuntimeProfile* pr

// start to send batch continually. this must be called after _init
if (bthread_start_background(&_sender_thread, nullptr, periodic_send_batch, (void*)this) != 0) {
return Status::Error<INTERNAL_ERROR>("bthread_start_backgroud failed");
return Status::Error<ErrorCode::INTERNAL_ERROR>("bthread_start_backgroud failed");
}
return Status::OK();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ suite("nereids_partial_update_native_insert_stmt", "p0") {
// but field `name` is not nullable and doesn't have default value
test {
sql """insert into ${tableName3}(id,score) values(2,400),(1,200),(4,400)"""
exception "INTERNAL_ERROR"
exception "the unmentioned column `name` should have default value or be nullable"
}
sql "set enable_unique_key_partial_update=false;"
sql "sync;"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ suite("test_partial_update_native_insert_stmt", "p0") {
// but field `name` is not nullable and doesn't have default value
test {
sql """insert into ${tableName3}(id,score) values(2,400),(1,200),(4,400)"""
exception "INTERNAL_ERROR"
exception "the unmentioned column `name` should have default value or be nullable"
}
sql "set enable_unique_key_partial_update=false;"
sql "sync;"
Expand Down

0 comments on commit e5b6826

Please sign in to comment.