Re: Allowing multiple concurrent base backups

From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Magnus Hagander <magnus(at)hagander(dot)net>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Allowing multiple concurrent base backups
Date: 2011-01-24 21:02:08
Message-ID: 4D3DE8D0.9080905@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 24.01.2011 22:31, Magnus Hagander wrote:
> On Mon, Jan 24, 2011 at 21:14, Heikki Linnakangas
> <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
>> On 24.01.2011 20:22, Magnus Hagander wrote:
>>> I can't see an explicit check for the user ttempting to do
>>> pg_stop_backup() when there is a nonexclusive backup running? Maybe
>>> I'm reading it wrong? The case being when a user has started a backup
>>> with pg_basebackup but then connects and manually does a
>>> pg_stop_backup. ISTM it drops us ina codepath that just doesn't do the
>>> decrement, but also doesn't throw an error?
>>
>> It throws an error later when it won't find backup_label. For better or
>> worse, it's always been like that.
>
> Isn't that going to leave us in a broken state though? As in a
> mistaken pg_stop_backup() will decrement the counter both breaking the
> streaming base backup, and also possibly throwing an assert when that
> one eventually wants to do it's do_pg_stop_backup()?

Umm, no. pg_stop_backup() won't decrement the counter unless there's an
exclusive backup in operation according to the exclusiveBackup flag.

Hmm, perhaps the code would be more readable if instead of the
forcePageWrites counter that counts exclusive and non-exclusive backups,
and an exclusiveBackup boolean indicating if one of the in-progress
backups is an exclusive one, we had a counter that only counts
non-exclusive backups, plus a boolean indicating if an exclusive backup
is in progress in addition to them.

Attached is a patch for that (against master branch, including only xlog.c).

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Attachment Content-Type Size
more-readable-forcePageWrites.patch text/x-diff 20.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kevin Grittner 2011-01-24 23:36:39 SSI future enhancement: de facto read only optimization
Previous Message Magnus Hagander 2011-01-24 20:31:16 Re: Allowing multiple concurrent base backups