Skip to content

[improvement](shutdown) not print thread pool error stack trace when shutdown - #24155

Merged
yiguolei merged 5 commits into
apache:masterfrom
yiguolei:fix_shutdown_stack
Sep 11, 2023
Merged

[improvement](shutdown) not print thread pool error stack trace when shutdown#24155
yiguolei merged 5 commits into
apache:masterfrom
yiguolei:fix_shutdown_stack

Conversation

@yiguolei

@yiguoleiyiguolei commented Sep 11, 2023

Copy link
Copy Markdown
Contributor

Proposed changes

  1. when thread pool shutdown, should not print error stack trace, it is very confuse.
  2. arrow flight server should not call shutdown, if it is not enabled, because it will print error stack.
  3. remove service unavailable from thrift because it is useless.

Part of this PR need to pick to 2.0 branch.

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@github-actions

Copy link
Copy Markdown
Contributor

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

@yiguolei

Copy link
Copy Markdown
ContributorAuthor

run buildall

@github-actions

Copy link
Copy Markdown
Contributor

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

@RoanHeNaNRoanHeNaN left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@github-actions

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@xinyiZzzxinyiZzz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@github-actionsgithub-actionsBot added the approved Indicates a PR has been approved by one committer. label Sep 11, 2023
@github-actions

Copy link
Copy Markdown
Contributor

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

@wangbowangbo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Gabriel39Gabriel39 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@doris-robot

Copy link
Copy Markdown

TeamCity be ut coverage result:
Function Coverage: 36.92% (7914/21433)
Line Coverage: 28.96% (63598/219621)
Region Coverage: 27.88% (32989/118318)
Branch Coverage: 24.45% (16933/69260)
Coverage Report: http://coverage.selectdb-in.cc/coverage/3a03fd883477290696cd6249266980bf367e7acf_3a03fd883477290696cd6249266980bf367e7acf/report/index.html

@yiguolei
yiguolei merged commit 134b210 into apache:masterSep 11, 2023
yiguolei added a commit that referenced this pull request Sep 11, 2023
…shutdown (#24155)
* [improvement](shutdown) not print thread pool error stack trace when shutdown
when thread pool shutdown, should not print error stack trace, it is very confuse.
arrow flight server should not call shutdown, if it is not enabled, because it will print error stack.
remove service unavailable from thrift because it is useless.
Part of this PR need to pick to 2.0 branch.
Co-authored-by: yiguolei <yiguolei@gmail.com>
xiaokang pushed a commit that referenced this pull request Sep 13, 2023
…shutdown (#24155)
* [improvement](shutdown) not print thread pool error stack trace when shutdown
when thread pool shutdown, should not print error stack trace, it is very confuse.
arrow flight server should not call shutdown, if it is not enabled, because it will print error stack.
remove service unavailable from thrift because it is useless.
Part of this PR need to pick to 2.0 branch.
Co-authored-by: yiguolei <yiguolei@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approvedIndicates a PR has been approved by one committer.dev/2.0.2-mergedreviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@yiguolei@doris-robot@wangbo@xinyiZzz@Gabriel39@RoanHeNaN@Doris-Extras