doc patch: clarify the naming rule for injection_points
Dear hackers,
This is a forked thread from [1]/messages/by-id/OSCPR01MB14966B78F3AF15C252EB9E02FF5B32@OSCPR01MB14966.jpnprd01.prod.outlook.com.
Naming rule of points is not determined yet, but most of them have lower cases
and each term are divided by dash "-". I think this is a good chance to formally
clarify it. PSA adding the description.
I was confused the correct place for the description. I added at the end of first
paragraph, because it describes how we add and use it. Suggestions are very
welcomed.
[1]: /messages/by-id/OSCPR01MB14966B78F3AF15C252EB9E02FF5B32@OSCPR01MB14966.jpnprd01.prod.outlook.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
0001-Doc-clarify-the-naming-rule-for-injection-points.patchapplication/octet-stream; name=0001-Doc-clarify-the-naming-rule-for-injection-points.patchDownload+2-2
Hi,
Naming rule of points is not determined yet, but most of them have lower cases
and each term are divided by dash "-". I think this is a good chance to formally
clarify it. PSA adding the description.I was confused the correct place for the description. I added at the end of first
paragraph, because it describes how we add and use it. Suggestions are very
welcomed.
```
- their own code using the same macro.
+ their own code using the same macro. The name of injection points must be
+ lower characters, and dashes must separate its terms.
```
Perhaps "must" is a too strong statement. I suggest something like:
"""
By convention, the name of injection points ...
"""
I have mixed feelings about the patch overall though. This would mean
that we need to rename injection points like this:
``
AtEOXact_Inval-with-transInvalInfo
heap_update-before-pin
```
Otherwise we would contradict our own documentation.
I don't mind heap-update-before-pin, but not 100% sure if:
at-eo-xact-inval-with-trans-inval-info
... would be a better name than the current one.
--
Best regards,
Aleksander Alekseev
Dear Aleksander,
``` - their own code using the same macro. + their own code using the same macro. The name of injection points must be + lower characters, and dashes must separate its terms. ```Perhaps "must" is a too strong statement. I suggest something like:
"""
By convention, the name of injection points ...
"""
It is always difficult for me to distinguish them, thanks for comments.
PSA updated.
I have mixed feelings about the patch overall though. This would mean
that we need to rename injection points like this:``
AtEOXact_Inval-with-transInvalInfo
heap_update-before-pin
```Otherwise we would contradict our own documentation.
Thanks for telling these counterexamples. I grepped with "INJECTION_POINTS" and
"IS_INJECTION_POINT_ATTACHED", and no others are found.
I don't mind heap-update-before-pin, but not 100% sure if:
at-eo-xact-inval-with-trans-inval-info
... would be a better name than the current one.
I also feel just converting lower case is not good. The point seems to locate in
the end-of-transaction callback and it accepts invalidation messages. Based on the
fact, how about "inval-process-invalidation-messages"?
0002 did simple replacements of these words.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
v2-0002-Follow-naming-rules-for-injection-points.patchapplication/octet-stream; name=v2-0002-Follow-naming-rules-for-injection-points.patchDownload+32-33
v2-0001-Doc-clarify-the-naming-rule-for-injection-points.patchapplication/octet-stream; name=v2-0001-Doc-clarify-the-naming-rule-for-injection-points.patchDownload+2-2
On Mon, Apr 14, 2025 at 12:36:20PM +0000, Hayato Kuroda (Fujitsu) wrote:
I also feel just converting lower case is not good. The point seems to locate in
the end-of-transaction callback and it accepts invalidation messages. Based on the
fact, how about "inval-process-invalidation-messages"?
0002 did simple replacements of these words.
- INJECTION_POINT("heap_update-before-pin");
+ INJECTION_POINT("heap-update-before-pin");
The former name is better IMO. heap_update() is the name of the
function where the point is defined.
- INJECTION_POINT("AtEOXact_Inval-with-transInvalInfo");
+ INJECTION_POINT("inval-process-invalidation-messages");
This naming has been puzzling me, though, so a rename looks fit. I am
not sure that this is the best name that matches with this code path,
as this lacks a reference regarding the end of a transaction. Perhaps
something like "end-transaction-process-inval" would be better?
--
Michael
Dear Michael,
I also feel just converting lower case is not good. The point seems to locate in
the end-of-transaction callback and it accepts invalidation messages. Based onthe
fact, how about "inval-process-invalidation-messages"?
0002 did simple replacements of these words.- INJECTION_POINT("heap_update-before-pin"); + INJECTION_POINT("heap-update-before-pin");The former name is better IMO. heap_update() is the name of the
function where the point is defined.- INJECTION_POINT("AtEOXact_Inval-with-transInvalInfo"); + INJECTION_POINT("inval-process-invalidation-messages");This naming has been puzzling me, though, so a rename looks fit. I am
not sure that this is the best name that matches with this code path,
as this lacks a reference regarding the end of a transaction. Perhaps
something like "end-transaction-process-inval" would be better?
Thanks for suggesting them. ISTM, you are correct. PSA updated version.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
v3-0001-Doc-clarify-the-naming-rule-for-injection-points.patchapplication/octet-stream; name=v3-0001-Doc-clarify-the-naming-rule-for-injection-points.patchDownload+2-2
v3-0002-Follow-naming-rules-for-injection-points.patchapplication/octet-stream; name=v3-0002-Follow-naming-rules-for-injection-points.patchDownload+16-17
On Mon, Apr 21, 2025 at 12:10:51PM +0000, Hayato Kuroda (Fujitsu) wrote:
Thanks for suggesting them. ISTM, you are correct. PSA updated version.
Thanks, I've applied some slight tweaks, and applied the result down
to v17, leaving the heap_update point alone.
--
Michael