-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
[Fix](hive-writer) Fix crash when hive partition writer building partition update. #35311
Conversation
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
run buildall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
} | ||
} | ||
|
||
Status VHivePartitionWriter::_open_internal(RuntimeState* state, RuntimeProfile* profile) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: function '_open_internal' exceeds recommended size/complexity thresholds [readability-function-size]
Status VHivePartitionWriter::_open_internal(RuntimeState* state, RuntimeProfile* profile) {
^
Additional context
be/src/vec/sink/writer/vhive_partition_writer.cpp:65: 94 lines including whitespace and comments (threshold 80)
Status VHivePartitionWriter::_open_internal(RuntimeState* state, RuntimeProfile* profile) {
^
955ecd6
to
e24fd30
Compare
run buildall |
TPC-H: Total hot run time: 39869 ms
|
TeamCity be ut coverage result: |
TPC-DS: Total hot run time: 169482 ms
|
ClickBench: Total hot run time: 31.11 s
|
7602eb9
to
e9b42ed
Compare
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
clang-tidy review says "All clean, LGTM! 👍" |
e9b42ed
to
f747df6
Compare
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 39987 ms
|
TPC-DS: Total hot run time: 171542 ms
|
ClickBench: Total hot run time: 31.7 s
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
Issue: #31442 ``` /home/zcp/repo_center/doris_master/doris/be/src/common/signal_handler.h:421 1# PosixSignals::chained_handler(int, siginfo*, void*) [clone .part.0] in /usr/lib/jvm/java-17-openjdk-amd64/lib/server/libjvm.so 2# JVM_handle_linux_signal in /usr/lib/jvm/java-17-openjdk-amd64/lib/server/libjvm.so 3# 0x00007F963FA9D090 in /lib/x86_64-linux-gnu/libc.so.6 4# doris::vectorized::VHivePartitionWriter::_build_partition_update() at /home/zcp/repo_center/doris_master/doris/be/src/vec/sink/writer/vhive_partition_writer.cpp:215 5# doris::vectorized::VHivePartitionWriter::close(doris::Status const&) at /home/zcp/repo_center/doris_master/doris/be/src/vec/sink/writer/vhive_partition_writer.cpp:164 6# doris::vectorized::VHiveTableWriter::close(doris::Status) at /home/zcp/repo_center/doris_master/doris/be/src/vec/sink/writer/vhive_table_writer.cpp:209 7# doris::vectorized::AsyncResultWriter::process_block(doris::RuntimeState*, doris::RuntimeProfile*) at /home/zcp/repo_center/doris_master/doris/be/src/vec/sink/writer/async_result_writer.cpp:184 8# doris::vectorized::AsyncResultWriter::start_writer(doris::RuntimeState*, doris::RuntimeProfile*)::$_0::operator()() const at ```
Issue: #31442 ``` /home/zcp/repo_center/doris_master/doris/be/src/common/signal_handler.h:421 1# PosixSignals::chained_handler(int, siginfo*, void*) [clone .part.0] in /usr/lib/jvm/java-17-openjdk-amd64/lib/server/libjvm.so 2# JVM_handle_linux_signal in /usr/lib/jvm/java-17-openjdk-amd64/lib/server/libjvm.so 3# 0x00007F963FA9D090 in /lib/x86_64-linux-gnu/libc.so.6 4# doris::vectorized::VHivePartitionWriter::_build_partition_update() at /home/zcp/repo_center/doris_master/doris/be/src/vec/sink/writer/vhive_partition_writer.cpp:215 5# doris::vectorized::VHivePartitionWriter::close(doris::Status const&) at /home/zcp/repo_center/doris_master/doris/be/src/vec/sink/writer/vhive_partition_writer.cpp:164 6# doris::vectorized::VHiveTableWriter::close(doris::Status) at /home/zcp/repo_center/doris_master/doris/be/src/vec/sink/writer/vhive_table_writer.cpp:209 7# doris::vectorized::AsyncResultWriter::process_block(doris::RuntimeState*, doris::RuntimeProfile*) at /home/zcp/repo_center/doris_master/doris/be/src/vec/sink/writer/async_result_writer.cpp:184 8# doris::vectorized::AsyncResultWriter::start_writer(doris::RuntimeState*, doris::RuntimeProfile*)::$_0::operator()() const at ```
Issue: apache#31442 ``` /home/zcp/repo_center/doris_master/doris/be/src/common/signal_handler.h:421 1# PosixSignals::chained_handler(int, siginfo*, void*) [clone .part.0] in /usr/lib/jvm/java-17-openjdk-amd64/lib/server/libjvm.so 2# JVM_handle_linux_signal in /usr/lib/jvm/java-17-openjdk-amd64/lib/server/libjvm.so 3# 0x00007F963FA9D090 in /lib/x86_64-linux-gnu/libc.so.6 4# doris::vectorized::VHivePartitionWriter::_build_partition_update() at /home/zcp/repo_center/doris_master/doris/be/src/vec/sink/writer/vhive_partition_writer.cpp:215 5# doris::vectorized::VHivePartitionWriter::close(doris::Status const&) at /home/zcp/repo_center/doris_master/doris/be/src/vec/sink/writer/vhive_partition_writer.cpp:164 6# doris::vectorized::VHiveTableWriter::close(doris::Status) at /home/zcp/repo_center/doris_master/doris/be/src/vec/sink/writer/vhive_table_writer.cpp:209 7# doris::vectorized::AsyncResultWriter::process_block(doris::RuntimeState*, doris::RuntimeProfile*) at /home/zcp/repo_center/doris_master/doris/be/src/vec/sink/writer/async_result_writer.cpp:184 8# doris::vectorized::AsyncResultWriter::start_writer(doris::RuntimeState*, doris::RuntimeProfile*)::$_0::operator()() const at ```
### Issue `The specified upload does not exist. The upload ID may be invalid, or the upload may have been aborted or completed. (Service: S3, Status Code: 404, Request ID: 66557027E897233333FFC198)` ### Root cause #35311 adjusted the order of building hive partition update information. This change caused the update id to be unable to be obtained, causing the s3 file committer to not work properly. ### Solution Because uploading to s3 occurs after s3 file writer close(), close() must be called first, and then the build hive partiton update information is called.
### Issue `The specified upload does not exist. The upload ID may be invalid, or the upload may have been aborted or completed. (Service: S3, Status Code: 404, Request ID: 66557027E897233333FFC198)` ### Root cause apache#35311 adjusted the order of building hive partition update information. This change caused the update id to be unable to be obtained, causing the s3 file committer to not work properly. ### Solution Because uploading to s3 occurs after s3 file writer close(), close() must be called first, and then the build hive partiton update information is called.
### Issue `The specified upload does not exist. The upload ID may be invalid, or the upload may have been aborted or completed. (Service: S3, Status Code: 404, Request ID: 66557027E897233333FFC198)` ### Root cause #35311 adjusted the order of building hive partition update information. This change caused the update id to be unable to be obtained, causing the s3 file committer to not work properly. ### Solution Because uploading to s3 occurs after s3 file writer close(), close() must be called first, and then the build hive partiton update information is called.
Proposed changes
Issue: #31442
Root cause
Building hive partition update information use
_file_format_transformer->written_len()
, but _file_format_transformer will be nullptr when hivePartitionWriteropen()
failed (return error status).Solution
Further comments
If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...