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-25779] Refactor Temp File Memory Buffer to Use FILEIO_PAGE Structure #5764

Open
wants to merge 2 commits into
base: feature/temp_file
Choose a base branch
from

Conversation

youngjinj
Copy link
Contributor

@youngjinj youngjinj commented Jan 3, 2025

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

Change the page structure for the temp file memory buffer to use FILEIO_PAGE.

  • Differentiate memory pages (PAGE_MEMORY) using the page type (ptype) in FILEIO_PAGE_RESERVED.

Removed

  1. enum qmgr_page_type, QMGR_PAGE_TYPE
  2. qmgr_get_page_type
  3. qfile_set_dirty_page, qfile_set_dirty_page_and_skip_logging
    • Replaced with qmgr_set_dirty_page.
  4. qfile_get_first_page
    • Already marked as ENABLE_UNUSED_FUNCTION.

Added

  1. Added PAGE_MEMORY to enum PAGE_TYPE.
    • Represents the page type for Temp File Memory Buffer, which always resides in memory.
    • PAGE_QRESULT and PAGE_MEMORY do not coexist.

Changed

  1. Moved CAST_PGPTR_TO_IOPGPTR and CAST_IOPGPTR_TO_PGPTR macro definitions from src/storage/page_buffer.c to page_buffer.h.
  2. Removed QMGR_TEMP_FILE pointer from the parameter lists of:
    • qmgr_free_old_page
    • qmgr_set_dirty_page
    • qmgr_free_old_page_and_init
  3. Renamed qmgr_free_old_page_and_init macro to uppercase.
  4. Changed routines that used qmgr_get_page_type for determining page type:
    • Affected functions:
      • qmgr_free_old_page
      • qmgr_set_dirty_page
    • Before:
      QMGR_PAGE_TYPE page_type;
      
      page_type = qmgr_get_page_type(page_p, tfile_vfid_p);
      
      if (page_type == QMGR_TEMP_FILE_PAGE)
        ...
    • After:
      FILEIO_PAGE *io_page_p;
      
      CAST_PGPTR_TO_IOPGPTR(io_page_p, page_p);
      
      if (io_page_p->prv.ptype != PAGE_MEMORY)
        ...
  5. qmgr_allocate_tempfile_with_buffer:
    • Allocates memory with IO_PAGESIZE instead of DB_PAGESIZE.
    • Sets membuf in QMGR_TEMP_FILE to page in FILEIO_PAGE.
  6. qmgr_get_new_page:
    • Initializes FILEIO_PAGE fields (FILEIO_PAGE_RESERVED, FILEIO_PAGE_WATERMARK) by calling fileio_initialize_res.
    • Sets (FILEIO_PAGE *)->prv.ptype to PAGE_MEMORY.
      FILEIO_PAGE *io_page_p;
      
      qfile_init_page_header(page_p);
      
      CAST_PGPTR_TO_IOPGPTR(io_page_p, page_p);
      fileio_initialize_res(thread_p, io_page_p, IO_PAGESIZE);
      io_page_p->prv.ptype = PAGE_MEMORY;
  7. Ensures consistency by initializing prv.volid and prv.pageid, even though they are not required.
    io_page_p->prv.volid = vpid_p->volid = NULL_VOLID;
    io_page_p->prv.pageid = vpid_p->pageid = tfile_vfid_p->membuf_last;

@youngjinj youngjinj self-assigned this Jan 3, 2025
@youngjinj youngjinj changed the title [CBRD-25779] Refactor Temporary Memory Buffer to Use FILEIO_PAGE Structure [CBRD-25779] Refactor Temp File Memory Buffer to Use FILEIO_PAGE Structure Jan 10, 2025
@youngjinj youngjinj marked this pull request as ready for review January 15, 2025 02:14
@shparkcubrid shparkcubrid requested a review from Hamkua January 15, 2025 03:34
@@ -162,7 +162,8 @@ typedef enum
PAGE_LOG, /* NONE - log page (unused) */
PAGE_DROPPED_FILES, /* Dropped files page. */
PAGE_VACUUM_DATA, /* Vacuum data. */
PAGE_LAST = PAGE_VACUUM_DATA
PAGE_MEMORY,
Copy link
Contributor

Choose a reason for hiding this comment

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

perf_monitor.h의 typedef enum PERF_PAGE_TYPE;에 추가 및 관련 로직 검토 필요할 것 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PAGE_TYPE의 관리 일관성을 유지하기 위해 PERF_PAGE_TYPEPERF_PAGE_MEMORY를 추가하는 것에 동의합니다.

PERF_PAGE_TYPE에 새로운 타입을 추가하는 것 외에 관련 로직을 검토해야 한다는 의견도 주셨습니다.

PERF_PAGE_TYPE은 페이지 버퍼에서 관리하는 페이지에 접근할 때,
PGBUF_FIX_PERF에 포함된 정보를 페이지 타입별로 나누어 관리하기 위해 사용되는 것으로 보입니다.

PAGE_MEMORY 타입은 페이지 버퍼에서 사용되지 않는 페이지를 구분하기 위한 목적으로 추가하고 있습니다.
따라서 PAGE_MEMORY 타입의 페이지는 페이지 버퍼에서 관리하는 페이지가 아니며,
PGBUF_FIX_PERF에 포함된 정보는 페이지 버퍼를 통해 필요한 페이지를 찾을 때만 의미가 있으므로,
관련 로직을 추가하거나 변경할 필요는 없다고 생각합니다.

PERF_PAGE_TYPE에만 PERF_PAGE_MEMORY를 추가하는 것으로 이 의견에 대한 변경을 마무리해도 괜찮은지
다시 한 번 의견을 주시면 감사하겠습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

넵. 확인 후 회신주셔서 감사합니다. PERF_PAGE_MEMORY 추가 시 관련 정보도 주석으로 남겨주시면, 추후 유지보수함에 있어서 의미있는 정보가 될 것 같습니다.

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