Re: [HACKERS] logical decoding of two-phase transactions

2017-09-07 Thread Nikhil Sontakke
nt HEAD, so I'll wait for a rebased > version. -- Nikhil Sontakke http://www.2ndQuadrant.com/ PostgreSQL/Postgres-XL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your su

Re: [HACKERS] StandbyRecoverPreparedTransactions recovers subtrans links incorrectly

2017-04-26 Thread Nikhil Sontakke
> I'm suggesting we take the approach that if there is a problem we can > recreate it as a way of exploring what conditions are required and > therefore work out the impact. Nikhil Sontakke appears to have > re-created something, but not quite what I had expected. I think he >

Re: [HACKERS] StandbyRecoverPreparedTransactions recovers subtrans links incorrectly

2017-04-24 Thread Nikhil Sontakke
--- I am wondering if StandbyRecoverPreparedTransactions() needs the parent linkage at all? I don't see SubTransGetParent() and related functions being called on the standby but we need to be careful here. As a quick test, If I don't call SubTransSetParent() as part of StandbyRecoverPrepa

Re: [HACKERS] Failed recovery with new faster 2PC code

2017-04-18 Thread Nikhil Sontakke
Please find attached a second version of my bug fix which is stylistically better and clearer than the first one. Regards, Nikhils On 18 April 2017 at 13:47, Nikhil Sontakke <nikh...@2ndquadrant.com> wrote: > Hi, > > There was a bug in the redo 2PC remove code path. Because of w

Re: [HACKERS] Failed recovery with new faster 2PC code

2017-04-18 Thread Nikhil Sontakke
know. Regards, Nikhils On 18 April 2017 at 12:09, Nikhil Sontakke <nikh...@2ndquadrant.com> wrote: > > > On 17 April 2017 at 15:02, Nikhil Sontakke <nikh...@2ndquadrant.com> > wrote: > >> >> >>> >> commit 728bd991c3c4389fb39c45dcb0fe57e4a1dc

Re: [HACKERS] Failed recovery with new faster 2PC code

2017-04-18 Thread Nikhil Sontakke
On 17 April 2017 at 15:02, Nikhil Sontakke <nikh...@2ndquadrant.com> wrote: > > >> >> commit 728bd991c3c4389fb39c45dcb0fe57e4a1dccd71 >> >> Author: Simon Riggs <si...@2ndquadrant.com> >> >> Date: Tue Apr 4 15:56:56 2017 -0400 >> >&g

Re: [HACKERS] Failed recovery with new faster 2PC code

2017-04-17 Thread Nikhil Sontakke
> >> commit 728bd991c3c4389fb39c45dcb0fe57e4a1dccd71 > >> Author: Simon Riggs > >> Date: Tue Apr 4 15:56:56 2017 -0400 > >> > >>Speedup 2PC recovery by skipping two phase state files in normal path > > > > Thanks Jeff for your tests. > > > > So that's now two crash

Re: [HACKERS] Speedup twophase transactions

2017-03-28 Thread Nikhil Sontakke
for committer". This patch/feature has been a long journey! :) Regards, Nikhils -- Nikhil Sontakke http://www.2ndQuadrant.com/ PostgreSQL/Postgres-XL Development, 24x7 Support, Training & Services

Re: [HACKERS] Speedup twophase transactions

2017-03-27 Thread Nikhil Sontakke
Please ignore reports about errors in other tests. Seem spurious.. Regards, Nikhils On 28 March 2017 at 10:40, Nikhil Sontakke <nikh...@2ndquadrant.com> wrote: > Hi Micheal, > > The latest patch looks good. By now doing a single scan of shmem two phase > data, we have remove

Re: [HACKERS] Speedup twophase transactions

2017-03-27 Thread Nikhil Sontakke
'' # expected: '1' not ok 8 - connect to node standby_1 if mode "any" and standby_1,master listed" Again, not related to this recovery code path, but not sure if others see this as well. Regards, Nikhils On 27 March 2017 at 05:35, Michael Paquier <michael.paqu...@gmail.com&

Re: [HACKERS] Speedup twophase transactions

2017-03-26 Thread Nikhil Sontakke
wrote: > > On Fri, Mar 17, 2017 at 5:00 PM, Nikhil Sontakke > > <nikh...@2ndquadrant.com> wrote: > >> Micheal, it looks like you are working on a final version of this > patch? I > >> will wait to review it from my end, then. > > > > I have to admit t

Re: [HACKERS] Speedup twophase transactions

2017-03-17 Thread Nikhil Sontakke
it looks like you are working on a final version of this patch? I will wait to review it from my end, then. Regards, Nikhils -- Nikhil Sontakke http://www.2ndQuadrant.com/ PostgreSQL/Postgres-XL Development, 24x7 Support, Training & Services

Re: [HACKERS] Speedup twophase transactions

2017-03-17 Thread Nikhil Sontakke
the shmem entries. BUT, we cannot ignore "inredo"+"ondisk" entries. For such entries, we will have to read and recover from the corresponding 2PC files. Regards, Nikhils -- Nikhil Sontakke http://www.2ndQuadrant.com/ PostgreSQL/Postgres-XL Development, 24x7 Support, Training & Services

Re: [HACKERS] Speedup twophase transactions

2017-03-17 Thread Nikhil Sontakke
ed_xacts number restricts the number of pending PREPARED transactions *across* the 2PC files and shmem inredo entries. We can never have more entries than this value. Regards, Nikhils -- Nikhil Sontakke http://www.2ndQuadrant.com/ PostgreSQL/Postgres-XL Development, 24x7 Support, Training & Services

Re: [HACKERS] Speedup twophase transactions

2017-03-16 Thread Nikhil Sontakke
> > + * * RecoverPreparedTransactions(), StandbyRecoverPreparedTransact > ions() > + *and PrescanPreparedTransactions() have been modified to go > throug > + *gxact->inredo entries that have not made to disk yet. > > It seems to me that there should be an initial scan of

Re: [HACKERS] Speedup twophase transactions

2017-03-15 Thread Nikhil Sontakke
UPDATE pgbench_accounts SET abalance = abalance + :delta WHERE aid = :to_aid; PREPARE TRANSACTION ':client_id.:scale'; COMMIT PREPARED ':client_id.:scale'; Created a base backup with scale factor 125 on an AWS t2.large instance. Set up archiving and did a 20 minute run with the

Re: [HACKERS] Speedup twophase transactions

2017-03-11 Thread Nikhil Sontakke
Hi David and Michael, > It would be great to get this thread closed out after 14 months and many > commits. > > PFA, latest patch which addresses Michael's comments. twophase.c: In function ‘PrepareRedoAdd’: > twophase.c:2539:20: warning: variable ‘gxact’ set but not used >

Re: [HACKERS] Speedup twophase transactions

2017-02-26 Thread Nikhil Sontakke
gt; + { > + found = true; > + break; > + } > I would not have thought so. > > Since we are using the TwoPhaseState structure to track redo entries, at end of recovery, we will find existing entries. Please see my comments above for RecoverPrep

Re: [HACKERS] Speedup twophase transactions

2017-02-01 Thread Nikhil Sontakke
se shared memory entries in PrescanPreparedTransactions, RecoverPreparedTransactions and StandbyRecoverPreparedTransactions. Other than this, I ran TAP tests and they succeed as needed. Regards, Nikhils -- Nikhil Sontakke http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Supp

Re: [HACKERS] Speedup twophase transactions

2017-02-01 Thread Nikhil Sontakke
ng prepared transaction, which I > think is true, then I agree with you. PFA, small patch to fix this, then. Regards, Nikhils -- Nikhil Sontakke http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services twophase_typo.patch Description: Binar

Re: [HACKERS] Speedup twophase transactions

2017-01-30 Thread Nikhil Sontakke
yesterday. How about adding entries in TwoPhaseState itself (which become valid later)? Only if it does not cause a lot of code churn. The current non-shmem list patch only needs to handle standby shutdowns correctly. Other aspects like standby promotion/recovery are handled ok AFAICS. Regards

Re: [HACKERS] Speedup twophase transactions

2017-01-30 Thread Nikhil Sontakke
equence will trigger the issue: 1) start master 2) start replica 3) prepare a transaction on master 4) shutdown master 5) shutdown replica CheckPointTwoPhase() in (5) does not sync this prepared transaction because the checkpointer's KnownPreparedList is empty. Regards, Nikhils -- Nikhil So

Re: [HACKERS] Speedup twophase transactions

2017-01-30 Thread Nikhil Sontakke
e when we do shutdown of standby. We could add code in XLOG_CHECKPOINT_SHUTDOWN and XLOG_CHECKPOINT_ONLINE xlog_redo code path. Regards, Nikhils -- Nikhil Sontakke http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via

Re: [HACKERS] Speedup twophase transactions

2017-01-27 Thread Nikhil Sontakke
will be modified to go through the KnownPreparedList to reload shmem appropriately for 2PC. So, all good in that case. Apologies for the digression. Regards, Nikhils -- Nikhil Sontakke http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & S

Re: [HACKERS] Speedup twophase transactions

2017-01-27 Thread Nikhil Sontakke
On 27 January 2017 at 15:37, Simon Riggs <si...@2ndquadrant.com> wrote: > On 27 January 2017 at 09:59, Nikhil Sontakke <nikh...@2ndquadrant.com> wrote: >>>> But, I put the recovery process and the checkpointer process of the >>>> standby under gdb with break

Re: [HACKERS] Speedup twophase transactions

2017-01-27 Thread Nikhil Sontakke
happen at promotion since 9.3. You can > still use fallback_promote as promote file to trigger the pre-9.2 (9.2 > included) behavior. Ok, so that means, we also need to fsync out these 2PC XIDs at promote time as well for their durability. Regards, Nikhils -- Nikhil Sontakke

Re: [HACKERS] Speedup twophase transactions

2017-01-26 Thread Nikhil Sontakke
ess of the standby under gdb with breakpoints on these functions, but both did not hit CreateRestartPoint() as well as CheckPointGuts() when I issued a promote :-| Regards, Nikhils -- Nikhil Sontakke http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &am

Re: [HACKERS] Speedup twophase transactions

2017-01-25 Thread Nikhil Sontakke
this patch. This is a problem as a > new restart point is created... Having the flush in CheckpointTwoPhase > really makes the most sense. Umm, AFAICS, CheckPointTwoPhase() does not get called in the "standby promote" code path. Regards, Nikhils -- Nikhil Sontakke http

Re: [HACKERS] Speedup twophase transactions

2017-01-25 Thread Nikhil Sontakke
arge of doing this work and not the > startup process, so this should go into CheckpointTwoPhase() instead. I don't see a function by the above name in the code? Regards, Nikhils -- Nikhil Sontakke http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training

Re: [HACKERS] Speedup twophase transactions

2017-01-25 Thread Nikhil Sontakke
>> patched, without constant cache_drop: >>total recovery time: 68s >> >> (while difference is significant, i bet that happens mostly because of >> database file segments should be re-read after cache drop) >> >> master, without constant cache_drop: >

Re: [HACKERS] Speedup twophase transactions

2017-01-24 Thread Nikhil Sontakke
: 86s > > patched, without constant cache_drop: >total recovery time: 68s > > (while difference is significant, i bet that happens mostly because of > database file segments should be re-read after cache drop) > > master, without constant cache_drop: >time to r

Re: [HACKERS] Speedup twophase transactions

2017-01-24 Thread Nikhil Sontakke
really more funky. > Yes, postgres-xl is full of 2PC, so hopefully this optimization should help a lot in that case as well. Regards, Nikhils -- Nikhil Sontakke http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailin

Re: [HACKERS] Speedup twophase transactions

2017-01-23 Thread Nikhil Sontakke
Hi Stas, > > So, here is brand new implementation of the same thing. > > Now instead of creating pgproc entry for prepared transaction during recovery, > I just store recptr/xid correspondence in separate 2L-list and > deleting entries in that > list if redo process faced commit/abort. In case of

Re: [HACKERS] Compression of full-page-writes

2013-08-29 Thread Nikhil Sontakke
Hi Fujii-san, I must be missing something really trivial, but why not try to compress all types of WAL blocks and not just FPW? Regards, Nikhils On Fri, Aug 30, 2013 at 8:25 AM, Fujii Masao masao.fu...@gmail.com wrote: Hi, Attached patch adds new GUC parameter 'compress_backup_block'.

Re: [HACKERS] Custom gucs visibility

2013-07-03 Thread Nikhil Sontakke
If we haven't loaded the .so yet, where would we get the list of custom GUCs from? This has come up before. We could show the string value of the GUC, if it's been set in postgresql.conf, but we do not have correct values for any of the other columns in pg_settings; nor are we even

[HACKERS] Custom gucs visibility

2013-07-02 Thread Nikhil Sontakke
Hi, So, if I have a contrib module which adds in a custom guc via DefineCustomIntVariable() for example. Now that custom guc is not visible via a show command unless that corresponding .so has been loaded into memory. E.g. postgres=# show pg_buffercache.fancy_custom_guc; ERROR: unrecognized

Re: [HACKERS] Bug in ALTER COLUMN SET DATA TYPE ?

2012-11-02 Thread Nikhil Sontakke
postgres=# CREATE TABLE test (a float check (a 10.2)); CREATE TABLE postgres=# CREATE TABLE test_child() INHERITS(test); CREATE TABLE postgres=# ALTER TABLE test ALTER COLUMN a SET DATA TYPE numeric; ERROR: constraint must be added to child tables too Interestingly, this works in

Re: [HACKERS] Bug in ALTER COLUMN SET DATA TYPE ?

2012-11-02 Thread Nikhil Sontakke
So coming back to the issue, do you think it's a good idea to teach ATAddCheckConstraint() that the call is coming from a late phase of ALTER TABLE ? +1 You mentioned AT_PASS_OLD_INDEX in your original mail, but I guess that you meant that we should check for AT_PASS_OLD_CONSTR and then not

Re: [HACKERS] xml_is_document and selective pg_re_throw

2012-06-13 Thread Nikhil Sontakke
No, I don't see any particular risk there. The places that might throw ERRCODE_INVALID_XML_DOCUMENT are sufficiently few (as in, exactly one, in this usage) that we can have reasonable confidence we know what the system state is when we catch that error. Hmmm, I was writing some code in

[HACKERS] xml_is_document and selective pg_re_throw

2012-06-12 Thread Nikhil Sontakke
Hi, Consider: SELECT xml 'foobar/foobarfoo/bar' IS DOCUMENT; And I was looking at xml_is_document() source code. It calls xml_parse which throws an error with code set to ERRCODE_INVALID_XML_DOCUMENT. The catch block of xml_parse then rethrows. Now xml_is_document does a selective rethrow only

Re: [HACKERS] B-tree page deletion boundary cases

2012-04-24 Thread Nikhil Sontakke
Was wondering if there's a similar bug which gets triggered while using VACUUM FULL. See for instance this thread: http://postgresql.1045698.n5.nabble.com/index-corruption-in-PG-8-3-13-td4257589.html This issue has been reported on-off from time to time and in most cases VACUUM or

Re: [HACKERS] B-tree page deletion boundary cases

2012-04-21 Thread Nikhil Sontakke
Hi Noah, Was wondering if there's a similar bug which gets triggered while using VACUUM FULL. See for instance this thread: http://postgresql.1045698.n5.nabble.com/index-corruption-in-PG-8-3-13-td4257589.html This issue has been reported on-off from time to time and in most cases VACUUM or

Re: [HACKERS] how to create a non-inherited CHECK constraint in CREATE TABLE

2012-04-16 Thread Nikhil Sontakke
Displace yes. It would error out if someone says ALTER TABLE ONLY... CHECK (); suggesting to use the ONLY with the CHECK. I'd say the behavior for that case can revert to the PostgreSQL 9.1 behavior. If the table has children, raise an error. Otherwise, add an inheritable CHECK

Re: [HACKERS] how to create a non-inherited CHECK constraint in CREATE TABLE

2012-04-11 Thread Nikhil Sontakke
Hi, So, I have a patch for this. This patch introduces support for CHECK ONLY syntax while doing a CREATE TABLE as well as during the usual ALTER TABLE command. Example: create table atacc7 (test int, test2 int CHECK ONLY (test0), CHECK (test210)); create table atacc8 () inherits (atacc7);

Re: [HACKERS] how to create a non-inherited CHECK constraint in CREATE TABLE

2012-04-11 Thread Nikhil Sontakke
Hi, Cumulative reaction to all the responses first: Whoa! :) I was under the impression that a majority of us felt that the current mechanism was inadequate. Also if you go through the nabble thread, the fact that CREATE TABLE did not support such constraints was considered to be an annoyance.

Re: [HACKERS] Review of patch renaming constraints

2012-01-19 Thread Nikhil Sontakke
Make check passed. Patch has tests for rename constraint. Most normal uses of alter table ... rename constraint ... worked normally. However, the patch does not deal correctly with constraints which are not inherited, such as primary key constraints: That appears to be because creating

Re: [HACKERS] Review of patch renaming constraints

2012-01-19 Thread Nikhil Sontakke
And primary keys are anyways not inherited. So why is the conisonly field interfering in rename? Seems quite orthogonal to me. In the past, each kind of constraint was either always inherited or always not, implicitly. Now, for check constraints we can choose what we want, and in the

Re: [HACKERS] how to create a non-inherited CHECK constraint in CREATE TABLE

2012-01-17 Thread Nikhil Sontakke
It appears that the only way to create a non-inherited CHECK constraint is using ALTER TABLE. Is there no support in CREATE TABLE planned? That looks a bit odd. There are no plans to do that AFAIR, though maybe you could convince Nikhil to write the patch to do so. That certainly

Re: [HACKERS] Review: Non-inheritable check constraints

2012-01-16 Thread Nikhil Sontakke
I have also tried to change the error message as per Alvaro's suggestions. I will really try to see if we have other issues. Really cannot have Robert reporting all the bugs and getting annoyed, but there are lotsa variations possible with inheritance.. So did you find anything?

Re: [HACKERS] Review: Non-inheritable check constraints

2012-01-16 Thread Nikhil Sontakke
I will really try to see if we have other issues. Really cannot have Robert reporting all the bugs and getting annoyed, but there are lotsa variations possible with inheritance.. BTW thank you for doing the work on this. Probably the reason no one bothers too much with inheritance is

Re: [HACKERS] Review: Non-inheritable check constraints

2011-12-26 Thread Nikhil Sontakke
I don't think this is a given ... In fact, IMO if we're only two or three fixes away from having it all nice and consistent, I think reverting is not necessary. Sure. It's the if part of that sentence that I'm not too sure about. Any specific area of the code that you think is/has

Re: [HACKERS] Review: Non-inheritable check constraints

2011-12-22 Thread Nikhil Sontakke
Hi, There is at least one other problem. Consider: rhaas=# create table a (ff1 int, constraint chk check (ff1 0)); CREATE TABLE rhaas=# create table b (ff1 int, constraint chk check (ff1 0)); CREATE TABLE rhaas=# alter table b inherit a; ALTER TABLE This needs to fail if chk is an

Re: [HACKERS] Review: Non-inheritable check constraints

2011-12-22 Thread Nikhil Sontakke
refused to inherit. Regards, Nikhils On Fri, Dec 23, 2011 at 8:55 AM, Nikhil Sontakke nikkh...@gmail.com wrote: Hi, There is at least one other problem. Consider: rhaas=# create table a (ff1 int, constraint chk check (ff1 0)); CREATE TABLE rhaas=# create table b (ff1 int, constraint chk

Re: [HACKERS] Review: Non-inheritable check constraints

2011-12-22 Thread Nikhil Sontakke
I don't think this is a given ... In fact, IMO if we're only two or three fixes away from having it all nice and consistent, I think reverting is not necessary. FWIW, here's a quick fix for the issue that Robert pointed out. Again it's a variation of the first issue that he reported. We have

Re: [HACKERS] Review: Non-inheritable check constraints

2011-12-20 Thread Nikhil Sontakke
Agreed. I just tried out the scenarios laid out by you both with and without the committed patch and AFAICS, normal inheritance semantics have been preserved properly even after the commit. No, they haven't. I didn't expect this to break anything when you have two constraints with

Re: [HACKERS] Review: Non-inheritable check constraints

2011-12-20 Thread Nikhil Sontakke
rhaas=# create table A(ff1 int); CREATE TABLE rhaas=# create table B () inherits (A); CREATE TABLE rhaas=# create table C () inherits (B); CREATE TABLE rhaas=# alter table only b add constraint chk check (ff1 0); ALTER TABLE rhaas=# alter table a add constraint chk check (ff1 0);

Re: [HACKERS] Review: Non-inheritable check constraints

2011-12-19 Thread Nikhil Sontakke
It seems hard to believe that ATExecDropConstraint() doesn't need any adjustment. Yeah, I think we could return early on for only type of constraints. Also, shouldn't the systable scan break out of the while loop after a matching constraint name has been found? As of now, it's doing a full

Re: [HACKERS] Review: Non-inheritable check constraints

2011-12-19 Thread Nikhil Sontakke
PFA, revised version containing the above changes based on Alvaro's v4 patch. Committed with these fixes, and with my fix for the pg_dump issue noted by Alex, too. Thanks. Thanks a lot Alvaro! Regards, Nikhils -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company -

Re: [HACKERS] Review: Non-inheritable check constraints

2011-12-19 Thread Nikhil Sontakke
But did you see Robert Haas' response upthread? It looks like there's still some work to do -- at least some research to determine that we're correctly handling all those cases. As the committer I'm responsible for it, but I don't have resources to get into it any time soon. Yeah, am

Re: [HACKERS] Review: Non-inheritable check constraints

2011-12-19 Thread Nikhil Sontakke
Hi Robert, First of all, let me state that this ONLY feature has not messed around with existing inheritance semantics. It allows attaching a constraint to any table (which can be part of any hierarchy) without the possibility of it ever playing any part in future or existing inheritance

Re: [HACKERS] Review: Non-inheritable check constraints

2011-12-17 Thread Nikhil Sontakke
it to ORDER BY 2 desc, 1 to have that effect. Your call finally. Regards, Nikhils On Sat, Dec 17, 2011 at 12:36 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Greg Smith's message of vie dic 16 15:02:20 -0300 2011: On 12/04/2011 02:22 AM, Nikhil Sontakke wrote

Re: [HACKERS] Review: Non-inheritable check constraints

2011-12-03 Thread Nikhil Sontakke
I had a look at this patch today. The pg_dump bits conflict with another patch I committed a few days ago, so I'm about to merge them. I have one question which is about this hunk: Thanks for taking a look Alvaro. @@ -2312,6 +2317,11 @@ MergeWithExistingConstraint(Relation rel, char

Re: [HACKERS] So where are we on the open commitfest?

2011-11-14 Thread Nikhil Sontakke
* Non-inheritable check constraints So, this patch got shifted to the next commitfest... Regards, Nikhils

Re: [HACKERS] Concurrent CREATE TABLE/DROP SCHEMA leaves inconsistent leftovers

2011-11-14 Thread Nikhil Sontakke
If all you need to do is lock a schema, you can just call LockDatabaseObject(NamespaceRelationId, namespace_oid, 0, AccessShareLock); there's no need to fake up an objectaddress just to take a lock. But I think that's not really all you need to do, because somebody could drop the namespace

Re: [HACKERS] Concurrent CREATE TABLE/DROP SCHEMA leaves inconsistent leftovers

2011-11-14 Thread Nikhil Sontakke
So it's probably going to take a while to get this completely nailed down, but we can keep chipping away at it. Agreed. So are you planning to commit this change? Or we want some more objects to be fixed? Last I looked at this, we will need locking to be done while creating tables, views,

Re: [HACKERS] Re: pg_dump: schema with OID XXXXX does not exist - was Concurrent CREATE TABLE/DROP SCHEMA leaves inconsistent leftovers

2011-11-11 Thread Nikhil Sontakke
Wasn't 7.3 the release that introduced schemas in the first place? I think there's a very good chance that the older reports with similar symptoms are completely unrelated, anyhow. Tom Lane is reluctant and that should tell me something :) So unless the list feels that this should be fixed

Re: [HACKERS] Concurrent CREATE TABLE/DROP SCHEMA leaves inconsistent leftovers

2011-11-10 Thread Nikhil Sontakke
Hi, Ok, understood. PFA, a patch against git head. We take the AccessShareLock lock on the schema in DefineRelation now. Note that we do not want to interlock with other concurrent creations in the schema. We only want to interlock with deletion activity. So even performance wise this should

Re: [HACKERS] Concurrent CREATE TABLE/DROP SCHEMA leaves inconsistent leftovers

2011-11-10 Thread Nikhil Sontakke
Um ... why would we do this only for tables, and not for creations of other sorts of objects that belong to schemas? Right, we need to do it for other objects like functions etc. too. Also, if we are going to believe that this is a serious problem, what of ALTER ... SET SCHEMA? I admit,

Re: [HACKERS] Concurrent CREATE TABLE/DROP SCHEMA leaves inconsistent leftovers

2011-11-10 Thread Nikhil Sontakke
Continuing in gdb, also completes the creation of c2 table without any errors. We are now left with a dangling entry in pg_class along with all the corresponding data files in our data directory. The problem becomes worse if c2 was created using a TABLESPACE. Now dropping of that

[HACKERS] pg_dump: schema with OID XXXXX does not exist - was Concurrent CREATE TABLE/DROP SCHEMA leaves inconsistent leftovers

2011-11-10 Thread Nikhil Sontakke
Hi, But if it's deemed to be a problem, I want to see a solution that's actually watertight.) After Daniel's hunch about pg_dump barfing due to such leftover entries proving out to be true, we have one credible explanation (there might be other reasons too) for this long standing issue. I

[HACKERS] Concurrent CREATE TABLE/DROP SCHEMA leaves inconsistent leftovers

2011-11-09 Thread Nikhil Sontakke
Hi, Consider the following sequence of events: s1 # CREATE SCHEMA test_schema; s1 # CREATE TABLE test_schema.c1(x int); Now open another session s2 and via gdb issue a breakpoint on heap_create_with_catalog() which is called by DefineRelation(). s2 # CREATE TABLE test_schema.c2(y int); The

Re: [HACKERS] Concurrent CREATE TABLE/DROP SCHEMA leaves inconsistent leftovers

2011-11-09 Thread Nikhil Sontakke
We definitely need some interlocking to handle this. For lack of better APIs, we could do a LockDatabaseObject() call in AccessShareLock mode on the namespace and release the same on completion of the creation of the object. Thoughts? In general, we've been reluctant to add locking

Re: [HACKERS] Concurrent CREATE TABLE/DROP SCHEMA leaves inconsistent leftovers

2011-11-09 Thread Nikhil Sontakke
Yeah thanks, that does the object locking. For pre-9.1 versions, we will need a similar solution. I encountered the issue on 8.3.x.. I don't think we should back-patch a fix of this type. There is a lot of cruftiness of this type scattered throughout the code base, and if we start

Re: [HACKERS] Review: Non-inheritable check constraints

2011-10-07 Thread Nikhil Sontakke
Hi Alex, Hmmm, your patch checks for a constraint being only via: !recurse !recursing I hope that is good enough to conclusively conclude that the constraint is 'only'. This check was not too readable in the existing code for me anyways ;). If we check at the grammar

Re: [HACKERS] Review: Non-inheritable check constraints

2011-10-07 Thread Nikhil Sontakke
for the thorough review and subsequent changes! Regards, Nikhils On Fri, Oct 7, 2011 at 12:18 PM, Alex Hunsaker bada...@gmail.com wrote: On Fri, Oct 7, 2011 at 00:28, Nikhil Sontakke nikkh...@gmail.com wrote: Hi Alex, So with it all spelled out now I see the constraint must be added to child

Re: [HACKERS] Review: Non-inheritable check constraints

2011-10-06 Thread Nikhil Sontakke
Hi Alex, I didn't care for the changes to gram.y so I reworked it a bit so we now pass is_only to AddRelationNewConstraint() (like we do with is_local). Seemed simpler but maybe I missed something. Comments? Hmmm, your patch checks for a constraint being only via: !recurse

[HACKERS] Issue with listing same tablenames from different schemas in the search_path

2011-10-02 Thread Nikhil Sontakke
Hi, Consider the following sequence of commands in a psql session: postgres=#create table public.sample(x int); postgres=#create schema new; postgres=#create table new.sample(x int); postgres=#set search_path=public,new; postgres=#\dt Schema | Name | Type | Owner

Re: [HACKERS] Issue with listing same tablenames from different schemas in the search_path

2011-10-02 Thread Nikhil Sontakke
postgres=#create table public.sample(x int); postgres=#create schema new; postgres=#create table new.sample(x int); postgres=#set search_path=public,new; postgres=#\dt Schema | Name | Type | Owner --**- public | sample | table | postgres

Re: [HACKERS] cataloguing NOT NULL constraints

2011-08-04 Thread Nikhil Sontakke
So after writing the code to handle named NOT NULL constraints for tables, I'm thinking that dumpConstraints needs to be fixed thusly: @@ -12888,6 +12968,27 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)                         NULL, NULL);        }    } +   else if

Re: [HACKERS] Check constraints on partition parents only?

2011-07-29 Thread Nikhil Sontakke
Hi, Any preferences for the name? connoinh conisonly constatic or confixed I'd probably pick conisonly from those choices. The use of \d inside psql will show ONLY constraints without any embellishments similar to normal constraints. E.g. ALTER TABLE ONLY a ADD CONSTRAINT achk CHECK

Re: [HACKERS] Check constraints on partition parents only?

2011-07-29 Thread Nikhil Sontakke
psql=# \d a Table public.a Column | Type | Modifiers +-+--- b | integer | Check constraints: achk CHECK (false) bchk CHECK (b 0) Is this acceptable? Or we need to put in work into psql to show ONLY somewhere in the description? If yes, ONLY

Re: [HACKERS] Check constraints on partition parents only?

2011-07-29 Thread Nikhil Sontakke
Yeah, I have already hacked it a bit. This constraint now needs to be spit out later as an ALTER command with ONLY attached to it appropriately. Earlier all CHECK constraints were generally emitted as part of the table definition itself. Hrm. That doesn't seem so good. Maybe we've got

Re: [HACKERS] Check constraints on partition parents only?

2011-07-29 Thread Nikhil Sontakke
Yeah, I have already hacked it a bit. This constraint now needs to be spit out later as an ALTER command with ONLY attached to it appropriately. Earlier all CHECK constraints were generally emitted as part of the table definition itself. IIRC, there's already support for splitting out a

Re: [HACKERS] Check constraints on partition parents only?

2011-07-29 Thread Nikhil Sontakke
We could imagine doing something like CHECK ONLY (foo), but that seems quite non-orthogonal with (a) everything else in CREATE TABLE, and (b) ALTER TABLE ONLY. Yeah, I thought about CHECK ONLY support as part of table definition, but as you say - it appears to be too non-standard right now

Re: [HACKERS] Check constraints on partition parents only?

2011-07-29 Thread Nikhil Sontakke
Hi all, PFA, patch which implements non-inheritable ONLY constraints. This has been achieved by introducing a new column conisonly in pg_constraint catalog. Specification of 'ONLY' in the ALTER TABLE ADD CONSTRAINT CHECK command is used to set this new column to true. Constraints which have this

Re: [HACKERS] Check constraints on partition parents only?

2011-07-29 Thread Nikhil Sontakke
Comments and further feedback, if any, appreciated. Did you look at how this conflicts with my patch to add not null rows to pg_constraint? https://commitfest.postgresql.org/action/patch_view?id=601 I was certainly not aware of this patch in the commitfest. Your patch has a larger

Re: [HACKERS] Check constraints on partition parents only?

2011-07-28 Thread Nikhil Sontakke
Now that we have coninhcnt, conislocal etc... we can probably support ONLY. But I agree with Robert it's probably a bit more than an afternoon to crank out :-) Heh, agreed :), I was just looking for some quick and early feedback. So what we need is basically a way to indicate that a

Re: [HACKERS] Check constraints on partition parents only?

2011-07-28 Thread Nikhil Sontakke
This approach certainly can't work, because a table can be both an inheritance parent and an inheritance child. It could have an ONLY constraint, and also inherit a copy of the same constraint for one or more parents. IOW, the fact that conislocal = true does not mean that coninhcount is

Re: [HACKERS] Check constraints on partition parents only?

2011-07-27 Thread Nikhil Sontakke
Hi, Yeah, but I think we need to take that chance. At the very least, we need to support the equivalent of a non-inherited CHECK (false) on parent tables. Indeed. I usually enforce that with a trigger that raises an exception, but of course that doesn't help at all with constraint

Re: [HACKERS] Check constraints on partition parents only?

2011-07-26 Thread Nikhil Sontakke
8.4 had this change: * Force child tables to inherit CHECK constraints from parents (Alex Hunsaker, Nikhil Sontakke, Tom) You're not the only one who occasionally bangs his head against it. Sorry for the occasional head bumps :) Yeah. I think it's

Re: [HACKERS] VACUUM FULL deadlock with backend startup

2011-03-22 Thread Nikhil Sontakke
Patch applied, thanks! Thanks Tom! Regards, Nikhils

Re: [HACKERS] VACUUM FULL deadlock with backend startup

2011-03-19 Thread Nikhil Sontakke
Hi, The fix is similar to the earlier commit by Tom. I tested this fix against 8.3.13. We lock the parent catalog now before calling index_open. Patch against git HEAD attached with this mail. I guess we will backpatch this? Tom's last commit was backpatched till 8.2 I think. Is it

[HACKERS] VACUUM FULL deadlock with backend startup

2011-03-18 Thread Nikhil Sontakke
Hi, We encountered a deadlock involving VACUUM FULL (surprise surprise! :)) in PG 8.3.13 (and still not fixed in 9.0 AFAICS although the window appears much smaller). The call spits out the following deadlock info: ERROR: SQLSTATE 40P01: deadlock detected DETAIL: Process 12479 waits for

[HACKERS] VACUUM FULL deadlock with backend startup

2011-03-18 Thread Nikhil Sontakke
Hi, We encountered a deadlock involving VACUUM FULL (surprise surprise! :)) in PG 8.3.13 (and still not fixed in 9.0 AFAICS although the window appears much smaller). The call spits out the following deadlock info: ERROR: SQLSTATE 40P01: deadlock detected DETAIL: Process 12479 waits for

Re: [HACKERS] Fwd: index corruption in PG 8.3.13

2011-03-16 Thread Nikhil Sontakke
Hi, To summarize, as I see it - the zeroed out block 523 should have been the second left-most leaf and should have pointed out to 522. Thus re-establishing the index chain 524 - 523 - 522 - 277 - ... Was there a machine restart in the picture as well? It seems there might have been a

Re: [HACKERS] Fwd: index corruption in PG 8.3.13

2011-03-16 Thread Nikhil Sontakke
Hi, Of course, as you mentioned earlier, it's not impossible there's a bug in the recovery code. Yeah, I was looking at the repair_frag function in 8.3.13 (yup it's ugly!) and found out that the normal ExecInsertIndexTuples call is used to insert the index entries. That is standard index code

Re: [HACKERS] Fwd: index corruption in PG 8.3.13

2011-03-14 Thread Nikhil Sontakke
Hi Daniel, I have also, coincidentally, encountered corruption of a system catalog index -- 8.3.11 -- I have saved the file for forensics.  Is it possible that I also receive a copy of this program? Will it be possible for you to share the file/logs off-list with me? I can also try to do

Re: [HACKERS] Fwd: index corruption in PG 8.3.13

2011-03-11 Thread Nikhil Sontakke
Hi, 1. Somebody inserts a bunch of new tuples into the relation, causing growth in the index. In case it's not obvious VACUUM FULL would do precisely that. Oh, I didn't even think about that.  Yeah, that could be it, too. Thanks a lot Greg and Robert. This theory seems very plausible. VF

Re: [HACKERS] Fwd: index corruption in PG 8.3.13

2011-03-11 Thread Nikhil Sontakke
VACUUM FULL - immediate shutdown - problem with recovery? An immediate shutdown == an intentional crash. OK, so you have the VACUUM FULL and the immediate shutdown just afterward. So we just need to figure out what happened during recovery. Right. But WAL replay should still have

Re: [HACKERS] Fwd: index corruption in PG 8.3.13

2011-03-11 Thread Nikhil Sontakke
Oh yeah, so if VF committed, the xlog should have been ok too, but can't say the same about the shared buffers. But there was a later block that *was* written out. What was the LSN on that block? everything in the WAL log should have been fsynced up to that point when that buffer was

  1   2   >