On Fri, 7 Aug 2026 at 16:05, Maksim.Melnikov <[email protected]> wrote: > > > On 7/17/26 11:54, Matthias van de Meent wrote: > > On Thu, 11 Jun 2026 at 16:31, Maksim.Melnikov <[email protected]> > > wrote: > >> Patch with fixes was attached. Thanks for review. > > You removed the pgxactoff field with its comment from PGPROC, but > > haven't yet added a comment on PROC_HDR's pgxactoffs. Please add a > > comment to the pgxactoffs array that describes its contents. A simple > > adaptation of the comment removed with PGPROC's pgxactoff field should > > be sufficient. > > > > The comments on PROC_HDR and PGPROC also reference the deleted > > PGPROC->pgxactoff field, so that also must be adjusted; there may be > > several other places, as I didn't do a full check on the codebase. > > > > Once the relevant comments are added and adjusted I think this is > > ready for a committer. > > > > > > Kind regards, > > > > Matthias van de Meent > > Databricks (https://www.databricks.com) > > > > > Sorry for the delay, was unavailable for several weeks. > > Thanks for review, patch was updated.
I noticed this patch wasn't registered at the commitfest yet, so I took the liberty to do that for you. It's registered at [0]. The CFBot then noticed crashes caused by the patch in various tests, which I think can be caused by a lack of initialization of pgxactoffs in ProcArrayShmemAttach, but I haven't worked on a fix. Attached are some copy-edits of comments on top of your v3, and a missed replacement of "GetNumberFromPGProc(MyProc)" with "MyProcNumber". This is an incremental patch on your v3, and doesn't fix the CFBot failures. Kind regards, Matthias van de Meent Databricks (https://www.databricks.com) [0]: https://commitfest.postgresql.org/patch/7139/
From 51619642f910f19788c55b06778629fffa3aa382 Mon Sep 17 00:00:00 2001 From: Matthias van de Meent <[email protected]> Date: Wed, 12 Aug 2026 22:13:38 +0200 Subject: [PATCH v4] Minor copy-edits - Adjusted wording in various comments - Adjust ProcGetMyXactOff to use MyProcNumber, not GetNumberFromPGProc(MyProc) --- src/backend/storage/ipc/procarray.c | 5 +++-- src/include/storage/proc.h | 19 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 3dd46d91811..e00a255fa6d 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -285,8 +285,9 @@ typedef enum KAXCompressReason static PGPROC *allProcs; /* - * offsets into various ProcGlobal->arrays with data mirrored from appropriate PGPROCs, - * procno define offset index in pgxactoffs array(See PROC_HDR for details). + * offsets into various ProcGlobal->arrays with data mirrored from + * appropriate PGPROCs, procno define offset index in pgxactoffs array(See + * PROC_HDR for details). */ static int *pgxactoffs; diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index e08d8dd8a4a..8cbc8928dd5 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -174,7 +174,7 @@ typedef enum * * Some fields in PGPROC (see "mirrored in ..." comment) are mirrored into an * element of more densely packed ProcGlobal arrays. These arrays are indexed - * by PROC_HDR->pgxactoffs array elements. Both copies need to be maintained + * by a PGPROC's pgxactoffs entry. Both copies need to be maintained * coherently. * * NB: The pgxactoff indexed value can *never* be accessed without holding @@ -400,11 +400,11 @@ extern PGDLLIMPORT PGPROC *MyProc; * for PGPROCs that have been added to the shared array with ProcArrayAdd() * (in contrast to PGPROC array which has unused PGPROCs interspersed). * - * The dense arrays are indexed by PROC_HDR->pgxactoffs array elements, procno - * define position of appropriate offset in pgxactoffs array. Any concurrent - * ProcArrayAdd() / ProcArrayRemove() can lead to pgxactoff of a procarray - * member to change. Therefore it is only safe to use appropriate proc's pgxactoff - * to access the dense array while holding either ProcArrayLock or XidGenLock. + * The dense arrays are indexed by the PGPROC's corresponding offset in + * pgxactoff. Any concurrent ProcArrayAdd() / ProcArrayRemove() can cause + * the pgxactoff of a procarray member to change. Therefore it is only safe + * to use a proc's pgxactoff to access the dense array while holding either + * ProcArrayLock or XidGenLock. * * As long as a PGPROC is in the procarray, the mirrored values need to be * maintained in both places in a coherent manner. @@ -446,9 +446,8 @@ typedef struct PROC_HDR PGPROC *allProcs; /* - * offsets into various ProcGlobal->arrays with data mirrored from - * appropriate PGPROCs. The PGPROC's appropriate pgxactoff element index - * is the same as PGPROC index in allProcs(check ProcGetMyXactOff). + * Dense offsets into various ProcGlobal->arrays with data mirrored from + * appropriate PGPROCs. Like allProcs, values are indexed by ProcNumber. */ int *pgxactoffs; @@ -518,7 +517,7 @@ extern PGDLLIMPORT PGPROC *PreparedXactProcs; #define GetPGProcByNumber(n) (&ProcGlobal->allProcs[(n)]) #define GetNumberFromPGProc(proc) ((proc) - &ProcGlobal->allProcs[0]) #define ProcGetXactOff(procno) (ProcGlobal->pgxactoffs[(procno)]) -#define ProcGetMyXactOff() (ProcGetXactOff(GetNumberFromPGProc(MyProc))) +#define ProcGetMyXactOff() (ProcGetXactOff(MyProcNumber)) /* * We set aside some extra PGPROC structures for "special worker" processes, -- 2.50.1 (Apple Git-155)
