Rename injection point names in test_aio
Hi all,
(well, Andres)
93bc3d75d8e1 has introduced a couple of new injection points, but
these don't follow the convention in-place where points are named
more-or-less-like-that. Please find attached a patch to make all
these more consistent.
Thoughts or comments?
--
Michael
Attachments:
aio-injp-names.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/storage/aio/aio.c b/src/backend/storage/aio/aio.c
index 86f7250b7a5f..64e8e81e1849 100644
--- a/src/backend/storage/aio/aio.c
+++ b/src/backend/storage/aio/aio.c
@@ -507,7 +507,7 @@ pgaio_io_process_completion(PgAioHandle *ioh, int result)
pgaio_io_update_state(ioh, PGAIO_HS_COMPLETED_IO);
- pgaio_io_call_inj(ioh, "AIO_PROCESS_COMPLETION_BEFORE_SHARED");
+ pgaio_io_call_inj(ioh, "aio-process-completion-before-shared");
pgaio_io_call_complete_shared(ioh);
diff --git a/src/backend/storage/aio/method_worker.c b/src/backend/storage/aio/method_worker.c
index 8ad17ec1ef73..0fde2a5b30da 100644
--- a/src/backend/storage/aio/method_worker.c
+++ b/src/backend/storage/aio/method_worker.c
@@ -525,7 +525,7 @@ IoWorkerMain(const void *startup_data, size_t startup_data_len)
* To be able to exercise the reopen-fails path, allow injection
* points to trigger a failure at this point.
*/
- pgaio_io_call_inj(ioh, "AIO_WORKER_AFTER_REOPEN");
+ pgaio_io_call_inj(ioh, "aio-worker-after-reopen");
error_errno = 0;
error_ioh = NULL;
diff --git a/src/test/modules/test_aio/test_aio.c b/src/test/modules/test_aio/test_aio.c
index 1d776010ef41..7745244b0e24 100644
--- a/src/test/modules/test_aio/test_aio.c
+++ b/src/test/modules/test_aio/test_aio.c
@@ -86,19 +86,19 @@ test_aio_shmem_startup(void)
inj_io_error_state->enabled_reopen = false;
#ifdef USE_INJECTION_POINTS
- InjectionPointAttach("AIO_PROCESS_COMPLETION_BEFORE_SHARED",
+ InjectionPointAttach("aio-process-completion-before-shared",
"test_aio",
"inj_io_short_read",
NULL,
0);
- InjectionPointLoad("AIO_PROCESS_COMPLETION_BEFORE_SHARED");
+ InjectionPointLoad("aio-process-completion-before-shared");
- InjectionPointAttach("AIO_WORKER_AFTER_REOPEN",
+ InjectionPointAttach("aio-worker-after-reopen",
"test_aio",
"inj_io_reopen",
NULL,
0);
- InjectionPointLoad("AIO_WORKER_AFTER_REOPEN");
+ InjectionPointLoad("aio-worker-after-reopen");
#endif
}
@@ -109,8 +109,8 @@ test_aio_shmem_startup(void)
* critical section.
*/
#ifdef USE_INJECTION_POINTS
- InjectionPointLoad("AIO_PROCESS_COMPLETION_BEFORE_SHARED");
- InjectionPointLoad("AIO_WORKER_AFTER_REOPEN");
+ InjectionPointLoad("aio-process-completion-before-shared");
+ InjectionPointLoad("aio-worker-after-reopen");
elog(LOG, "injection point loaded");
#endif
}
Dear Michael,
93bc3d75d8e1 has introduced a couple of new injection points, but
these don't follow the convention in-place where points are named
more-or-less-like-that. Please find attached a patch to make all
these more consistent.
I have no objections for the patch, but I feel there are no concrete naming rules
(I confused while creating patches). Can we clarify that? E.g., first term should
be a module or process, or something like that.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
On Mon, Apr 14, 2025 at 08:13:44AM +0000, Hayato Kuroda (Fujitsu) wrote:
I have no objections for the patch, but I feel there are no concrete naming rules
(I confused while creating patches).
There has been a sort of implied rule in the code to use lower
characters, with terms separated by dashes. Perhaps we could make
that more official with an extra sentence in the docs about that.
Can we clarify that? E.g., first term should be a module or process,
or something like that.
Not sure that it would be a good thing to put context-specific
restrictions here.
Anyway, would you like to propose a patch for the documentation?
--
Michael
Dear Michael,
I have no objections for the patch, but I feel there are no concrete naming rules
(I confused while creating patches).There has been a sort of implied rule in the code to use lower
characters, with terms separated by dashes. Perhaps we could make
that more official with an extra sentence in the docs about that.
Agreed.
Can we clarify that? E.g., first term should be a module or process,
or something like that.Not sure that it would be a good thing to put context-specific
restrictions here.
My main concern is that in someday the name of injection points might be conflict,
and it might be painful to consider after the number of points is increased.
But it's OK to leave here now.
Anyway, would you like to propose a patch for the documentation?
Sure, I did. Please see [1]/messages/by-id/OSCPR01MB14966E14C1378DEE51FB7B7C5F5B32@OSCPR01MB14966.jpnprd01.prod.outlook.com.
[1]: /messages/by-id/OSCPR01MB14966E14C1378DEE51FB7B7C5F5B32@OSCPR01MB14966.jpnprd01.prod.outlook.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
On Mon, Apr 14, 2025 at 11:14:59AM +0000, Hayato Kuroda (Fujitsu) wrote:
Anyway, would you like to propose a patch for the documentation?
Sure, I did. Please see [1].
[1]: /messages/by-id/OSCPR01MB14966E14C1378DEE51FB7B7C5F5B32@OSCPR01MB14966.jpnprd01.prod.outlook.com
Thanks, I'll check that.
If anybody objects about the points getting renamed here, please let
me know.
--
Michael