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

[CBRD-24462] Improve the logic that determines the number of threads to be used when running the backupdb utility, and additionally print the number of threads specified by administrator #5537

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

H2SU
Copy link
Contributor

@H2SU H2SU commented Oct 10, 2024

http://jira.cubrid.org/browse/CBRD-24462

Purpose

  • 문제 상황

    • backup 유틸리티 사용 시 -t 옵션을 사용해 백업에 사용할 쓰레드 개수를 설정할 수 있는데
      -o 옵션을 사용해 자세한 백업 정보를 출력할 때, 사용자가 입력한 쓰레드 개수가 출력되지 않음
    • CPU 코어 수를 구하는 함수(fileio_os_sysconf)는 플랫폼 별 구현이 따로 되어있어 유지보수 측면에서 좋지 않아 교체가 필요함
    • CPU 코어 수를 구하는 함수는 하이퍼쓰레딩을 자체적으로 고려하고 있는데 함수 외부에서 하이퍼쓰레딩을 이중으로 고려하는 문제
  • 1번 문제 원인

    • 사용자가 입력한 쓰레드 개수와 CPU 코어 수, cubrid.conf 파일에 설정된 max_clients 파라미터 값 중 가장 작은 값을
      백업에 사용될 쓰레드 개수로 설정하고, 해당 값을 출력해주고 있음
    • 사용자가 입력한 쓰레드 개수 값을 추가로 출력
  • 2번 문제 원인

    • fileio_os_sysconf 함수는 플랫폼 별 구현이 따로 되어있어 추후 유지보수 측면을 고려하여 변경하는 것이 좋다고 판단
    • fileio_os_sysconf 함수를 호출하는 모든 코드를 수정
  • 3번 문제 원인

    • CPU 코어 수를 구하는 함수에서 자체적으로 하이퍼쓰레딩을 고려하고 있는데
      함수 외부에서 추가로 하이퍼쓰레딩을 고려하여 x2 를 해주고 있어 쓰레드 계산식이 잘못되는 문제가 발생함

주요 함수 및 변수

  • CPU 코어 수를 구하는 함수
    • fileio_os_sysconf()
    • cubthread::system_core_count()
  • 사용자 CPU 코어 수
    • num_cpus
  • 사용자가 입력한 쓰레드 개수
    • num_threads
    • specified_thread_count
  • max_clients 파라미터 값
    • NUM_NORMAL_TRANS
  • 최종적으로 계산된 사용할 쓰레드 개수
    • thread_info_p->num_threads
    • session.read_thread_info.num_threads

Implementation

  • 사용자가 입력한 쓰레드 개수를 추가로 출력하도록 수정
  • CPU 코어 수를 구하는 함수를 모두 fileio_os_sysconf 함수에서 cubthread::system_core_count로 변경
  • fileio_os_sysconf 함수는 사용하는 곳이 없기 때문에 #if defined(ENABLE_UNUSED_FUNCTION) ~#endif 처리
  • 백업에 사용될 쓰레드 개수를 정하는 계산식에서 하이퍼쓰레딩을 고려하여 위 함수를 통해 구한 CPU 코어 수에 x2 를 해주는 부분 삭제

…to be used when running the backupdb utility, and additionally print the number of threads specified by administrator
@H2SU H2SU requested a review from hornetmj as a code owner October 10, 2024 09:41
@H2SU H2SU marked this pull request as draft October 10, 2024 09:41
@H2SU H2SU self-assigned this Oct 10, 2024
@H2SU H2SU marked this pull request as ready for review October 10, 2024 10:05
src/storage/external_sort.c Show resolved Hide resolved
src/transaction/log_page_buffer.c Outdated Show resolved Hide resolved
@hornetmj hornetmj marked this pull request as draft October 12, 2024 03:19
@H2SU H2SU requested a review from YeunjunLee October 14, 2024 10:51
@H2SU H2SU marked this pull request as ready for review October 14, 2024 10:52
Comment on lines 1495 to 1496
sort_param->px_height_max = (int) sqrt ((double) num_cpus); /* n */
sort_param->px_array_size = num_cpus; /* 2^^n */
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
sort_param->px_height_max = (int) sqrt ((double) num_cpus); /* n */
sort_param->px_array_size = num_cpus; /* 2^^n */
sort_param->px_height_max = (int) sqrt ((double) num_cpus);
sort_param->px_array_size = num_cpus;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

수정하였습니다.

Comment on lines 46 to 47
#include "thread_worker_pool.hpp"

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#include "thread_worker_pool.hpp"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

수정하였습니다.

@H2SU H2SU requested a review from hornetmj October 15, 2024 05:34
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.

3 participants