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

[CUBRIDMAN-216] Add manuals for system parameter 'enable_memory_monitoring' and memmon utility #517

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

Conversation

@Rudeus Rudeus self-assigned this May 10, 2024
@Rudeus Rudeus marked this pull request as ready for review May 13, 2024 04:22
@Rudeus Rudeus requested review from hornetmj and joohok May 13, 2024 04:22
@Rudeus Rudeus marked this pull request as draft May 13, 2024 04:24
@Rudeus Rudeus marked this pull request as ready for review May 13, 2024 04:38
@Rudeus Rudeus marked this pull request as draft May 13, 2024 04:39
en/admin/config.rst Outdated Show resolved Hide resolved
@Rudeus Rudeus requested a review from joohok May 13, 2024 05:09
@joohok
Copy link
Contributor

joohok commented May 13, 2024

I think you need to build manual again to show the changes correctly in http://192.168.2.112:9002/admin/config.html#other-parameters

ko/admin/admin_utils.rst Outdated Show resolved Hide resolved
en/admin/admin_utils.rst Outdated Show resolved Hide resolved
@Rudeus Rudeus requested a review from joohok May 13, 2024 07:12
ko/admin/config.rst Outdated Show resolved Hide resolved
ko/admin/admin_utils.rst Outdated Show resolved Hide resolved
ko/admin/admin_utils.rst Outdated Show resolved Hide resolved
ko/admin/admin_utils.rst Outdated Show resolved Hide resolved
ko/admin/admin_utils.rst Outdated Show resolved Hide resolved
ko/admin/admin_utils.rst Outdated Show resolved Hide resolved
ko/admin/admin_utils.rst Outdated Show resolved Hide resolved
ko/admin/admin_utils.rst Outdated Show resolved Hide resolved
ko/admin/admin_utils.rst Outdated Show resolved Hide resolved
ko/admin/admin_utils.rst Outdated Show resolved Hide resolved
@kwonhoil kwonhoil requested a review from sjkimxxx May 21, 2024 10:24
@Rudeus Rudeus requested a review from junsklee May 29, 2024 03:17
@Rudeus
Copy link
Author

Rudeus commented May 29, 2024

The output unit of memmon has been changed from KB to Bytes, and the utility manual examples have been updated accordingly. Since this is a minor modification, I am notifying you via comment instead of re-requesting a review.

transaction/log_page_buffer.c:584 | 262147560 Bytes( 28%)
...

**cubrid memmon** 유틸리티를 실행하면 데이터베이스 이름, 힙 메모리 총 사용량, 할당된 힙 메모리를 추적 관리하기 위해 서버 내부적으로 유지되는 메타 정보의 메모리 사용량이 출력된다. 세부 항목으로는 힙 메모리 할당이 발생한 모든 파일과 라인 별[파일:라인] 메모리 사용량과 점유율이 사용량 기준으로 정렬되어 출력된다. 만약 --disable-force 옵션을 이용하여 메모리 모니터링 기능을 강제로 중지시킨 경우, 에러 메시지가 출력된다.
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
**cubrid memmon** 유틸리티를 실행하면 데이터베이스 이름, 힙 메모리 총 사용량, 할당된 힙 메모리를 추적 관리하기 위해 서버 내부적으로 유지되는 메타 정보의 메모리 사용량이 출력된다. 세부 항목으로는 힙 메모리 할당이 발생한 모든 파일과 라인 별[파일:라인] 메모리 사용량과 점유율이 사용량 기준으로 정렬되어 출력된다. 만약 --disable-force 옵션을 이용하여 메모리 모니터링 기능을 강제로 중지시킨 경우, 에러 메시지가 출력된다.
**cubrid memmon** 유틸리티를 실행하면 데이터베이스 이름, 힙 메모리 총 사용량, 할당된 힙 메모리를 추적 관리하기 위해 서버 내부적으로 유지되는 메타 정보의 메모리 사용량이 출력된다. 세부 항목으로는 힙 메모리 할당이 발생한 모든 파일과 라인 별[파일:라인] 메모리 사용량과 점유율이 사용량 기준으로 정렬되어 출력된다. 시스템 파라미터 **enable_memory_monitoring** 가 no로 설정된 경우 또는 --disable-force 옵션을 이용하여 메모리 모니터링 기능을 강제로 중지시킨 경우, 에러 메시지가 출력된다.

Copy link
Author

Choose a reason for hiding this comment

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

Done.


.. option:: --disable-force

메모리 모니터링 기능을 강제로 중지하는 옵션이다. 이 옵션을 이용해 메모리 모니터링 기능을 강제로 중지시키면 **cubrid memmon** 유틸리티를 실행해도 서버의 힙 메모리 사용량은 출력되지 않는다. 메모리 사용량을 다시 추적하기 위해서는 서버를 재구동해야 한다. ::
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
메모리 모니터링 기능을 강제로 중지하는 옵션이다. 이 옵션을 이용해 메모리 모니터링 기능을 강제로 중지시키면 **cubrid memmon** 유틸리티를 실행해도 서버의 힙 메모리 사용량은 출력되지 않는다. 메모리 사용량을 다시 추적하기 위해서는 서버를 재구동해야 한다. ::
메모리 모니터링 기능을 강제로 중지하는 옵션이다. 이 옵션을 이용해 메모리 모니터링 기능을 강제로 중지시키면 **cubrid memmon** 유틸리티를 실행해도 서버의 힙 메모리 사용량은 출력되지 않는다. 메모리 사용량을 다시 추적하기 위해서는 반드시 서버를 재구동해야 한다. ::

Copy link
Author

Choose a reason for hiding this comment

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

Done.

.. note::

* Linux 계열에서만 사용할 수 있다.
* 메모리 사용량을 추적하기 위해 매크로를 이용한 코드 수준의 자동화 과정에서 glibc와의 충돌이 발생하였다. 이를 막기 위해 glibc 내부에서 발생하는 메모리 사용량(STL 컨테이너)은 추적하지 않으며, 동일한 이유로 헤더 파일에서 발생하는 메모리 사용량은 추적하지 않는다.
Copy link
Contributor

Choose a reason for hiding this comment

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

How about removing this content from the manual, since this content is briefly explained in the following notice ?

Copy link
Author

Choose a reason for hiding this comment

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

@hornetmj I think it needs discussion.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mhoh3963 Is it okay if we don't explicitly specify the constraints? Or, how about combining the two NOTES into one and removing the redundant parts?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is no need to specify detailed restrictions, because the below notice is already described "큐브리드의 메모리 사용량 모니터링 기능이 헤더 파일 및 glibc 내부에서 발생하는 메모리 할당을 추적하지 않기 때문이며".

Copy link
Contributor

Choose a reason for hiding this comment

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

@mhoh3963 What I would like to ask is whether it is also necessary to remove the following two constraints.

  • Linux 계열에서만 사용할 수 있다.
  • HA 환경 구성 시 슬레이브 노드는 마스터 노드의 로그를 처리하는 과정에서 메모리 할당 비중이 높다. 이로 인해 메모리 사용량 모니터링의 비용이 증가하여 성능 문제가 발생할 수 있다. 따라서 슬레이브 노드에서는 사용을 권장하지 않는다.

If that is not the case, I was wondering if it would be possible to consolidate the two separate 'Notes' into a single 'Note'.

Copy link
Contributor

@mhoh3963 mhoh3963 May 31, 2024

Choose a reason for hiding this comment

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

@hornetmj
It is okay to leave two sentences in the manual and combine them into one.

Copy link
Author

Choose a reason for hiding this comment

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

@mhoh3963 @hornetmj @joohok I revise the note. Please check it.

@Rudeus Rudeus requested a review from mhoh3963 May 30, 2024 02:10
@Rudeus Rudeus requested review from hornetmj and joohok June 3, 2024 07:04
ko/admin/config.rst Outdated Show resolved Hide resolved
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.

10 participants