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-25700] Modify the behavior to retain the session when automatically restarting due to the CAS memory size limit #5717

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

Conversation

H2SU
Copy link
Contributor

@H2SU H2SU commented Dec 13, 2024

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

Purpose

  • 문제 상황

    • cas가 메모리 사용량 한계치 초과로 인해 restart 될 때 기존 세션 정보를 유지하지 않아 생기는 문제
  • 문제 원인

    • cas의 메모리 사용량 한계치를 초과하여 cas가 restart하게 될 때, 기존 cas에서 할당받은 세션 정보를 새로운 cas에서 재사용
    • 하지만 CBRD-25414 이슈로 인해 세션 정보를 free하도록 수정되어 유지되지 않아 문제가 발생
    • 세션 정보를 유지해야할 때는 세션정보를 free하지 않도록 수정

주요 함수 및 변수

  • db_Keep_session
    • 세션을 유지할 지 판단하는 전역변수
    • 클라이언트에서 서버쪽으로 보내진다
    • 서버쪽에서는 해당 값을 보고 session_state의 is_keep_session 값을 true로 변경
  • FN_KEEP_SESS
    • 세션을 유지해야하는 상황에서의 리턴값
  • XXX_end_session()
    • 세션 종료를 요청하는 함수
    • 최종적으로 session_state_destroy 함수를 호출하여 세션 정리
  • session_state_destroy()
    • 세션정보 정리하는 함수
    • 함수 내부에서 is_keep_session 값이 true라면 session_state의 is_keep_session 값을 true로 변경
  • session_state->is_keep_session
    • 아래 데몬에서 만료된 세션이더라도 삭제하지 않도록 하는 변수
  • session_remove_expired_sessions()
    • 60초마다 만료된 세션을 찾아 정리하는 데몬에서 호출하는 함수
    • hashmap에서 session 정보를 불러와 session_check_timeout() 함수를 통해
      현재 시간과 session에 저장되어있는 active_time을 비교하여
      그 둘의 차이가 timeout 값 이상이면 만료된 세션이라고 판단하여 free하고 있음

Implementation

  • 세션을 유지해야하는 경우 (FN_KEEP_SESS)에 db_Keep_session 값을 true로 설정하여 서버에 전달
  • 서버에서는 해당 값을 읽어 session_state의 is_keep_session 값을 설정
  • 만료된 세션을 정리하는 데몬에서 만료된 세션이더라도 유지해야한다면 세션을 정리하지않도록 수정

@H2SU H2SU self-assigned this Dec 13, 2024
…ally restarting due to the CAS memory size limit
@H2SU H2SU force-pushed the CBRD-25700 branch 3 times, most recently from 5a69340 to 71ab032 Compare December 17, 2024 07:57
@H2SU H2SU marked this pull request as draft December 17, 2024 07:59
@H2SU H2SU marked this pull request as ready for review December 18, 2024 06:15
@@ -2170,6 +2172,10 @@ process_request (SOCKET sock_fd, T_NET_BUF * net_buf, T_REQ_INFO * req_info)
net_buf->client_version = req_info->client_version;
set_hang_check_time ();
fn_ret = (*server_fn) (sock_fd, argc, argv, net_buf, req_info);
if (fn_ret == FN_KEEP_SESS)
{
db_set_keep_session (true);
Copy link
Contributor

Choose a reason for hiding this comment

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

여기서 db_set_keep_session(true)를 호출해 주어야 하는 이유가 뭔가요?

SESSION_ID id;
OR_ALIGNED_BUF (OR_INT_SIZE) a_reply;
char *reply = OR_ALIGNED_BUF_START (a_reply);
char *ptr = NULL;

(void) or_unpack_int (request, (int *) &id);
ptr = or_unpack_int (request, (int *) &id);
ptr = or_unpack_int (ptr, (int *) &is_keep_session);
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
ptr = or_unpack_int (ptr, (int *) &is_keep_session);
ptr = or_unpack_int (ptr, &is_keep_session);

@@ -79,6 +79,7 @@ struct valcnv_buffer
};

SESSION_ID db_Session_id = DB_EMPTY_SESSION;
bool db_Keep_session = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

cas의 FN_KEEP_SESS 처리를 위해 필요한 전역 변수가 이곳에 선언될 필요가 있는지 의문입니다. 혹시 어떤 기준으로 이곳에 선언했는지 추가 설명 가능할까요? 추가적으로 다음 부분 역시 의견드립니다.

  1. cas.c 수준에서 전역변수를 선언하는게 어떨까요?
  2. 이 정보는 cas에서 설정되는 FN_KEEP_SESS를 위해 유효하기 때문에 외부로 노출할 필요는 없어 보입니다. 나머지 부분에서 db_get_keep_session() 등을 호출하기 보다는 명시적으로 false를 넘겨주는게 어떨까요?
  3. bool is_keep_session 파라미터는 ux_end_session()과 db_end_session() 인터페이스를 통해 전달되는게 어떨까요? cas에서 FN_KEEP_SESS 검사 후 ux_end_session() -> db_end_session()이 호출되는데, 명시적으로 session 유지 여부를 인자로 넘겨주는게 직관적 입니다.

{
session_p->is_keep_session = true;
pthread_mutex_unlock (&session_p->mutex);
return NO_ERROR;
Copy link
Contributor

Choose a reason for hiding this comment

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

기존과 동일한 처리를 위해 이곳에서 return 하는 것으로 알고 있습니다. 다만, session 정보는 유지되지만 연결은 끊어진 상태인데 ref_count 등을 처리하지 않아도 문제가 되지 않는지 검증 되었나요?

session_remove_expired_sessions() 함수에서 if (state->is_keep_session == true) 검사 이전에 assert (state->ref_count == 0); 조건이 검사되어 지고 있습니다. 참고하세요.

@@ -762,6 +765,13 @@ session_state_destroy (THREAD_ENTRY * thread_p, const SESSION_ID id)
return ER_SES_SESSION_EXPIRED;
}

if (is_keep_session == true)
{
session_p->is_keep_session = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

true로 설정된 이후 다시 client와 연결되었을 때 언제 false로 리셋되나요?

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.

6 participants