Re: MultiXact truncation, startup et al.

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: MultiXact truncation, startup et al.
Date: 2013-11-29 15:59:42
Message-ID: 20131129155942.GA20057@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2013-11-29 12:49:32 -0300, Alvaro Herrera wrote:
> + * - xidFullScanLimit (also known as the table freeze age) represents the
> + * minimum Xid age past which a vacuum will be turned into a full-table one,
> + * to freeze tuples across the whole table. Vacuuming a table younger than
> + * this can use a partial scan.

Imo "age" isn't really appropriate, since it's a concrete xid that's the
cutoff. It's determined by vacuum_freeze_table_age, sure, but at that
point it's an absolute value.

> + * - mxactFullScanLimit (as xidFullScanLimit) represents the minimum MultiXact
> + * age past which a vacuum will be turned into a full-table one, as with
> + * xidFullScanLimit.

Not an age again.

> + scan_all |= MultiXactIdPrecedesOrEquals(onerel->rd_rel->relminmxid,
> + mxactFullScanLimit);

Hah. That's cute ;).
> @@ -1906,6 +1931,7 @@ CheckPointMultiXact(void)
> SimpleLruFlush(MultiXactOffsetCtl, true);
> SimpleLruFlush(MultiXactMemberCtl, true);
>
> +
> TRACE_POSTGRESQL_MULTIXACT_CHECKPOINT_DONE(true);
> }

My fault, but superflous newline added.

> @@ -8619,6 +8623,22 @@ CreateRestartPoint(int flags)
> }
> LWLockRelease(ControlFileLock);
>
> +

Also an inconsistent newline, again by me :(

> - multi = HeapTupleHeaderGetRawXmax(tuple);
> - if (MultiXactIdPrecedes(multi, cutoff_multi))
> - return true;
> + nmembers = GetMultiXactIdMembers(xid, &members, true);
> + for (i = 0; i < nmembers; i++)
> + {
> + TransactionId member = members[i].xid;
> + Assert(TransactionIdIsNormal(member));
> +
> + /* we don't care about lockers */
> + if (ISUPDATE_from_mxstatus(members[i].status))
> + continue;

Isn't there a ! missing?

> diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
> index c389bf3..518c22d 100644
> --- a/src/backend/access/transam/multixact.c
> +++ b/src/backend/access/transam/multixact.c
> @@ -445,6 +445,10 @@ MultiXactIdExpand(MultiXactId multi, TransactionId xid, MultiXactStatus status)
>
> for (i = 0, j = 0; i < nmembers; i++)
> {
> + /*
> + * FIXME: is it possible that we copy over too old updater xids
> + * here?
> + */
> if (TransactionIdIsInProgress(members[i].xid) ||
> ((members[i].status > MultiXactStatusForUpdate) &&
> TransactionIdDidCommit(members[i].xid)))

That's not really a new thing though, so I am fine with leaving that as
is for now.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2013-11-29 16:33:42 Re: [GENERAL] pg_upgrade ?deficiency
Previous Message Heikki Linnakangas 2013-11-29 15:51:28 Re: [PATCH 1/2] SSL: GUC option to prefer server cipher order