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

[Refactor] Replace some boost to std in OlapScanNode #3934

Merged
merged 5 commits into from
Jun 29, 2020

Conversation

morningman
Copy link
Contributor

@morningman morningman commented Jun 23, 2020

Replace some boost to std in OlapScanNode.

This refactor seems solve the problem describe in #3929.
Because I found that BE will crash to calling boost::condition_variable.notify_all().
But after upgrade to this, BE does not crash any more.

@morningman morningman added the kind/refactor Issues or PRs to refactor code label Jun 23, 2020
@@ -186,9 +185,9 @@ Status OlapScanNode::get_next(RuntimeState* state, RowBatch* row_batch, bool* eo

// check if Canceled.
if (state->is_cancelled()) {
boost::unique_lock<boost::mutex> l(_row_batches_lock);
std::unique_lock<std::mutex> l(_row_batches_lock);
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use util/condition_variable.h.
It comes from kudu, it's use is simple than std::

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR is only try to solve the BE crash problem and I don't want to introduce any new potential problems.
I can't see any necessary to replace the std::condition_variable with ConditionVariable. If it is,
maybe we can replace all std::condition_variable in another PR.

@morningman morningman added the approved Indicates a PR has been approved by one committer. label Jun 28, 2020
@morningman morningman merged commit 0cbacaf into apache:master Jun 29, 2020
morningman added a commit to morningman/doris that referenced this pull request Jun 29, 2020
Replace some boost to std in OlapScanNode.

This refactor seems solve the problem describe in apache#3929.
Because I found that BE will crash to calling `boost::condition_variable.notify_all()`.
But after upgrade to this, BE does not crash any more.

Change-Id: I4baeeb6e76ecc751cb042796d52692d79432994e
wuyunfeng pushed a commit to wuyunfeng/incubator-doris that referenced this pull request Jun 30, 2020
Replace some boost to std in OlapScanNode.

This refactor seems solve the problem describe in apache#3929.
Because I found that BE will crash to calling `boost::condition_variable.notify_all()`.
But after upgrade to this, BE does not crash any more.
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. kind/refactor Issues or PRs to refactor code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants