[CBRD-25365] Change 'Creation_time' in 'SHOW VOLUME|LOG|ARCHIVE LOG HEADER' statements to Volume creation time. #5230

H2SU · 2024-05-24T09:40:12Z

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

Purpose

현재, 'SHOW VOLUME HEADER' 구문은 각 볼륨 별 생성시간을 출력하지 않습니다.
사용자의 요청에 따라 데이터베이스의 생성시간을 출력하고 있던 'Creation_time'을 각 볼륨 별 생성시간으로 변경합니다.

또한, 'SHOW VOLUME HEADER' 뿐만 아니라
'SHOW LOG HEADER', 'SHOW ARCHIVE LOG HEADER'의 'Creation_time'도 '볼륨 생성 시간'으로 변경하여 출력합니다.

Implementation

새 볼륨이 생성될 때 볼륨 생성시간을 저장하기 위해 disk_volume_header 구조체에 'vol_creation' field가 추가됩니다.

before

struct disk_volume_header
{
... 
  INT64 db_creation;
...
};

after

struct disk_volume_header
{
... 
  INT64 db_creation;
  INT64 vol_creation;
...
};

log header 구조체와 archive log header 구조체, background archive log header 구조체에 vol_creation_time 필드가 추가됩니다.

log header struct

typedef struct log_header LOG_HEADER;
struct log_header
{
  /* Log header information */
...
  INT64 db_creation;	/* Database creation time. For safety reasons, this value is set on all volumes and the
			 * log. The value is generated by the log manager */
  INT64 vol_creation;      /* Volume creation time */
...

archive log header struct

typedef struct log_arv_header LOG_ARV_HEADER;
struct log_arv_header
{
  /* Log archive header information */
...
  INT64 db_creation;	/* Database creation time. For safety reasons, this value is set on all volumes and the
			 * log. The value is generated by the log manager */
  INT64 vol_creation;	/* Volume creation time */
...
}

background archive log header struct

struct log_bgarv_header
{				
  /* Background log archive header information */
...
  INT64 db_creation;	/* Database creation time */
  INT64 vol_creation;	/* Volume creation time */
...
};

Remarks

N/A

…e addition of 'vol_creation_time'.

…ge 'Creation_time' in the 'show volume header' statement to 'volume creation time' instead of 'database creation time'.

…OG_HEADER', and enabled checking the creation time of each log volume using the 'SHOW' command.

… update vol_creation_time only when a new database file is created.

… volume only at initial creation. Update the archive log volume creation time to reflect the time when it is archived.

src/storage/disk_manager.c

hornetmj · 2024-07-02T11:16:28Z

src/storage/disk_manager.c

+ assert (*new_dbcreation == vhdr->db_creation_time);
+ }
+
+ if ((vhdr->chkpt_lsa.pageid != new_chkptlsa->pageid) || (vhdr->chkpt_lsa.offset != new_chkptlsa->offset))


Suggested change

if ((vhdr->chkpt_lsa.pageid != new_chkptlsa->pageid) || (vhdr->chkpt_lsa.offset != new_chkptlsa->offset))

if (!LSA_EQ (&vhdr->chkpt_lsa, new_chkptlsa))

추가적으로, is_new_db_creation == true를 만족하는 경우에만 !LSA_EQ (&vhdr->chkpt_lsa, new_chkptlsa) 조건이 만족합니다. 따라서, if 구문을 하나로 합칠 수 있습니다.

@joohok 위 조건이 만족하지 않는 경우가 있는지 더블 체크해 주세요.

호출관계만 간단히 봤을 땐, volume header에 저장된 checkpoint lsa와 disk_set_creation 함수가 불릴 당시의 checkpoint lsa가 같을 것이란 보장이 없을 것 같습니다.

@joohok 달라질 수 있는 예시 호출 관계 추가로 남겨주세요. 확인해 보겠습니다.

@H2SU 위에 제안드린 if 조건식 변경 - if (!LSA_EQ (&vhdr->chkpt_lsa, new_chkptlsa))이 아직 반영되지 않은 상태에서 리뷰가 다시 요청되었습니다.

@hornetmj
분석해보니, 현재까지의 구현에서는 is_new_db_creation == true 를 만족하는 상황에서만 !LSA_EQ (&vhdr->chkpt_lsa, new_chkptlsa) 조건을 만족시키네요. backup, restore 하는 과정에서 위 조건을 만족하지 못하는 상황이 있을 거라 짐작했는데, 아니었습니다. 하지만, assertion을 추가하여 확인해보는 것도 좋을 거라 생각됩니다.

수정하였습니다.

@H2SU 지금 수정하신 것처럼 db_creation과 chkpt_lsa는 별도로 체크해 주시면 되겠습니다.

@joohok log_final() 호출 시 다음과 같이 활성 로그 볼륨 헤더의 checkpoint lsa 정보만 변경하는 로직이 있네요. 따라서, 활성 로그 볼륨의 checkpoint lsa 정보와 데이터 볼륨의 checkpoint lsa 정보가 다른 경우가 존재합니다.

// 콜스택 일부 #0 LSA_COPY (plsa1=0x7ffff707e320 <log_Gl+288>, plsa2=0x7ffff707e318 <log_Gl+280>) at ../src/transaction/log_lsa.hpp:143 #1 0x00007ffff64a7d43 in log_final (thread_p=0x698850) at ../src/transaction/log_manager.c:1798 #2 0x00007ffff6464e42 in xboot_shutdown_server (thread_p=@0x7fffffffaf60: 0x698850, is_er_final=ER_ALL_FINAL) at ../src/transa ction/boot_sr.c:3131 // 변경된 checkpoint lsa, 데이터 볼륨 헤더는 Old value를 갖음 Old value = {pageid = 0, offset = 14248} New value = {pageid = 136, offset = 14648}

src/storage/disk_manager.c

src/transaction/log_manager.c

src/storage/disk_manager.c

src/transaction/log_page_buffer.c

src/transaction/log_storage.hpp

… to db_creation and vol_creation.

…tility

…ocal variable

…ocal variables

…ameters

src/transaction/log_manager.c

joohok · 2024-07-10T05:37:54Z

불필요한 변경이 많습니다. 지역 변수, 함수명, 함수 인자값, 그리고 시스템 카탈로그의 스펙까지 변경됩니다. 코드 전반적으로 다시 검토 부탁드립니다.

넵 재검토해보겠습니다.

불필요한 변경이 많습니다. 재검토 부탁드립니다.

불필요한 수정은 전부 원복했다고 생각했는데 불필요한 부분이 어떤 부분인지 알 수 있을까요..?

#5230 (comment)

src/transaction/log_applier.c

src/transaction/log_manager.c

src/storage/disk_manager.c

joohok · 2024-07-10T06:14:08Z

build가 실패합니다.
/home/src/transaction/log_applier.c:1592:19: error: ‘LA_HA_APPLY_INFO’ {aka ‘struct la_ha_apply_info’} has no member named ‘db_creation’

H2SU · 2024-07-10T06:42:59Z

build가 실패합니다. /home/src/transaction/log_applier.c:1592:19: error: ‘LA_HA_APPLY_INFO’ {aka ‘struct la_ha_apply_info’} has no member named ‘db_creation’

빌드 실패하는 부분 전부 수정하여 반영하였습니다.

hornetmj · 2024-07-10T14:18:54Z

src/storage/disk_manager.c

@@ -599,6 +600,7 @@ disk_format (THREAD_ENTRY * thread_p, const char *dbname, VOLID volid, DBDEF_VOL
 vhdr->nsect_max = ext_info->nsect_max;
 vhdr->db_charset = lang_charset ();
 vhdr->hint_allocsect = NULL_SECTID;
+ vhdr->vol_creation = time (NULL);


NIT:

vhdr->db_creation 값을 설정하는 log_get_db_start_parameters() 호출 다음에 설정하는게 어떨까요? 이후 유지보수 관점에서 두 멤버 변수는 함께 검토되게 될 것인데, 일관되게 db_creation와 vol_creation는 순서대로 근처에서 처리되는게 좋을 것 같아요.

그렇게 변경하는게 좀 더 가독성 측면에서도 좋은 것 같습니다. 변경했습니다

hornetmj · 2024-07-10T14:43:43Z

src/transaction/log_applier.c

@@ -455,7 +455,7 @@ static void la_decache_page_buffers (LOG_PAGEID from, LOG_PAGEID to);
 static int la_find_required_lsa (LOG_LSA * required_lsa);

 static int la_get_ha_apply_info (const char *log_path, const char *prefix_name, LA_HA_APPLY_INFO * ha_apply_info);
-static int la_insert_ha_apply_info (DB_DATETIME * creation_time);
+static int la_insert_ha_apply_info (DB_DATETIME * db_creation);


이슈와 무관한 수정입니다. 원복해 주세요.

원복했습니다

src/transaction/log_applier.c

hornetmj · 2024-07-10T14:58:05Z

src/transaction/log_page_buffer.c

@@ -2344,7 +2346,7 @@ logpb_write_page_to_disk (THREAD_ENTRY * thread_p, LOG_PAGE * log_pgptr, LOG_PAG
 PGLENGTH
 logpb_find_header_parameters (THREAD_ENTRY * thread_p, const bool force_read_log_header, const char *db_fullname,
 const char *logpath, const char *prefix_logname, PGLENGTH * io_page_size,
- PGLENGTH * log_page_size, INT64 * creation_time, float *db_compatibility, int *db_charset)
+ PGLENGTH * log_page_size, INT64 * db_creation, float *db_compatibility, int *db_charset)


이슈와 무관해 보입니다. 원복해 주세요.

원복했습니다

src/storage/disk_manager.c

hornetmj · 2024-07-11T04:38:19Z

src/storage/disk_manager.c

@@ -612,14 +613,14 @@ disk_format (THREAD_ENTRY * thread_p, const char *dbname, VOLID volid, DBDEF_VOL
 goto exit;
 }

- /* Find the time of the creation of the database and the current LSA checkpoint. */
-
+ /* Find the time of the creation of the database, volume and the current LSA checkpoint. */


기존 주석이 불필요한 주석 같습니다. 제거하시죠.

넵 제거하였습니다.

hornetmj · 2024-07-11T04:39:47Z

src/storage/disk_manager.c

+ vhdr->vol_creation = time (NULL);
+ memcpy (&vhdr->db_creation, new_dbcreation, sizeof (*new_dbcreation));


NIT:

Suggested change

vhdr->vol_creation = time (NULL);

memcpy (&vhdr->db_creation, new_dbcreation, sizeof (*new_dbcreation));

memcpy (&vhdr->db_creation, new_dbcreation, sizeof (*new_dbcreation));

vhdr->vol_creation = time (NULL);

수정하였습니다.

src/storage/disk_manager.c

hornetmj · 2024-07-11T04:49:30Z

src/transaction/log_impl.h

@@ -1029,7 +1029,7 @@ extern int logpb_fetch_header_from_active_log (THREAD_ENTRY * thread_p, const ch
 extern PGLENGTH logpb_find_header_parameters (THREAD_ENTRY * thread_p, const bool force_read_log_header,
 const char *db_fullname, const char *logpath,
 const char *prefix_logname, PGLENGTH * io_page_size,
- PGLENGTH * log_page_size, INT64 * db_creation, float *db_compatibility,
+ PGLENGTH * log_page_size, INT64 * creation_time, float *db_compatibility,


원복 시 diff가 발생하면 안됩니다.

해당 문제에 대해 팀즈로 질문드렸습니다!

joohok

고생하셨습니다

src/storage/disk_manager.c

…er' structure

H2SU requested a review from joohok May 24, 2024 09:40

H2SU self-assigned this May 24, 2024

H2SU force-pushed the feature/volumeheader branch 2 times, most recently from f723921 to c034f21 Compare May 28, 2024 08:41

H2SU added 2 commits June 19, 2024 16:27

[CBRD-25365] Changing 'creation_time' to 'db_creation_time' due to th…

ef0f433

…e addition of 'vol_creation_time'.

[CBRD-25365] Add 'vol_creation_time' to 'disk_volume_header' and chan…

48e2ec5

…ge 'Creation_time' in the 'show volume header' statement to 'volume creation time' instead of 'database creation time'.

H2SU force-pushed the feature/volumeheader branch from c034f21 to 0d689ad Compare June 19, 2024 07:27

H2SU changed the title ~~[CBRD-25365] Add the 'Volume_time' information in the result of the 'SHOW VOLUME HEADER' csql command.~~ [CBRD-25365] Change 'Creation_time' in 'SHOW VOLUME|LOG|ARCHIVE LOG HEADER' statements to Volume creation time. Jun 19, 2024

H2SU force-pushed the feature/volumeheader branch from 0d689ad to 8a45b9b Compare June 24, 2024 06:40

H2SU added 3 commits June 24, 2024 15:43

[CBRD-25365] Added 'vol_creation_time' to 'LOG_HEADER' and 'ARCHIVE_L…

b8a0191

…OG_HEADER', and enabled checking the creation time of each log volume using the 'SHOW' command.

[CBRD-25365] Add a new parameter to the disk_set_creation function to…

d3220ee

… update vol_creation_time only when a new database file is created.

[CBRD-25365] Reset vol_creation_time of active log upon archiving.

75c9960

H2SU force-pushed the feature/volumeheader branch from 8a45b9b to 75c9960 Compare June 24, 2024 06:43

[CBRD-25365] Modify to initialize the creation time of the active log…

e9c6864

… volume only at initial creation. Update the archive log volume creation time to reflect the time when it is archived.

H2SU marked this pull request as ready for review July 1, 2024 06:29

H2SU requested review from beyondykk9 and hornetmj as code owners July 1, 2024 06:29

H2SU requested a review from YeunjunLee July 1, 2024 06:30

hornetmj reviewed Jul 2, 2024

View reviewed changes

H2SU added 5 commits July 5, 2024 11:15

[CBRD-25365] delete comments

2b406e0

[CBRD-25365] db_creation_time and vol_creation_time have been changed…

017f622

… to db_creation and vol_creation.

[CBRD-25365] restore schema_system_catalog_install.cpp file

8a95a58

[CBRD-25365] Add vol_creation to be output when using the applyinfo u…

798e17e

…tility

[CBRD-25365] Remove is_new_db_creation parameter and replace with a l…

022dcdb

…ocal variable

H2SU requested a review from hornetmj July 5, 2024 09:11

H2SU added 3 commits July 5, 2024 18:14

[CBRD-25365] Modify if statement condition

92029da

[CBRD-25365] Add vol_creation to volume header dump

3c6b001

[CBRD-25365] Rename log_verify_dbcreation function, parameters, and l…

ce79520

…ocal variables

H2SU force-pushed the feature/volumeheader branch from 585824c to ce79520 Compare July 5, 2024 09:55

[CBRD-25365] Remove vol_creation parameter from logpb_find_header_par…

2553638

…ameters

joohok reviewed Jul 10, 2024

View reviewed changes

src/transaction/log_manager.c Outdated Show resolved Hide resolved

H2SU requested a review from joohok July 10, 2024 06:00

joohok reviewed Jul 10, 2024

View reviewed changes

src/transaction/log_applier.c Outdated Show resolved Hide resolved

src/transaction/log_manager.c Outdated Show resolved Hide resolved

src/storage/disk_manager.c Show resolved Hide resolved

H2SU force-pushed the feature/volumeheader branch from 8b9ecfa to 52b85b1 Compare July 10, 2024 06:39

H2SU requested a review from joohok July 10, 2024 06:41

hornetmj reviewed Jul 10, 2024

View reviewed changes

[CBRD-25365] Revert unnecessary changes

e72ba1c

H2SU force-pushed the feature/volumeheader branch from 52b85b1 to e72ba1c Compare July 11, 2024 02:27

[CBRD-25365] Revert log_applier.c

87342ae

H2SU requested a review from hornetmj July 11, 2024 04:20

hornetmj reviewed Jul 11, 2024

View reviewed changes

[CBRD-25365] Change the position of vol_creation value assignment

db2e0e3

H2SU force-pushed the feature/volumeheader branch from 44fa8ee to f58b335 Compare July 11, 2024 07:50

[CBRD-25365] Revert unnecessary changes

a7778c8

H2SU force-pushed the feature/volumeheader branch from f58b335 to a7778c8 Compare July 11, 2024 08:00

H2SU requested a review from hornetmj July 11, 2024 08:01

joohok approved these changes Jul 11, 2024

View reviewed changes

src/storage/disk_manager.c Show resolved Hide resolved

hornetmj approved these changes Jul 11, 2024

View reviewed changes

src/storage/disk_manager.c Show resolved Hide resolved

H2SU removed the request for review from beyondykk9 July 11, 2024 08:46

[CBRD-25365] Add comment

5d7f397

H2SU force-pushed the feature/volumeheader branch from c931aa0 to 5d7f397 Compare July 15, 2024 15:23

YeunjunLee approved these changes Jul 19, 2024

View reviewed changes

[CBRD-25365] Remove the 'vol_creation' field from the 'log_bgarv_head…

09487ee

…er' structure

H2SU force-pushed the feature/volumeheader branch from d8a0caa to 09487ee Compare July 25, 2024 07:09

H2SU changed the base branch from develop to feature/show_volume_header August 13, 2024 01:22

hornetmj merged commit 52ea64f into CUBRID:feature/show_volume_header Aug 13, 2024
7 of 9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CBRD-25365] Change 'Creation_time' in 'SHOW VOLUME|LOG|ARCHIVE LOG HEADER' statements to Volume creation time. #5230

[CBRD-25365] Change 'Creation_time' in 'SHOW VOLUME|LOG|ARCHIVE LOG HEADER' statements to Volume creation time. #5230

H2SU commented May 24, 2024 •

edited

Loading

hornetmj Jul 2, 2024

hornetmj Jul 2, 2024

joohok Jul 5, 2024

hornetmj Jul 5, 2024

hornetmj Jul 5, 2024

joohok Jul 8, 2024

H2SU Jul 8, 2024

hornetmj Jul 10, 2024

joohok commented Jul 10, 2024

joohok commented Jul 10, 2024

H2SU commented Jul 10, 2024

hornetmj Jul 10, 2024

H2SU Jul 11, 2024

hornetmj Jul 10, 2024

H2SU Jul 11, 2024

hornetmj Jul 10, 2024

H2SU Jul 11, 2024

hornetmj Jul 11, 2024

H2SU Jul 11, 2024

hornetmj Jul 11, 2024

H2SU Jul 11, 2024

hornetmj Jul 11, 2024

H2SU Jul 11, 2024

joohok left a comment

	if ((vhdr->chkpt_lsa.pageid != new_chkptlsa->pageid) \|\| (vhdr->chkpt_lsa.offset != new_chkptlsa->offset))
	if (!LSA_EQ (&vhdr->chkpt_lsa, new_chkptlsa))

		vhdr->vol_creation = time (NULL);
		memcpy (&vhdr->db_creation, new_dbcreation, sizeof (*new_dbcreation));

[CBRD-25365] Change 'Creation_time' in 'SHOW VOLUME|LOG|ARCHIVE LOG HEADER' statements to Volume creation time. #5230

[CBRD-25365] Change 'Creation_time' in 'SHOW VOLUME|LOG|ARCHIVE LOG HEADER' statements to Volume creation time. #5230

Conversation

H2SU commented May 24, 2024 • edited Loading

Purpose

Implementation

Remarks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joohok commented Jul 10, 2024

joohok commented Jul 10, 2024

H2SU commented Jul 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joohok left a comment

Choose a reason for hiding this comment

H2SU commented May 24, 2024 •

edited

Loading