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

[fix](multi-catalog) Revert #36575 and check nullptr of data column #37086

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

AshinGau
Copy link
Member

@AshinGau AshinGau commented Jul 1, 2024

Proposed changes

Revert #36575, because VScanner::get_block will check DCHECK(block->rows() == 0), so block should be cleared when eof = true.

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@AshinGau
Copy link
Member Author

AshinGau commented Jul 1, 2024

run buildall

Copy link
Contributor

github-actions bot commented Jul 1, 2024

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.34% (9151/25185)
Line Coverage: 27.88% (74680/267891)
Region Coverage: 26.79% (38513/143782)
Branch Coverage: 23.48% (19520/83136)
Coverage Report: http://coverage.selectdb-in.cc/coverage/8aafc51275fb21c6dcd1adfe2403df7402e76e45_8aafc51275fb21c6dcd1adfe2403df7402e76e45/report/index.html

Copy link
Contributor

@morningman morningman left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Jul 1, 2024
Copy link
Contributor

github-actions bot commented Jul 1, 2024

PR approved by at least one committer and no changes requested.

Copy link
Contributor

github-actions bot commented Jul 1, 2024

PR approved by anyone and no changes requested.

Copy link
Contributor

@kaka11chen kaka11chen left a comment

Choose a reason for hiding this comment

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

LGTM

@morningman morningman merged commit d0eea38 into apache:branch-2.1 Jul 2, 2024
22 of 24 checks passed
morningman pushed a commit that referenced this pull request Jul 13, 2024
…er low mem exception. (#36574)

## Proposed changes

### Issue

```
SIGSEGV address not mapped to object (@0x8) received by PID 2264997 (TID 2267559 OR 0x7ff60b2a4640) from PID 8; stack trace: ***
 0# doris::signal::(anonymous namespace)::FailureSignalHandler(int, siginfo_t*, void*) at /home/zcp/repo_center/doris_branch-2.1/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# 0x00007FFB96D77520 in /lib/x86_64-linux-gnu/libc.so.6
 4# doris::vectorized::Block::clear_column_data(int) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/core/block.cpp:705
 5# doris::vectorized::OrcReader::get_next_block_impl(doris::vectorized::Block*, unsigned long*, bool*) in /mnt/hdd01/PERFORMANCE_ENV/be/lib/doris_be
 6# doris::vectorized::OrcReader::get_next_block(doris::vectorized::Block*, unsigned long*, bool*) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/exec/format/orc/vorc_reader.cpp:1533
 7# doris::vectorized::VFileScanner::_get_block_wrapped(doris::RuntimeState*, doris::vectorized::Block*, bool*) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/exec/scan/vfile_scanner.cpp:355
 8# doris::vectorized::VFileScanner::_get_block_impl(doris::RuntimeState*, doris::vectorized::Block*, bool*) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/exec/scan/vfile_scanner.cpp:298
 9# doris::vectorized::VScanner::get_block(doris::RuntimeState*, doris::vectorized::Block*, bool*) in /mnt/hdd01/PERFORMANCE_ENV/be/lib/doris_be
10# doris::vectorized::VScanner::get_block_after_projects(doris::RuntimeState*, doris::vectorized::Block*, bool*) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/exec/scan/vscanner.cpp:96
11# doris::vectorized::ScannerScheduler::_scanner_scan(std::shared_ptr<doris::vectorized::ScannerContext>, std::shared_ptr<doris::vectorized::ScanTask>) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/exec/scan/scanner_scheduler.cpp:258
```
### Root cause
It is found that when executing conjuncts expressions when there is
insufficient memory, column names and types may be inserted, but the
columns are not inserted into the block, which eventually leads to a
crash.

If an exception occurs after `block->insert({nullptr, _data_type,
_expr_name});`, it will lead to the situation mentioned above
```
Status VectorizedFnCall::_do_execute(doris::vectorized::VExprContext* context,
                                     doris::vectorized::Block* block, int* result_column_id,
                                     std::vector<size_t>& args) {
   ...
    // prepare a column to save result
    block->insert({nullptr, _data_type, _expr_name});
    if (_can_fast_execute) {
        auto can_fast_execute = fast_execute(*block, args, num_columns_without_result,
                                             block->rows(), _function->get_name());
        if (can_fast_execute) {
            *result_column_id = num_columns_without_result;
            return Status::OK();
        }
    }
```

### Solution
#37086
morningman pushed a commit to morningman/doris that referenced this pull request Jul 15, 2024
…lumn (apache#37086)

Revert apache#36575, because `VScanner::get_block` will check
`DCHECK(block->rows() == 0)`, so block should be cleared when `eof =
true`.
seawinde pushed a commit to seawinde/doris that referenced this pull request Jul 17, 2024
…er low mem exception. (apache#36574)

## Proposed changes

### Issue

```
SIGSEGV address not mapped to object (@0x8) received by PID 2264997 (TID 2267559 OR 0x7ff60b2a4640) from PID 8; stack trace: ***
 0# doris::signal::(anonymous namespace)::FailureSignalHandler(int, siginfo_t*, void*) at /home/zcp/repo_center/doris_branch-2.1/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# 0x00007FFB96D77520 in /lib/x86_64-linux-gnu/libc.so.6
 4# doris::vectorized::Block::clear_column_data(int) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/core/block.cpp:705
 5# doris::vectorized::OrcReader::get_next_block_impl(doris::vectorized::Block*, unsigned long*, bool*) in /mnt/hdd01/PERFORMANCE_ENV/be/lib/doris_be
 6# doris::vectorized::OrcReader::get_next_block(doris::vectorized::Block*, unsigned long*, bool*) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/exec/format/orc/vorc_reader.cpp:1533
 7# doris::vectorized::VFileScanner::_get_block_wrapped(doris::RuntimeState*, doris::vectorized::Block*, bool*) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/exec/scan/vfile_scanner.cpp:355
 8# doris::vectorized::VFileScanner::_get_block_impl(doris::RuntimeState*, doris::vectorized::Block*, bool*) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/exec/scan/vfile_scanner.cpp:298
 9# doris::vectorized::VScanner::get_block(doris::RuntimeState*, doris::vectorized::Block*, bool*) in /mnt/hdd01/PERFORMANCE_ENV/be/lib/doris_be
10# doris::vectorized::VScanner::get_block_after_projects(doris::RuntimeState*, doris::vectorized::Block*, bool*) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/exec/scan/vscanner.cpp:96
11# doris::vectorized::ScannerScheduler::_scanner_scan(std::shared_ptr<doris::vectorized::ScannerContext>, std::shared_ptr<doris::vectorized::ScanTask>) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/exec/scan/scanner_scheduler.cpp:258
```
### Root cause
It is found that when executing conjuncts expressions when there is
insufficient memory, column names and types may be inserted, but the
columns are not inserted into the block, which eventually leads to a
crash.

If an exception occurs after `block->insert({nullptr, _data_type,
_expr_name});`, it will lead to the situation mentioned above
```
Status VectorizedFnCall::_do_execute(doris::vectorized::VExprContext* context,
                                     doris::vectorized::Block* block, int* result_column_id,
                                     std::vector<size_t>& args) {
   ...
    // prepare a column to save result
    block->insert({nullptr, _data_type, _expr_name});
    if (_can_fast_execute) {
        auto can_fast_execute = fast_execute(*block, args, num_columns_without_result,
                                             block->rows(), _function->get_name());
        if (can_fast_execute) {
            *result_column_id = num_columns_without_result;
            return Status::OK();
        }
    }
```

### Solution
apache#37086
dataroaring pushed a commit that referenced this pull request Jul 17, 2024
…er low mem exception. (#36574)

## Proposed changes

### Issue

```
SIGSEGV address not mapped to object (@0x8) received by PID 2264997 (TID 2267559 OR 0x7ff60b2a4640) from PID 8; stack trace: ***
 0# doris::signal::(anonymous namespace)::FailureSignalHandler(int, siginfo_t*, void*) at /home/zcp/repo_center/doris_branch-2.1/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# 0x00007FFB96D77520 in /lib/x86_64-linux-gnu/libc.so.6
 4# doris::vectorized::Block::clear_column_data(int) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/core/block.cpp:705
 5# doris::vectorized::OrcReader::get_next_block_impl(doris::vectorized::Block*, unsigned long*, bool*) in /mnt/hdd01/PERFORMANCE_ENV/be/lib/doris_be
 6# doris::vectorized::OrcReader::get_next_block(doris::vectorized::Block*, unsigned long*, bool*) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/exec/format/orc/vorc_reader.cpp:1533
 7# doris::vectorized::VFileScanner::_get_block_wrapped(doris::RuntimeState*, doris::vectorized::Block*, bool*) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/exec/scan/vfile_scanner.cpp:355
 8# doris::vectorized::VFileScanner::_get_block_impl(doris::RuntimeState*, doris::vectorized::Block*, bool*) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/exec/scan/vfile_scanner.cpp:298
 9# doris::vectorized::VScanner::get_block(doris::RuntimeState*, doris::vectorized::Block*, bool*) in /mnt/hdd01/PERFORMANCE_ENV/be/lib/doris_be
10# doris::vectorized::VScanner::get_block_after_projects(doris::RuntimeState*, doris::vectorized::Block*, bool*) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/exec/scan/vscanner.cpp:96
11# doris::vectorized::ScannerScheduler::_scanner_scan(std::shared_ptr<doris::vectorized::ScannerContext>, std::shared_ptr<doris::vectorized::ScanTask>) at /home/zcp/repo_center/doris_branch-2.1/doris/be/src/vec/exec/scan/scanner_scheduler.cpp:258
```
### Root cause
It is found that when executing conjuncts expressions when there is
insufficient memory, column names and types may be inserted, but the
columns are not inserted into the block, which eventually leads to a
crash.

If an exception occurs after `block->insert({nullptr, _data_type,
_expr_name});`, it will lead to the situation mentioned above
```
Status VectorizedFnCall::_do_execute(doris::vectorized::VExprContext* context,
                                     doris::vectorized::Block* block, int* result_column_id,
                                     std::vector<size_t>& args) {
   ...
    // prepare a column to save result
    block->insert({nullptr, _data_type, _expr_name});
    if (_can_fast_execute) {
        auto can_fast_execute = fast_execute(*block, args, num_columns_without_result,
                                             block->rows(), _function->get_name());
        if (can_fast_execute) {
            *result_column_id = num_columns_without_result;
            return Status::OK();
        }
    }
```

### Solution
#37086
@yiguolei yiguolei mentioned this pull request Jul 19, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. dev/2.1.5-merged reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants