Re: foreign key locks, 2nd attempt

From: Noah Misch <noah(at)leadboat(dot)com>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: foreign key locks, 2nd attempt
Date: 2011-12-13 14:44:49
Message-ID: 20111213144449.GB5736@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Dec 12, 2011 at 05:20:39PM -0300, Alvaro Herrera wrote:
> Excerpts from Noah Misch's message of dom dic 04 09:20:27 -0300 2011:
>
> > Second, I tried a SELECT FOR SHARE on a table of 1M tuples; this might incur
> > some cost due to the now-guaranteed use of pg_multixact for FOR SHARE. See
> > attached fklock-test-forshare.sql. The median run slowed by 7% under the
> > patch, albeit with a rather brief benchmark run. Both master and patched
> > PostgreSQL seemed to exhibit a statement-scope memory leak in this test case:
> > to lock 1M rows, backend-private memory grew by about 500M. When trying 10M
> > rows, I cancelled the query after 1.2 GiB of consumption. This limited the
> > duration of a convenient test run.
>
> I found that this is caused by mxid_to_string being leaked all over the
> place :-( I "fixed" it by making the returned string be a static that's
> malloced and then freed on the next call. There's still virtsize growth
> (not sure it's a legitimate leak) with that, but it's much smaller.

Great. I'll retry that benchmark with the next patch version. I no longer
see a leak on master, so I probably messed up that part of the test somehow.

By the way, do you have a rapid procedure for finding the call site behind a
leak like this?

> This being a debugging aid, I don't think there's any need to backpatch
> this.

Agreed.

> diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
> index ddf76b3..c45bd36 100644
> --- a/src/backend/access/transam/multixact.c
> +++ b/src/backend/access/transam/multixact.c
> @@ -1305,9 +1305,14 @@ mxstatus_to_string(MultiXactStatus status)
> static char *
> mxid_to_string(MultiXactId multi, int nmembers, MultiXactMember *members)
> {
> - char *str = palloc(15 * (nmembers + 1) + 4);
> + static char *str = NULL;
> int i;
>
> + if (str != NULL)
> + free(str);
> +
> + str = malloc(15 * (nmembers + 1) + 4);

Need a check for NULL return.

> +
> snprintf(str, 47, "%u %d[%u (%s)", multi, nmembers, members[0].xid,
> mxstatus_to_string(members[0].status));

Thanks,
nm

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2011-12-13 14:59:48 Re: Command Triggers
Previous Message Andrew Dunstan 2011-12-13 14:44:24 Re: JSON for PG 9.2