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-23620] Destroy temp file in tempcache when temp volume is full #5524

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

Conversation

YeunjunLee
Copy link
Contributor

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

  • Purpose
    • Temp Volume이 temp_file_max_size_in_pages parameter를 통해 용량이 제한되는 상황에서, tempcache에 temp file들이 caching 되어 있는 경우, 그 temp file들의 용량 때문에 새로운 sector를 reserve 하지 못하는 상황 발생
      • tempcache는 사용이 끝난 max_pages_in_temp_file_cache parameter보다 크기가 작은 temp file들을 caching 함 (Reserve한 sector들을 반환하지 않음)
      • 추후에 새로운 temp file이 요구될 때, tempcache에 만들어진 temp file이 존재한다면, 이를 먼저 사용함. (크기나 type과 무관, 사용 중 이미 reserve 된 공간보다 더 필요하다면, 한 sector 씩 추가로 reserve)
      • sector reserve시, temp volume에 reserve 된 space와 추가로 할당해야 될 space의 합이 temp_file_max_size_in_pages parameter를 넘게 된다면, error 발생 (ER_BO_MAXTEMP_SPACE_HAS_BEEN_EXCEEDED)
  • Implementation
    • ER_BO_MAXTEMP_SPACE_HAS_BEEN_EXCEEDED 를 expected error에 추가하여 file manager에 전달
    • File manager가 temp file을 allocation 하는 상황에서 해당 error를 받았다면, temp file을 temp cache에서 제거한 이후, allocation 재시도.

@YeunjunLee YeunjunLee self-assigned this Sep 30, 2024
@YeunjunLee YeunjunLee marked this pull request as ready for review October 2, 2024 00:43
if (error_code == ER_BO_MAXTEMP_SPACE_HAS_BEEN_EXCEEDED)
{
file_tempcache_lock ();
if (file_Tempcache.ncached_numerable > 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

numerable 부터 제거하는 이유가 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

numerable을 먼저 지우던, not_numerable을 먼저 지우던, temp volume의 used space를 감소시키면 새로운 sector를 reserve하는 데는 문제가 없기 때문에 제거하는 순서에 의미를 두지 않았습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

preserved 된 것 또한 제거 대상에 넣으면 안될까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

제거하는 거에 우선순위가 있으면 좋겠습니다.
numerable 특성을 가진 temp page 할당 비용이 더 크다면, numerable 파일의 제거를 후순위로 하면 어떨까요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

numerable과 not numerable file의 reserve 비용이 큰 차이는 없어 보이지만, 굳이 비교하자면, file_create에서 vpid_find_nth_lastvpid_last_user_page_ftab등의 변수를 갱신하는 비용때문에 numerable file이 다시 생성할 때 비용이 조금 클 것 같습니다. non numerable file이 캐싱되어 있다면, 이를 먼저 제거하도록 변경하였습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

preserved의 경우에는 Tempcache가 별도로 관리하지 않기 때문에, numerable과 non-numerable list에 같이 존재하고 있습니다. 따라서, 현재에도 제거 대상에 포함되어 있습니다.

assert_release (false);
goto exit;
}
file_Tempcache.cached_numerable = file_Tempcache.cached_numerable->next;
Copy link
Contributor

Choose a reason for hiding this comment

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

cache list에서만 제거해도 공간 확보가 될까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

실제 reserve된 sector를 unreserve 시키는 작업은 file_destroy()에서 진행됩니다. 공간에 대한 정보도 이곳에서 갱신시킵니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

cache list에서 제거된 엔트리의 공간이 확보되는 로직을 알려주실 수 있나요? 연결이 안돼서요.
어디선가 file_temp_retire을 호출할 것 같은데 말이죠.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

사용이 끝난 temp file은 file_temp_retire를 통해 캐싱 / 제거가 시작됩니다. 함수 내에서 file_tempcache_put 함수를 통해 caching을 시도하고, 캐싱에 성공하였다면, 그대로 종료되고, 캐싱에 실패하였다면, file_destroy를 통해 file 제거가 시작됩니다.

file_destory에서는 예약된 sector들을 unreserve 시키고, 해당 용량만큼 temp volume의 free space 증가시켜 줍니다.

@YeunjunLee YeunjunLee requested a review from joohok October 4, 2024 04:24
/* reserve a sector */
error_code =
disk_reserve_sectors (thread_p, DB_TEMPORARY_DATA_PURPOSE, fhead->volid_last_expand, 1, &partsect_new.vsid);
if (error_code != NO_ERROR)
{
if (error_code == ER_BO_MAXTEMP_SPACE_HAS_BEEN_EXCEEDED)
{
file_tempcache_lock ();
Copy link
Contributor

@hornetmj hornetmj Oct 10, 2024

Choose a reason for hiding this comment

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

file_tempcache_ 접두어를 갖는 함수를 하나 소개하는게 좋겠습니다. (ex) file_tempcache_find_victim_and_destroy())

또한, numerable 기준으로 먼저 접근하는 캐쉬를 결정하는게 일관성 있어 보입니다. 위 논의된 컨텍스트는 생성 비용 관점이네요. 문맥상 numerable에서 못찾았는데 !numerable을 먼저 날리거나 그 반대가 되면 문맥이 자연스럽지 않습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

file_tempcache_find_victim_and_destroy 함수를 통해 temp file을 지우도록 하였습니다.
input 중 is_numerable 값은 numerable file을 지울지, non-numerable file을 지울 지의 option이고, 제거하는데 성공하였다면 true, 해당 temp file이 tempcache에 존재하지 않아서 제거에 실패하였다면 false를 return 합니다.

또한, file head를 통해 file의 numerable 여부를 판단하여, 먼저 제거를 시도할 file의 type을 정하도록 변경하였습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

file_tempcache_lock(), file_tempcache_unlock()는 file_tempcache_find_victim_and_destroy() 안쪽으로 들어가도 되겠네요.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

file_tempcache_lock, file_tempcache_unlock 함수를 file_tempcache_find_victim_and_destroy 안으로 위치시켰습니다.

/* reserve a sector */
error_code =
disk_reserve_sectors (thread_p, DB_TEMPORARY_DATA_PURPOSE, fhead->volid_last_expand, 1, &partsect_new.vsid);
if (error_code != NO_ERROR)
{
if (error_code == ER_BO_MAXTEMP_SPACE_HAS_BEEN_EXCEEDED)
{
file_tempcache_lock ();
Copy link
Contributor

Choose a reason for hiding this comment

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

file_tempcache_lock(), file_tempcache_unlock()는 file_tempcache_find_victim_and_destroy() 안쪽으로 들어가도 되겠네요.

{
if (file_Tempcache.ncached_numerable > 0)
{
if (file_destroy (thread_p, &file_Tempcache.cached_numerable->vfid, true) != 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.

file_destroy()가 호출된 이후에는 file_tempcache_retire_entry()를 호출해서 FILE_TEMPCACHE_ENTRY 메모리를 정리해야 합니다. file_tempcache_cache_or_drop_entries() 등을 참고해 보세요.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

file_tempcache_retire_entry 함수를 통해 destroy한 tempfile entry를 free list로 추가하였습니다.

@hornetmj hornetmj removed the request for review from joohok October 19, 2024 05:39
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.

4 participants