[CBRD-25312] Modify diagdb utility to accept a class-name parameter #5141

YeunjunLee · 2024-04-23T07:26:23Z

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

Modify heap dump of diagdb utility to accept class-name parameter.

Add -c and --class-name flag at diagdb option table and option map.
Modify usage message of diagdb to provide information of class-name parameter.
Modify diagdb main function to fetch class-name parameter
Create file_class_dump_specific_heap_file() function, which dumps a specific heap file with class name.

hornetmj · 2024-04-23T14:07:45Z

src/executables/util_sa.c

+      class_name = utility_get_option_string_value (arg_map, DIAG_CLASS_NAME_S, 0);
+      if (class_name == NULL)
+	{
+	  /* this dumps the contents of all heaps */


Suggested change

/* this dumps the contents of all heaps */

The DUMP OF ALL HEAPS in the next line has enough information.

hornetmj · 2024-04-23T14:15:34Z

src/executables/util_sa.c

+	}
+      else
+	{
+	  /* this dumps the contents of a specific heap for given class name */


Suggested change

/* this dumps the contents of a specific heap for given class name */

hornetmj · 2024-04-23T14:18:39Z

src/executables/util_sa.c

+	  /* this dumps the contents of a specific heap for given class name */
+	  if (!sm_check_system_class_by_name (class_name))
+	    {
+	      /* check if the format of class name is valid */


Suggested change

/* check if the format of class name is valid */

You don't always need to leave a comment about what it does. This should be fully explained, mainly through the function name.

hornetmj · 2024-04-23T14:33:03Z

src/storage/file_manager.h

@@ -215,6 +215,8 @@ extern DISK_ISVALID file_tracker_check (THREAD_ENTRY * thread_p);
 extern int file_tracker_dump (THREAD_ENTRY * thread_p, FILE * fp);
 extern int file_tracker_dump_all_capacities (THREAD_ENTRY * thread_p, FILE * fp);
 extern int file_tracker_dump_all_heap (THREAD_ENTRY * thread_p, FILE * fp, bool dump_records);
+extern void file_class_dump_specific_heap_file (THREAD_ENTRY * thread_p, FILE * fp, bool dump_records,


Based on the determined processing method, this function should be defined in heap_file.c/h. And this function might call the following functions to convert class name into hfid.

xlocator_find_class_oid()

heap_hfid_cache_get()

I moved the function to heap_file.c and change its name to heap_dump_specific_file.

hornetmj · 2024-04-23T14:41:51Z

msg/en_US/utils.msg

@@ -834,7 +834,11 @@ valid options:\n\
                                6 dump disk bitmaps\n\
                                7 dump catalog\n\
                                8 dump log\n\
-                                9 dump heap\n
+                                9 dump heap / class\n\


This revision is not a final edition and should remain a hidden option until the final version is released. There is even a possibility of changing the specifications as follows to compatible with other utilies.

cubrid diagdb [OPTION] database-name [class_name1 class_name2 ...]

One option is to separate the issue of usage output.

Rollback usage message to original diagdb usage message.

hornetmj · 2024-04-23T14:48:24Z

src/executables/util_sa.c

+		  goto error_exit;
+		}
+	    }
+	  fprintf (outfp, "\n*** DUMP OF SPECIFIC HEAP(S) ***\n");


To be kinder, it would be better to include the class name entered with the --class-name option in the heading.

*** DUMP HEAP OF user_name.class_name ***

src/executables/util_sa.c

… exists on DIAGDUMP_ALL option

src/executables/util_sa.c

Co-authored-by: Jooho Kim <[email protected]>

src/storage/heap_file.c

msg/en_US/utils.msg

Co-authored-by: Jooho Kim <[email protected]>

hornetmj · 2024-04-25T05:11:03Z

src/executables/util_sa.c

+  if (diag == DIAGDUMP_ALL && class_name != NULL)
+    {
+      goto print_diag_usage;
+    }


Suggested change

if (diag == DIAGDUMP_ALL && class_name != NULL)

{

goto print_diag_usage;

}

if (diag == DIAGDUMP_ALL && class_name != NULL)

{

goto print_diag_usage;

}

hornetmj · 2024-04-25T05:12:22Z

src/executables/util_sa.c

+	}
+      else
+	{
+	  assert (diag != DIAGDUMP_ALL);


It is already checked.

Suggested change

assert (diag != DIAGDUMP_ALL);

hornetmj · 2024-04-25T05:38:21Z

src/storage/heap_file.h

@@ -554,6 +554,7 @@ extern int heap_set_autoincrement_value (THREAD_ENTRY * thread_p, HEAP_CACHE_ATT
 					 HEAP_SCANCACHE * scan_cache, int *is_set);

 extern void heap_dump (THREAD_ENTRY * thread_p, FILE * fp, HFID * hfid, bool dump_records);
+extern void heap_dump_specific_file (THREAD_ENTRY * thread_p, FILE * fp, bool dump_records, const char *class_name);


Suggested change

extern void heap_dump_specific_file (THREAD_ENTRY * thread_p, FILE * fp, bool dump_records, const char *class_name);

#if defined (SA_MODE)

extern void heap_dump_specific_file (THREAD_ENTRY * thread_p, FILE * fp, bool dump_records, const char *class_name);

#endif

hornetmj · 2024-04-25T05:38:59Z

src/storage/heap_file.c

+/*
+ * heap_dump_specific_file () - dump a specific heap file with class name
+ *
+ * return            :
+ * thread_p (in)     : thread entry
+ * fp (in)           : output file
+ * dump_records (in) : true to dump records
+ * class_name (in)   : name of class to dump
+ */
+void
+heap_dump_specific_file (THREAD_ENTRY * thread_p, FILE * fp, bool dump_records, const char *class_name)
+{
+  /* TODO: Fetch HFID of class which corresponds to class_name, and dump heap file by HFID. Will be handled at CBRD-25313 and CBRD-25314. */
+  fprintf (fp, "\n\n*** DUMP A CLASS NAMED %s ***\n\n", class_name);
+}
+


Suggested change

/*

* heap_dump_specific_file () - dump a specific heap file with class name

*

* return :

* thread_p (in) : thread entry

* fp (in) : output file

* dump_records (in) : true to dump records

* class_name (in) : name of class to dump

*/

void

heap_dump_specific_file (THREAD_ENTRY * thread_p, FILE * fp, bool dump_records, const char *class_name)

{

/* TODO: Fetch HFID of class which corresponds to class_name, and dump heap file by HFID. Will be handled at CBRD-25313 and CBRD-25314. */

fprintf (fp, "\n\n*** DUMP A CLASS NAMED %s ***\n\n", class_name);

}

#if defined (SA_MODE)

/*

* heap_dump_specific_file () - dump a specific heap file with class name

*

* return :

* thread_p (in) : thread entry

* fp (in) : output file

* dump_records (in) : true to dump records

* class_name (in) : name of class to dump

*/

void

heap_dump_specific_file (THREAD_ENTRY * thread_p, FILE * fp, bool dump_records, const char *class_name)

{

/* TODO: Fetch HFID of class which corresponds to class_name, and dump heap file by HFID. Will be handled at CBRD-25313 and CBRD-25314. */

fprintf (fp, "\n\n*** DUMP A CLASS NAMED %s ***\n\n", class_name);

}

#endif

hornetmj · 2024-04-25T05:39:59Z

src/storage/heap_file.c

+ * class_name (in)   : name of class to dump
+ */
+void
+heap_dump_specific_file (THREAD_ENTRY * thread_p, FILE * fp, bool dump_records, const char *class_name)


Suggested change

heap_dump_specific_file (THREAD_ENTRY * thread_p, FILE * fp, bool dump_records, const char *class_name)

heap_dump_heap_file (THREAD_ENTRY * thread_p, FILE * fp, bool dump_records, const char *class_name)

hornetmj · 2024-04-25T05:42:37Z

src/storage/heap_file.c

+heap_dump_specific_file (THREAD_ENTRY * thread_p, FILE * fp, bool dump_records, const char *class_name)
+{
+  /* TODO: Fetch HFID of class which corresponds to class_name, and dump heap file by HFID. Will be handled at CBRD-25313 and CBRD-25314. */
+  fprintf (fp, "\n\n*** DUMP A CLASS NAMED %s ***\n\n", class_name);


Does this need to print out this message? This information is already included in the heading.

I think it's duplicated information. It's been deleted.

YeunjunLee added 2 commits April 23, 2024 16:24

modify diagdb utility to accept a class-name parameter

1ce74a3

indent

f9968bb

YeunjunLee self-assigned this Apr 23, 2024

YeunjunLee added 2 commits April 23, 2024 17:26

change argument flag, function name.

456117c

indent

473e627

YeunjunLee marked this pull request as ready for review April 23, 2024 08:56

YeunjunLee requested a review from hornetmj as a code owner April 23, 2024 08:56

YeunjunLee requested review from Rudeus and joohok April 23, 2024 08:56

hornetmj reviewed Apr 23, 2024

View reviewed changes

Rudeus reviewed Apr 23, 2024

View reviewed changes

src/executables/util_sa.c Outdated Show resolved Hide resolved

check dump mode and class name to print usage message when class name…

c2916c9

… exists on DIAGDUMP_ALL option

joohok reviewed Apr 24, 2024

View reviewed changes

src/executables/util_sa.c Outdated Show resolved Hide resolved

YeunjunLee and others added 4 commits April 24, 2024 17:43

delete redundant check

78e5377

Co-authored-by: Jooho Kim <[email protected]>

indent

03d40b6

minor fix

535fff4

indent

5032a77

YeunjunLee requested review from Rudeus, joohok and hornetmj April 25, 2024 02:19

joohok approved these changes Apr 25, 2024

View reviewed changes

src/storage/heap_file.c Outdated Show resolved Hide resolved

msg/en_US/utils.msg Outdated Show resolved Hide resolved

YeunjunLee and others added 2 commits April 25, 2024 12:38

Update src/storage/heap_file.c

0ae9fe3

Co-authored-by: Jooho Kim <[email protected]>

Update msg/en_US/utils.msg

aae7f38

Co-authored-by: Jooho Kim <[email protected]>

Rudeus approved these changes Apr 25, 2024

View reviewed changes

hornetmj reviewed Apr 25, 2024

View reviewed changes

limit function only for SA_MODE

d06428d

YeunjunLee requested a review from hornetmj April 25, 2024 06:10

hornetmj approved these changes Apr 25, 2024

View reviewed changes

YeunjunLee merged commit 78f96e0 into CUBRID:feature/heap_dump Apr 25, 2024
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CBRD-25312] Modify diagdb utility to accept a class-name parameter #5141

[CBRD-25312] Modify diagdb utility to accept a class-name parameter #5141

YeunjunLee commented Apr 23, 2024 •

edited

Loading

hornetmj Apr 23, 2024

hornetmj Apr 23, 2024

hornetmj Apr 23, 2024

hornetmj Apr 23, 2024

YeunjunLee Apr 25, 2024

hornetmj Apr 23, 2024

YeunjunLee Apr 25, 2024

hornetmj Apr 23, 2024

hornetmj Apr 25, 2024

hornetmj Apr 25, 2024

hornetmj Apr 25, 2024

hornetmj Apr 25, 2024

hornetmj Apr 25, 2024

hornetmj Apr 25, 2024

YeunjunLee Apr 25, 2024

	heap_dump_specific_file (THREAD_ENTRY * thread_p, FILE * fp, bool dump_records, const char *class_name)
	heap_dump_heap_file (THREAD_ENTRY * thread_p, FILE * fp, bool dump_records, const char *class_name)

[CBRD-25312] Modify diagdb utility to accept a class-name parameter #5141

[CBRD-25312] Modify diagdb utility to accept a class-name parameter #5141

Conversation

YeunjunLee commented Apr 23, 2024 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YeunjunLee commented Apr 23, 2024 •

edited

Loading