Him
On 2014-02-01 17:03:46 +0100, Christian Kruse wrote:
> diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
> index a37e6b6..bef5912 100644
> --- a/doc/src/sgml/monitoring.sgml
> +++ b/doc/src/sgml/monitoring.sgml
> @@ -629,6 +629,16 @@ postgres: <replaceable>user</> <replaceable>database</>
> <replaceable>host</> <re
> </entry>
> </row>
> <row>
> + <entry><structfield>transactionid</structfield></entry>
> + <entry><type>xid</type></entry>
> + <entry>The current transaction identifier.</entry>
> + </row>
The other entries refer to the current backend. Maybe
Toplevel transaction identifier of this backend, if any.
> + <row>
> + <entry><structfield>xmin</structfield></entry>
> + <entry><type>xid</type></entry>
> + <entry>The current <xref linked="ddl-system-columns">xmin</xref>
> value.</entry>
> + </row>
> + <row>
I don't think that link is correct, the xmin you're linking to is about
a row's xmin, while the column you're documenting is the backends
current xmin horizon.
Maybe:
The current backend's xmin horizon.
> <entry><structfield>query</></entry>
> <entry><type>text</></entry>
> <entry>Text of this backend's most recent query. If
> @@ -1484,6 +1494,11 @@ postgres: <replaceable>user</>
> <replaceable>database</> <replaceable>host</> <re
> </entry>
> </row>
> <row>
> + <entry><structfield>xmin</structfield></entry>
> + <entry><type>xid</type></entry>
> + <entry>The current <xref linked="ddl-system-columns">xmin
> value.</xref></entry>
> + </row>
> + <row>
Wrong link again. This should probably read
"This standby's xmin horizon reported by hot_standby_feedback."
> @@ -2785,6 +2787,8 @@ pgstat_read_current_status(void)
> volatile PgBackendStatus *beentry;
> PgBackendStatus *localtable;
> PgBackendStatus *localentry;
> + PGPROC *proc;
> + PGXACT *xact;
A bit hard to read from the diff only, but aren't they now unused?
> char *localappname,
> *localactivity;
> int i;
> @@ -2848,6 +2852,8 @@ pgstat_read_current_status(void)
> /* Only valid entries get included into the local array */
> if (localentry->st_procpid > 0)
> {
> + BackendIdGetTransactionIds(localentry->st_procpid,
> &localentry->transactionid, &localentry->xmin);
> +
That's a bit of a long line, try to keep it to 79 chars.
> /*
> + * BackendIdGetTransactionIds
> + * Get the PGPROC structure for a backend, given the backend ID.
> Also
> + * get the xid and xmin of the backend. The result may be out of
> date
> + * arbitrarily quickly, so the caller must be careful about how
> this
> + * information is used. NULL is returned if the backend is not
> active.
> + */
> +PGPROC *
> +BackendIdGetTransactionIds(int backendPid, TransactionId *xid, TransactionId
> *xmin)
> +{
Hm, why do you even return the PGPROC here? Not that's problematic, but
it seems a bit pointless. If you remove it you can remove a fair bit of
the documentation. I think it should note instead that the two values
can be out of whack with each other.
> + PGPROC *result = NULL;
> + ProcState *stateP;
> + SISeg *segP = shmInvalBuffer;
> + int index = 0;
> + PGXACT *xact;
> +
> + /* Need to lock out additions/removals of backends */
> + LWLockAcquire(SInvalWriteLock, LW_SHARED);
> +
> + if (backendPid > 0)
> + {
> + for (index = 0; index < segP->lastBackend; index++)
> + {
> + if (segP->procState[index].procPid == backendPid)
> + {
> + stateP = &segP->procState[index];
> + result = stateP->proc;
> + xact = &ProcGlobal->allPgXact[result->pgprocno];
> +
> + *xid = xact->xid;
> + *xmin = xact->xmin;
> +
> + break;
> + }
> + }
> + }
> +
> + LWLockRelease(SInvalWriteLock);
> +
> + return result;
> +}
Uh, why do we suddenly need the loop here? BackendIdGetProc() works
without one, so this certainly shouldn't need it, or am I missing
something? Note that Robert withdrew his complaint about relying on
indexing via BackendId in
CA+TgmoaFjcji=Hu9YhCSEL0Z116TY2zEKZf7Z5=da+bptey...@mail.gmail.com .
I also think you need to initialize *xid/*xmin to InvalidTransactionId
if the proc hasn't been found because it exited, otherwise we'll report
stale values.
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
