Thread: Re: [Postgres-xc-developers] Patch for bug#3013984: Sequence scope

Hi Benny,

I had a look at your patch,
and it is really a nice feature you implemented.

With this functionnality, it is possible to drop sequences on GTM in a very
smooth way when a database is dropped.
I have a couple of comments about it though.

1) in gtm_seq.c, function seq_start_with_nsp
You make a memory compare that will always return a true result because you
compare the same string.
       return ((memcmp(nsp->gsk_key, nsp->gsk_key, nsp->gsk_keylen) ==
0)
                       && (seq->gsk_key[nsp->gsk_keylen] == '.'));
I believe this is the right code:
       return ((memcmp(nsp->gsk_key, seq->gsk_key, nsp->gsk_keylen) ==
0)
                       && (seq->gsk_key[nsp->gsk_keylen] == '.'));

2) in gtm_seq.c, function GTM_RmSeqWithNsp
When you encounter a sequence that is used by another process, you mark it
for deletion and then you exit the process,
letting perhaps a lot of sequences not dropped.
I think priority should be given to deletion, as DROP DATABASE has to be
forced.
Btw, a busy sequence cannot basically be found because that would mean that
an instance is still using the database,
and in this case database drop is not possible.

3) in gtm_c.h, about the message type definition.
I made a modification in message type so as not to use a unique character.
Using an enumeration (typedef enum) makes it more consistent with
Postgres-xc code, and it makes code maintenance easier.

4) Also I notice a couple of typo problems, take care when you use spaces
and tabs!

I corrected those points above in the patch attached.
I also changed a couple of function names to make it clearer.

You should check it and make a couple of additional tests also.
Perhaps you have additional ideas about this implementation, don't hesitate
to share what you think.

Regards,

-- 
Michael Paquier
http://michaelpq.users.sourceforge.net

When I tried to test this, I noticed some problem. Maybe it exists 
without the patch though:

mds1=# create sequence myseq2;
CREATE SEQUENCE
mds1=# select nextval('myseq2');
  nextval
---------
        0
(1 row)

mds1=# select nextval('myseq2');
  nextval
---------
        0
(1 row)

Mason

On 11/12/10 1:23 PM, Michael Paquier wrote:
> Hi Benny,
>
> I had a look at your patch,
> and it is really a nice feature you implemented.
>
> With this functionnality, it is possible to drop sequences on GTM in a 
> very smooth way when a database is dropped.
> I have a couple of comments about it though.
>
> 1) in gtm_seq.c, function seq_start_with_nsp
> You make a memory compare that will always return a true result 
> because you compare the same string.
>        return ((memcmp(nsp->gsk_key, nsp->gsk_key, nsp->gsk_keylen) == 0)
> && (seq->gsk_key[nsp->gsk_keylen] == '.'));
> I believe this is the right code:
>        return ((memcmp(nsp->gsk_key, seq->gsk_key, nsp->gsk_keylen) == 0)
> && (seq->gsk_key[nsp->gsk_keylen] == '.'));
>
> 2) in gtm_seq.c, function GTM_RmSeqWithNsp
> When you encounter a sequence that is used by another process, you 
> mark it for deletion and then you exit the process,
> letting perhaps a lot of sequences not dropped.
> I think priority should be given to deletion, as DROP DATABASE has to 
> be forced.
> Btw, a busy sequence cannot basically be found because that would mean 
> that an instance is still using the database,
> and in this case database drop is not possible.
>
> 3) in gtm_c.h, about the message type definition.
> I made a modification in message type so as not to use a unique character.
> Using an enumeration (typedef enum) makes it more consistent with 
> Postgres-xc code, and it makes code maintenance easier.
>
> 4) Also I notice a couple of typo problems, take care when you use 
> spaces and tabs!
>
> I corrected those points above in the patch attached.
> I also changed a couple of function names to make it clearer.
>
> You should check it and make a couple of additional tests also.
> Perhaps you have additional ideas about this implementation, don't 
> hesitate to share what you think.
>
> Regards,
>
> -- 
> Michael Paquier
> http://michaelpq.users.sourceforge.net
>
>
> ------------------------------------------------------------------------------
> Centralized Desktop Delivery: Dell and VMware Reference Architecture
> Simplifying enterprise desktop deployment and management using
> Dell EqualLogic storage and VMware View: A highly scalable, end-to-end
> client virtualization framework. Read more!
> http://p.sf.net/sfu/dell-eql-dev2dev
>
>
> _______________________________________________
> Postgres-xc-developers mailing list
> Pos...@li...
> https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers
>    


-- 
Mason Sharp
EnterpriseDB Corporation
The Enterprise Postgres Company


This e-mail message (and any attachment) is intended for the use of
the individual or entity to whom it is addressed. This message
contains information from EnterpriseDB Corporation that may be
privileged, confidential, or exempt from disclosure under applicable
law. If you are not the intended recipient or authorized to receive
this for the intended recipient, any use, dissemination, distribution,
retention, archiving, or copying of this communication is strictly
prohibited. If you have received this e-mail in error, please notify
the sender immediately by reply e-mail and delete this message.

I created a couple of databases with a couple of sequences, bounced the 
coordinators and data nodes (not GTM), then if I try and drop a 
database, it causes a crash of both the coordinator and GTM.

mds1=# drop database msd2;
ERROR:  database "msd2" does not exist
mds1=# drop database mds2;
WARNING:  can not connect to GTM: Connection refused
ERROR:  Deletion of sequences on database mds2 not completed
server closed the connection unexpectedly
     This probably means the server terminated abnormally
     before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
!> \q

#0  0x907916fa in select$DARWIN_EXTSN ()
(gdb) bt
#0  0x907916fa in select$DARWIN_EXTSN ()
#1  0x00002dd1 in ServerLoop ()
#2  0x00002b7f in main ()

and

(gdb) bt
#0  0x003e54ed in gtmpqPutMsgStart ()
#1  0x003e89fd in abort_transaction ()
#2  0x00077272 in RollbackTranGTM (gxid=1142) at gtm.c:174
#3  0x000583f5 in AbortTransaction () at xact.c:2320
#4  0x00058e33 in AbortCurrentTransaction () at xact.c:2783
#5  0x00058f18 in AbortCurrentTransactionOnce () at xact.c:2911
#6  0x0029aa16 in PostgresMain (argc=4, argv=0x1002ff8, 
username=0x1002fc8 "masonsharp") at postgres.c:3622
#7  0x0025665c in BackendRun (port=0x7016f0) at postmaster.c:3607
#8  0x002559c3 in BackendStartup (port=0x7016f0) at postmaster.c:3216
#9  0x002523f5 in ServerLoop () at postmaster.c:1445
#10 0x00251a01 in PostmasterMain (argc=5, argv=0x7005a0) at 
postmaster.c:1098
#11 0x001cd3a5 in main (argc=5, argv=0x7005a0) at main.c:

We should probably check if the connection is NULL.

Thanks,

Mason

-- 
Mason Sharp
EnterpriseDB Corporation
The Enterprise Postgres Company


This e-mail message (and any attachment) is intended for the use of
the individual or entity to whom it is addressed. This message
contains information from EnterpriseDB Corporation that may be
privileged, confidential, or exempt from disclosure under applicable
law. If you are not the intended recipient or authorized to receive
this for the intended recipient, any use, dissemination, distribution,
retention, archiving, or copying of this communication is strictly
prohibited. If you have received this e-mail in error, please notify
the sender immediately by reply e-mail and delete this message.

This is going to be a pretty long message, but...
I made a couple of tests with this patch.

Successively I created two databases, sequences on it, then dropped the
databases.
I created again and a couple of times databases (and so sequences) and
didn't notice any error.
What are the SQL you used for your tests? Is it reproducible?

-- create databases
template1=# create database foo1;
CREATE DATABASE
template1=# create database foo2;
CREATE DATABASE
template1=# \c foo1
psql (8.4.5, サバ 8.4.3)
デタベス "foo1" に接しました。.
foo1=# create sequence foo;
CREATE SEQUENCE
foo1=# create sequence foo2;
CREATE SEQUENCE
foo1=# \c foo2
psql (8.4.5, サバ 8.4.3)
デタベス "foo2" に接しました。.
foo2=# create sequence foo;
CREATE SEQUENCE
foo2=# create sequence foo2;
CREATE SEQUENCE
foo2=# select nextval('foo');
 nextval
---------
       1
(1 行)

foo2=# select nextval('foo');
 nextval
---------
       2
(1 行)

foo2=# \c template1
psql (8.4.5, サバ 8.4.3)
デタベス "template1" に接しました。.
template1=# drop database foo1;
DROP DATABASE
template1=# clean connection to all for database foo2;
CLEAN CONNECTION
template1=# drop database foo2;
DROP DATABASE
template1=# create database foo2;
CREATE DATABASE
template1=# \c foo2
psql (8.4.5, サバ 8.4.3)
デタベス "foo2" に接しました。.
foo2=# create sequence foo;
CREATE SEQUENCE
foo2=# create sequence foo2;
CREATE SEQUENCE
foo2=# select nextval('foo');
 nextval
---------
       1
(1 行)

foo2=# select nextval('foo');
 nextval
---------
       2
(1 行)

foo2=# select nextval('foo2');
 nextval
---------
       1
(1 行)

foo2=# select nextval('foo2');
 nextval
---------
       2
(1 行)

foo2=# \c template1
psql (8.4.5, サバ 8.4.3)
デタベス "template1" に接しました。.
template1=# create database foo1;
CREATE DATABASE
template1=# \c foo1
psql (8.4.5, サバ 8.4.3)
デタベス "foo1" に接しました。.
foo1=# create sequence foo;
CREATE SEQUENCE
foo1=# create sequence foo2;
CREATE SEQUENCE
foo1=# select nextval('foo');
 nextval
---------
       1
(1 行)

foo1=# select nextval('foo');
 nextval
---------
       2
(1 行)

foo1=# select nextval('foo2');
 nextval
---------
       1
(1 行)
foo1=# \c template1
psql (8.4.5, サバ 8.4.3)
デタベス "template1" に接しました。.
template1=# clean connection to all for database foo1;
CLEAN CONNECTION
template1=# clean connection to all for database foo2;
CLEAN CONNECTION
template1=# drop database foo1;
DROP DATABASE
template1=# drop database foo2;
DROP DATABASE
template1=# create database foo1;
CREATE DATABASE
template1=# create database foo2;
CREATE DATABASE
template1=# \c foo1
psql (8.4.5, サバ 8.4.3)
デタベス "foo1" に接しました。.
foo1=# create sequence foo1;
CREATE SEQUENCE
foo1=# create sequence foo2;
CREATE SEQUENCE
foo1=# create sequence foo;
CREATE SEQUENCE
foo1=# select nextval('foo1');
 nextval
---------
       1
(1 行)

foo1=# alter sequence foo2 increment 2;
ALTER SEQUENCE
foo1=# select nextval('foo2');
 nextval
---------
       1
(1 行)

foo1=# select nextval('foo2');
 nextval
---------
       3
(1 行)

foo1=# select nextval('foo2');
 nextval
---------
       5
(1 行)

-- 
Michael Paquier
http://michaelpq.users.sourceforge.net

On 11/18/10 9:44 AM, Michael Paquier wrote:
> This is going to be a pretty long message, but...
> I made a couple of tests with this patch.
>
> Successively I created two databases, sequences on it, then dropped
> the databases.
> I created again and a couple of times databases (and so sequences) and
> didn't notice any error.
> What are the SQL you used for your tests? Is it reproducible?
>
Yes, it is still happening. My sequence of steps was a bit different:

$ inst/bin/createdb -p 5445 mds1
$ inst/bin/createdb -p 5445 mds2
$ inst/bin/psql -p 5445 mds1
psql (8.4.3)
Type "help" for help.

mds1=# create sequence foo;
CREATE SEQUENCE
mds1=# create sequence foo2;
CREATE SEQUENCE
mds1=# \c mds2
psql (8.4.3)
You are now connected to database "mds2".
mds2=# create sequence foo;
CREATE SEQUENCE
mds2=# create sequence foo2;
CREATE SEQUENCE
mds2=# select nextval('foo');
nextval
---------
1
(1 row)

mds2=# select nextval('foo');
nextval
---------
2
(1 row)

mds2=# select nextval('foo2');
nextval
---------
1
(1 row)

mds2=# select nextval('foo2');
nextval
---------
2
(1 row)

mds2=# \q

72:postgres-xc masonsharp$ stopnodes.sh
waiting for server to shut down.... done
server stopped
waiting for server to shut down.... done
server stopped
waiting for server to shut down.... done
server stopped
waiting for server to shut down.... done
server stopped

(this stops the coordinators and datanodes, but not GTM)

72:postgres-xc masonsharp$ startnodes.sh
LOG: database system was interrupted; last known up at 2010-11-18
11:46:52 JST
LOG: database system was not properly shut down; automatic recovery in
progress
LOG: redo starts at 0/4BCF34
LOG: record with zero length at 0/4BCFE4
LOG: redo done at 0/4BCFB8
LOG: last completed transaction was at log time 2010-11-18 11:48:52.44638+09
LOG: autovacuum launcher started
LOG: database system is ready to accept connections
LOG: database system was interrupted; last known up at 2010-11-18
11:46:52 JST
LOG: database system was not properly shut down; automatic recovery in
progress
LOG: redo starts at 0/4BCF34
LOG: record with zero length at 0/4BCFE4
LOG: redo done at 0/4BCFB8
LOG: last completed transaction was at log time 2010-11-18 11:48:52.43121+09
LOG: autovacuum launcher started
LOG: database system is ready to accept connections
14151: 0: LOG: database system was shut down at 2010-11-18 11:49:16 JST
14159: 0: LOG: autovacuum launcher started
14142: 0: LOG: database system is ready to accept connections
14160: 0: LOG: database system was shut down at 2010-11-18 11:49:17 JST
14143: 0: LOG: database system is ready to accept connections
14165: 0: LOG: autovacuum launcher started
72:postgres-xc masonsharp$ inst/bin/psql -p 5445 mds1
psql (8.4.3)
Type "help" for help.

mds1=# drop database mds2;
WARNING: can not connect to GTM: Connection refused
ERROR: Deletion of sequences on database mds2 not completed
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
!>

Regards,

Mason


> -- create databases
> template1=# create database foo1;
> CREATE DATABASE
> template1=# create database foo2;
> CREATE DATABASE
> template1=# \c foo1
> psql (8.4.5, サバ 8.4.3)
> デタベス "foo1" に接しました。.
> foo1=# create sequence foo;
> CREATE SEQUENCE
> foo1=# create sequence foo2;
> CREATE SEQUENCE
> foo1=# \c foo2
> psql (8.4.5, サバ 8.4.3)
> デタベス "foo2" に接しました。.
> foo2=# create sequence foo;
> CREATE SEQUENCE
> foo2=# create sequence foo2;
> CREATE SEQUENCE
> foo2=# select nextval('foo');
> nextval
> ---------
> 1
> (1 行)
>
> foo2=# select nextval('foo');
> nextval
> ---------
> 2
> (1 行)
>
> foo2=# \c template1
> psql (8.4.5, サバ 8.4.3)
> デタベス "template1" に接しました。.
> template1=# drop database foo1;
> DROP DATABASE
> template1=# clean connection to all for database foo2;
> CLEAN CONNECTION
> template1=# drop database foo2;
> DROP DATABASE
> template1=# create database foo2;
> CREATE DATABASE
> template1=# \c foo2
> psql (8.4.5, サバ 8.4.3)
> デタベス "foo2" に接しました。.
> foo2=# create sequence foo;
> CREATE SEQUENCE
> foo2=# create sequence foo2;
> CREATE SEQUENCE
> foo2=# select nextval('foo');
> nextval
> ---------
> 1
> (1 行)
>
> foo2=# select nextval('foo');
> nextval
> ---------
> 2
> (1 行)
>
> foo2=# select nextval('foo2');
> nextval
> ---------
> 1
> (1 行)
>
> foo2=# select nextval('foo2');
> nextval
> ---------
> 2
> (1 行)
>
> foo2=# \c template1
> psql (8.4.5, サバ 8.4.3)
> デタベス "template1" に接しました。.
> template1=# create database foo1;
> CREATE DATABASE
> template1=# \c foo1
> psql (8.4.5, サバ 8.4.3)
> デタベス "foo1" に接しました。.
> foo1=# create sequence foo;
> CREATE SEQUENCE
> foo1=# create sequence foo2;
> CREATE SEQUENCE
> foo1=# select nextval('foo');
> nextval
> ---------
> 1
> (1 行)
>
> foo1=# select nextval('foo');
> nextval
> ---------
> 2
> (1 行)
>
> foo1=# select nextval('foo2');
> nextval
> ---------
> 1
> (1 行)
> foo1=# \c template1
> psql (8.4.5, サバ 8.4.3)
> デタベス "template1" に接しました。.
> template1=# clean connection to all for database foo1;
> CLEAN CONNECTION
> template1=# clean connection to all for database foo2;
> CLEAN CONNECTION
> template1=# drop database foo1;
> DROP DATABASE
> template1=# drop database foo2;
> DROP DATABASE
> template1=# create database foo1;
> CREATE DATABASE
> template1=# create database foo2;
> CREATE DATABASE
> template1=# \c foo1
> psql (8.4.5, サバ 8.4.3)
> デタベス "foo1" に接しました。.
> foo1=# create sequence foo1;
> CREATE SEQUENCE
> foo1=# create sequence foo2;
> CREATE SEQUENCE
> foo1=# create sequence foo;
> CREATE SEQUENCE
> foo1=# select nextval('foo1');
> nextval
> ---------
> 1
> (1 行)
>
> foo1=# alter sequence foo2 increment 2;
> ALTER SEQUENCE
> foo1=# select nextval('foo2');
> nextval
> ---------
> 1
> (1 行)
>
> foo1=# select nextval('foo2');
> nextval
> ---------
> 3
> (1 行)
>
> foo1=# select nextval('foo2');
> nextval
> ---------
> 5
> (1 行)
>
> -- 
> Michael Paquier
> http://michaelpq.users.sourceforge.net
>


-- 
Mason Sharp
EnterpriseDB Corporation
The Enterprise Postgres Company


This e-mail message (and any attachment) is intended for the use of
the individual or entity to whom it is addressed. This message
contains information from EnterpriseDB Corporation that may be
privileged, confidential, or exempt from disclosure under applicable
law. If you are not the intended recipient or authorized to receive
this for the intended recipient, any use, dissemination, distribution,
retention, archiving, or copying of this communication is strictly
prohibited. If you have received this e-mail in error, please notify
the sender immediately by reply e-mail and delete this message.

Hi,

I made the same test as you and didn't notice any errors when dropping the
second database.

By Having a look at the logs of your nodes when you stopped them, it looks
that it entered in recovery mode.
So I assume you stopped it in "fast" mode with pg_ctl or with SIGQUIT.
In this case database shuts down without waiting for the client to
disconnect.

My cluster structure is 2 Coordinators and 2 datanodes, with 2 proxies.
After doing the same SQL, I stopped my nodes in the most violent way.
GTM and proxies were not shut down.

pgsql]$ ./bin/pg_ctl stop -m fast -D ~/pgsql/coord1/
waiting for server to shut down.... done
server stopped
pgsql]$ ./bin/pg_ctl stop -m fast -D ~/pgsql/coord2/
waiting for server to shut down.... done
server stopped
pgsql]$ ./bin/pg_ctl stop -m fast -D ~/pgsql/datanode2/
waiting for server to shut down.... done
server stopped
pgsql]$ ./bin/pg_ctl stop -m fast -D ~/pgsql/datanode1/
waiting for server to shut down.... done
server stopped

Then I restarted the nodes, and dropped a database. It worked.

I did also a second test.
I stopped my nodes, then the proxies (GTM not touched):
pgsql]$ ./bin/gtm_ctl stop -m fast -S gtm_proxy -D
/home/michael/pgsql/proxy2
waiting for server to shut down.... done
server stopped
pgsql]$ ./bin/gtm_ctl stop -m fast -S gtm_proxy -D
/home/michael/pgsql/proxy1
waiting for server to shut down... done
server stopped
I restart the whole cluster, then drop again a dabatase.
It worked once again.

I made a couple of times the same tests and it looked to work all the time.
The logs on each node also looked correct.

Regards,

-- 
Michael Paquier
http://michaelpq.users.sourceforge.net

On 11/21/10 9:13 PM, Michael Paquier wrote:
>
> I made a couple of times the same tests and it looked to work all the 
> time.
> The logs on each node also looked correct.
>
I just committed, after doing a bug fix. The problem appeared to be that 
the list was getting
reassigned when an item is deleted in the foreach loop that depended on it.
I changed that to be more careful.

You guys probably just got lucky in your environment, but without the 
change it would
always crash under MacOS.

I also made a small change to check if the connection to GTM is NULL 
before rolling back.
(The GTM crash was causing a coordinator crash.)

Thanks,

Mason

> Regards,
>
> -- 
> Michael Paquier
> http://michaelpq.users.sourceforge.net
>


-- 
Mason Sharp
EnterpriseDB Corporation
The Enterprise Postgres Company


This e-mail message (and any attachment) is intended for the use of
the individual or entity to whom it is addressed. This message
contains information from EnterpriseDB Corporation that may be
privileged, confidential, or exempt from disclosure under applicable
law. If you are not the intended recipient or authorized to receive
this for the intended recipient, any use, dissemination, distribution,
retention, archiving, or copying of this communication is strictly
prohibited. If you have received this e-mail in error, please notify
the sender immediately by reply e-mail and delete this message.