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

dir, stored: start statistics threads only if needed #1040

Conversation

alaaeddineelamri
Copy link
Contributor

@alaaeddineelamri alaaeddineelamri commented Dec 30, 2021

Description:

The director statistics thread starts even when not needed. In order to optimize and have less unnecessary workload, the statistics thread for the director is not started by default, unless collect statistics and statistics collect interval are both specified.

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General
  • PR name is meaningful
  • Purpose of the PR is understood
  • Separate commit for this PR in the CHANGELOG.md, PR number referenced is same
  • Commit descriptions are understandable and well formatted
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
  • bareos-check-sources --since-merge does not report any problems
  • git status should not report modifications in the source tree after building and testing

@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/change-director-default-stat-thread-behavior branch 4 times, most recently from 4cf3c5d to 8591cd0 Compare January 3, 2022 14:11
@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/change-director-default-stat-thread-behavior branch from 9370a04 to e73e565 Compare January 7, 2022 15:14
@pstorz pstorz self-assigned this Jan 10, 2022
core/src/dird/stats.cc Outdated Show resolved Hide resolved
core/src/tests/addresses_and_ports.cc Outdated Show resolved Hide resolved
@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/change-director-default-stat-thread-behavior branch 2 times, most recently from 8c19271 to ef6947b Compare January 12, 2022 12:09
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

The code that handles the return status of StartStatisticsThread() in both storage and director needs to be added.
Good work!

core/src/dird/stats.cc Outdated Show resolved Hide resolved
@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/change-director-default-stat-thread-behavior branch 2 times, most recently from 82df1d7 to 8f34523 Compare January 20, 2022 08:29
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

Good work, however I would use M_ERROR_TERM instead of M_WARNING if the statistics thread is required but cannot be started. This probably is a serious problem.

Other than that, the last commit message could be reformatted as it has a single "s" in one line that needs to be removed.

Thank you!

core/src/dird/dird.cc Outdated Show resolved Hide resolved
core/src/dird/dird.cc Outdated Show resolved Hide resolved
core/src/stored/stored.cc Outdated Show resolved Hide resolved
@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/change-director-default-stat-thread-behavior branch 3 times, most recently from 21edc97 to 502f282 Compare January 28, 2022 13:32
@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/change-director-default-stat-thread-behavior branch from 34f81ff to c1c9bf3 Compare February 1, 2022 10:08
@pstorz pstorz changed the title dir: Change director default statistics thread behavior dir, stored: start statistics threads only if needed Feb 4, 2022
@pstorz pstorz force-pushed the dev/alaaeddineelamri/master/change-director-default-stat-thread-behavior branch from c1c9bf3 to ac40b3c Compare February 4, 2022 11:36
@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/change-director-default-stat-thread-behavior branch from 04d7bc6 to d347940 Compare February 11, 2022 14:01
@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/change-director-default-stat-thread-behavior branch from d347940 to b84df04 Compare February 21, 2022 10:10
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

Good work!

@pstorz pstorz merged commit 20bf27c into bareos:master Feb 24, 2022
@pstorz pstorz deleted the dev/alaaeddineelamri/master/change-director-default-stat-thread-behavior branch February 24, 2022 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants