-
Notifications
You must be signed in to change notification settings - Fork 123
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
base: develop
Are you sure you want to change the base?
Conversation
…ally restarting due to the CAS memory size limit
5a69340
to
71ab032
Compare
@@ -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); |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cas의 FN_KEEP_SESS 처리를 위해 필요한 전역 변수가 이곳에 선언될 필요가 있는지 의문입니다. 혹시 어떤 기준으로 이곳에 선언했는지 추가 설명 가능할까요? 추가적으로 다음 부분 역시 의견드립니다.
- cas.c 수준에서 전역변수를 선언하는게 어떨까요?
- 이 정보는 cas에서 설정되는 FN_KEEP_SESS를 위해 유효하기 때문에 외부로 노출할 필요는 없어 보입니다. 나머지 부분에서 db_get_keep_session() 등을 호출하기 보다는 명시적으로 false를 넘겨주는게 어떨까요?
- 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; |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true로 설정된 이후 다시 client와 연결되었을 때 언제 false로 리셋되나요?
http://jira.cubrid.org/browse/CBRD-25700
Purpose
문제 상황
문제 원인
주요 함수 및 변수
현재 시간과 session에 저장되어있는 active_time을 비교하여
그 둘의 차이가 timeout 값 이상이면 만료된 세션이라고 판단하여 free하고 있음
Implementation