date:20150116

Re: [HACKERS] Turning recovery.conf into GUCs

2015-01-16 Thread Michael Paquier

On Wed, Jan 7, 2015 at 6:22 AM, Peter Eisentraut pete...@gmx.net wrote:
 I have written similar logic, and while it's not pleasant, it's doable.
  This issue would really only go away if you don't use a file to signal
 recovery at all, which you have argued for, but which is really a
 separate and more difficult problem.
Moving this patch to the next CF and marking it as returned with
feedback for current CF as there is visibly no consensus reached.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Turning recovery.conf into GUCs

2015-01-16 Thread Andres Freund

On 2015-01-16 21:43:43 +0900, Michael Paquier wrote:
 On Wed, Jan 7, 2015 at 6:22 AM, Peter Eisentraut pete...@gmx.net wrote:
  I have written similar logic, and while it's not pleasant, it's doable.
   This issue would really only go away if you don't use a file to signal
  recovery at all, which you have argued for, but which is really a
  separate and more difficult problem.
 Moving this patch to the next CF and marking it as returned with
 feedback for current CF as there is visibly no consensus reached.

I don't think that's a good idea. If we defer this another couple months
we'l *never* reach anything coming close to concensus.

Greetings,

Andres Freund

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Turning recovery.conf into GUCs

2015-01-16 Thread Michael Paquier

On Fri, Jan 16, 2015 at 9:45 PM, Andres Freund and...@2ndquadrant.com wrote:
 On 2015-01-16 21:43:43 +0900, Michael Paquier wrote:
 On Wed, Jan 7, 2015 at 6:22 AM, Peter Eisentraut pete...@gmx.net wrote:
  I have written similar logic, and while it's not pleasant, it's doable.
   This issue would really only go away if you don't use a file to signal
  recovery at all, which you have argued for, but which is really a
  separate and more difficult problem.
 Moving this patch to the next CF and marking it as returned with
 feedback for current CF as there is visibly no consensus reached.

 I don't think that's a good idea. If we defer this another couple months
 we'l *never* reach anything coming close to concensus.
What makes you think that the situation could move suddendly move into
a direction more than another?
(FWIW, my vote goes to the all GUC approach with standby.enabled.)
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] parallel mode and parallel contexts

2015-01-16 Thread Andres Freund

On 2015-01-05 11:27:57 -0500, Robert Haas wrote:
 On Sat, Jan 3, 2015 at 7:31 PM, Andres Freund and...@2ndquadrant.com wrote:
  * I wonder if parallel contexts shouldn't be tracked via resowners
 
 That is a good question.  I confess that I'm somewhat fuzzy about
 which things should be tracked via the resowner mechanism vs. which
 things should have their own bespoke bookkeeping.  However, the
 AtEOXact_Parallel() stuff happens well before ResourceOwnerRelease(),
 which makes merging them seem not particularly clean.

I'm not sure either. But I think the current location is wrong anyway -
during AtEOXact_Parallel() before running user defined queries via
AfterTriggerFireDeferred() seems wrong.

  * combocid.c should error out in parallel mode, as insurance
 
 Eh, which function?  HeapTupleHeaderGetCmin(),
 HeapTupleHeaderGetCmax(), and AtEOXact_ComboCid() are intended to work
 in parallel mode. HeapTupleHeaderAdjustCmax() could
 Assert(!IsInParallelMode()) but the only calls are in heap_update()
 and heap_delete() which already have error checks, so putting yet
 another elog() there seems like overkill.

To me it seems like a good idea, but whatever.

  * I doubt it's a good idea to allow heap_insert, heap_inplace_update,
index_insert. I'm not convinced that those will be handled correct and
relaxing restrictions is easier than adding them.
 
 I'm fine with adding checks to heap_insert() and
 heap_inplace_update().  I'm not sure we really need to add anything to
 index_insert(); how are we going to get there without hitting some
 other prohibited operation first?

I'm not sure. But it's not that hard to imagine that somebody will start
adding codepaths that insert into indexes first. Think upsert.

  * I'd much rather separate HandleParallelMessageInterrupt() in one
variant that just tells the machinery there's a interrupt (called from
signal handlers) and one that actually handles them. We shouldn't even
consider adding any new code that does allocations, errors and such in
signal handlers. That's just a *bad* idea.
 
 That's a nice idea, but I didn't invent the way this crap works today.
 ImmediateInterruptOK gets set to true while performing a lock wait,
 and we need to be able to respond to errors while in that state.  I
 think there's got to be a better way to handle that than force every
 asynchronous operation to have to cope with the fact that
 ImmediateInterruptOK may be set or not set, but as of today that's
 what you have to do.

I personally think it's absolutely unacceptable to add any more of
that. That the current mess works is more luck than anything else - and
I'm pretty sure there's several bugs in it. But since I realize I can't
force you to develop a alternative solution, I tried to implement enough
infrastructure to deal with it without too much work.

As far as I can see this could relatively easily be implemented ontop of
the removal of ImmediateInterruptOK in the patch series I posted
yesterday? Afaics you just need to remove most of
+HandleParallelMessageInterrupt() and replace it by a single SetLatch().

  * I'm not a fan of the shm_toc stuff... Verbose and hard to read. I'd
much rather place a struct there and be careful about not using
pointers. That also would obliviate the need to reserve some ids.
 
 I don't see how that's going to work with variable-size data
 structures, and a bunch of the things that we need to serialize are
 variable-size.

Meh. Just appending the variable data to the end of the structure and
calculating offsets works just fine.

 Moreover, I'm not really interested in rehashing this
 design again.  I know it's not your favorite; you've said that before.

Well, if I keep having to read code using it, I'll keep being annoyed by
it. I guess we both have to live with that.


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

Re: [HACKERS] Turning recovery.conf into GUCs

2015-01-16 Thread Andres Freund

On 2015-01-16 21:50:16 +0900, Michael Paquier wrote:
 On Fri, Jan 16, 2015 at 9:45 PM, Andres Freund and...@2ndquadrant.com wrote:
  On 2015-01-16 21:43:43 +0900, Michael Paquier wrote:
  On Wed, Jan 7, 2015 at 6:22 AM, Peter Eisentraut pete...@gmx.net wrote:
   I have written similar logic, and while it's not pleasant, it's doable.
This issue would really only go away if you don't use a file to signal
   recovery at all, which you have argued for, but which is really a
   separate and more difficult problem.
  Moving this patch to the next CF and marking it as returned with
  feedback for current CF as there is visibly no consensus reached.
 
  I don't think that's a good idea. If we defer this another couple months
  we'l *never* reach anything coming close to concensus.

 What makes you think that the situation could move suddendly move into
 a direction more than another?

That we have to fix this.

I see absolutely no advantage of declaring the discussion closed for
now. That doesn't exactly increase the chance of this ever succeeding.

Greetings,

Andres Freund

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Bug in pg_dump

2015-01-16 Thread Gilles Darold

On 16/01/2015 01:06, Jim Nasby wrote:
On 1/15/15 5:26 AM, Gilles Darold wrote:
Hello,

There's a long pending issue with pg_dump and extensions that have
table members with foreign keys. This was previously reported in this
thread
http://www.postgresql.org/message-id/ca+tgmoyvzkadmgh_8el7uvm472geru0b4pnnfjqye6ss1k9...@mail.gmail.com
and discuss by Robert. All PostgreSQL users that use the PostGis
extension postgis_topology are facing the issue because the two
members tables (topology and layer) are linked by foreign keys.

If you dump a database with this extension and try to import it you
will experience this error:

pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 3345; 0
157059176 TABLE DATA layer gilles
pg_restore: [archiver (db)] COPY failed for table layer: ERROR:
insert or update on table layer violates foreign key constraint
layer_topology_id_fkey
DETAIL: Key (topology_id)=(1) is not present in table topology.
WARNING: errors ignored on restore: 1

The problem is that, whatever export type you choose (plain/custom
and full-export/data-only) the data of tables topology and layer
are always exported in alphabetic order. I think this is a bug
because outside extension, in data-only export, pg_dump is able to
find foreign keys dependency and dump table's data in the right order
but not with extension's members. Default is alphabetic order but
that should not be the case with extension's members because
constraints are recreated during the CREATE EXTENSION order. I hope I
am clear enough.

Here we have three solutions:

1/ Inform developers of extensions to take care to alphabetical
order when they have member tables using foreign keys.
2/ Inform DBAs that they have to restore the failing table
independently. The use case above can be resumed using the following
command:

pg_restore -h localhost -n topology -t layer -Fc -d
testdb_empty testdump.dump

3/ Inform DBAs that they have to restore the schema first then
the data only using --disable-triggers

I don't like 1-3, and I doubt anyone else does...

4/ Patch pg_dump to solve this issue.

5. Disable FK's during load.
This is really a bigger item than just extensions. It would have the
nice benefit of doing a wholesale FK validation instead of firing
per-row triggers, but it would leave the database in a weird state if
a restore failed...

I think this is an other problem. Here we just need to apply to
extension's members tables the same work than to normal tables. I guess
this is what this patch try to solve.

I attach a patch that solves the issue in pg_dump, let me know if it
might be included in Commit Fest or if the three other solutions are
a better choice. I also join a sample extension (test_fk_in_ext) to
be able to reproduce the issue and test the patch. Note that it might
exists a simpler solution than the one I used in this patch, if this
is the case please point me on the right way, I will be pleased to
rewrite and send an other patch.

The only problem I see with this approach is circular FK's:

decibel@decina.local=# create table a(a_id serial primary key, b_id int);
CREATE TABLE
decibel@decina.local=# create table b(b_id serial primary key, a_id
int references a);
CREATE TABLE
decibel@decina.local=# alter table a add foreign key(b_id) references b;
ALTER TABLE
decibel@decina.local=#

That's esoteric enough that I think it's OK not to directly support
them, but pg_dump shouldn't puke on them (and really should throw a
warning). Though it looks like it doesn't handle that in the data-only
case anyway...

The patch is taking care or circular references and you will be warn if
pg_dump found it in the extension members. That was not the case before.
If you try do dump a database with the postgis extension you will be
warn about FK defined on the edge_data table.

--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

81 matches

Mail list logo