From: Ashutosh B. <ash...@en...> - 2013-06-25 08:55:50
|
HI Abbas, The testcases are ok now, but please annotate them saying replication tests, round robin tests etc. We don't need both n1 and n2 tests. Either of them would be fine. You may commit the patch, after taking care of these comments. On Mon, Jun 24, 2013 at 4:31 PM, Abbas Butt <abb...@en...>wrote: > Hi, > Thanks for the review. > Here is the updated patch that contains reduced test cases. > Also I have updated code comments. > The statement failing is already in the comments. > Regards > > > > On Mon, Jun 17, 2013 at 12:36 PM, Ashutosh Bapat < > ash...@en...> wrote: > >> Hi Abbas, >> I think the patch for this is in the other thread (11_fix ..). I looked >> at the patch. Here are the comments >> 1. There are just too many tests in the patch, without much difference. >> Please add only the tests which are needed, and also add comments about the >> purpose of the statements. Considering the time at hand, I don't think I >> can review all of the tests, so it would be good if it can be reduced to a >> minimal set. >> 2. The code is fine, but the comment need not have specific details of >> the statement failing. Getting preferred node is general practice >> everywhere and not just this portion of the code. By the way, we are not >> getting "just first node" from the node list but we try to get the >> preferred node. >> >> >> On Wed, Mar 27, 2013 at 3:55 PM, Abbas Butt <abb...@en...>wrote: >> >>> Bug ID 3608374 >>> >>> On Fri, Mar 8, 2013 at 12:25 PM, Abbas Butt <abb...@en... >>> > wrote: >>> >>>> Attached please find revised patch that provides the following in >>>> addition to what it did earlier. >>>> >>>> 1. Uses GetPreferredReplicationNode() instead of list_truncate() >>>> 2. Adds test cases to xc_alter_table and xc_copy. >>>> >>>> I tested the following in reasonable detail to find whether any other >>>> caller of GetRelationNodes() needs some fixing or not and found that none >>>> of the other callers needs any more fixing. >>>> I tested >>>> a) copy >>>> b) alter table redistribute >>>> c) utilities >>>> d) dmls etc >>>> >>>> However while testing ALTER TABLE, I found that replicated to hash is >>>> not working correctly. >>>> >>>> This test case fails, since only SIX rows are expected in the final >>>> result. >>>> >>>> test=# create table t_r_n12(a int, b int) distribute by replication to >>>> node (DATA_NODE_1, DATA_NODE_2); >>>> CREATE TABLE >>>> test=# insert into t_r_n12 >>>> values(1,777),(3,4),(5,6),(20,30),(NULL,999), (NULL, 999); >>>> INSERT 0 6 >>>> test=# -- rep to hash >>>> test=# ALTER TABLE t_r_n12 distribute by hash(a); >>>> ALTER TABLE >>>> test=# SELECT * FROM t_r_n12 order by 1; >>>> a | b >>>> ----+----- >>>> 1 | 777 >>>> 3 | 4 >>>> 5 | 6 >>>> 20 | 30 >>>> | 999 >>>> | 999 >>>> | 999 >>>> | 999 >>>> (8 rows) >>>> >>>> test=# drop table t_r_n12; >>>> DROP TABLE >>>> >>>> I have added a source forge bug tracker id to this case (Artifact >>>> 3607290<https://sourceforge.net/tracker/?func=detail&aid=3607290&group_id=311227&atid=1310232>). >>>> The reason for this error is that the function distrib_delete_hash does not >>>> take into account that the distribution column can be null. I will provide >>>> a separate fix for that one. >>>> Regression shows no extra failure except that test case xc_alter_table >>>> would fail until 3607290 is fixed. >>>> >>>> Regards >>>> >>>> >>>> >>>> On Mon, Feb 25, 2013 at 10:18 AM, Ashutosh Bapat < >>>> ash...@en...> wrote: >>>> >>>>> Thanks a lot Abbas for this quick fix. >>>>> >>>>> I am sorry, it's caused by my refactoring of GetRelationNodes(). >>>>> >>>>> If possible, can you please examine the other callers of >>>>> GetRelationNodes() which would face the problems, esp. the ones for DML and >>>>> utilities. This is other instance, where deciding the nodes to execute on >>>>> at the time of execution will help. >>>>> >>>>> About the fix >>>>> Can you please use GetPreferredReplicationNode() instead of >>>>> list_truncate()? It will pick the preferred node instead of first one. If >>>>> you find more places where we need this fix, it might be better to create a >>>>> wrapper function and use it at those places. >>>>> >>>>> On Sat, Feb 23, 2013 at 2:59 PM, Abbas Butt < >>>>> abb...@en...> wrote: >>>>> >>>>>> Hi, >>>>>> PFA a patch to fix a crash when COPY TO is used on a replicated table. >>>>>> >>>>>> This test case produces a crash >>>>>> >>>>>> create table tab_rep(a int, b int) distribute by replication; >>>>>> insert into tab_rep values(1,2), (3,4), (5,6), (7,8); >>>>>> COPY tab_rep (a, b) TO stdout; >>>>>> >>>>>> Here is a description of the problem and the fix >>>>>> In case of a read from a replicated table GetRelationNodes() >>>>>> returns all nodes and expects that the planner can choose >>>>>> one depending on the rest of the join tree. >>>>>> In case of COPY TO we should choose the first one in the node list >>>>>> This fixes a system crash and makes pg_dump work fine. >>>>>> >>>>>> -- >>>>>> Abbas >>>>>> Architect >>>>>> EnterpriseDB Corporation >>>>>> The Enterprise PostgreSQL Company >>>>>> >>>>>> Phone: 92-334-5100153 >>>>>> >>>>>> Website: www.enterprisedb.com >>>>>> EnterpriseDB Blog: http://blogs.enterprisedb.com/ >>>>>> Follow us on Twitter: http://www.twitter.com/enterprisedb >>>>>> >>>>>> 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. >>>>>> >>>>>> ------------------------------------------------------------------------------ >>>>>> Everyone hates slow websites. So do we. >>>>>> Make your web apps faster with AppDynamics >>>>>> Download AppDynamics Lite for free today: >>>>>> http://p.sf.net/sfu/appdyn_d2d_feb >>>>>> _______________________________________________ >>>>>> Postgres-xc-developers mailing list >>>>>> Pos...@li... >>>>>> https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers >>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> Best Wishes, >>>>> Ashutosh Bapat >>>>> EntepriseDB Corporation >>>>> The Enterprise Postgres Company >>>>> >>>> >>>> >>>> >>>> -- >>>> -- >>>> Abbas >>>> Architect >>>> EnterpriseDB Corporation >>>> The Enterprise PostgreSQL Company >>>> >>>> Phone: 92-334-5100153 >>>> >>>> Website: www.enterprisedb.com >>>> EnterpriseDB Blog: http://blogs.enterprisedb.com/ >>>> Follow us on Twitter: http://www.twitter.com/enterprisedb >>>> >>>> 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. >>>> >>> >>> >>> >>> -- >>> -- >>> Abbas >>> Architect >>> EnterpriseDB Corporation >>> The Enterprise PostgreSQL Company >>> >>> Phone: 92-334-5100153 >>> >>> Website: www.enterprisedb.com >>> EnterpriseDB Blog: http://blogs.enterprisedb.com/ >>> Follow us on Twitter: http://www.twitter.com/enterprisedb >>> >>> 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. >> >> >> >> >> -- >> Best Wishes, >> Ashutosh Bapat >> EntepriseDB Corporation >> The Postgres Database Company >> > > > > -- > -- > *Abbas* > Architect > > Ph: 92.334.5100153 > Skype ID: gabbasb > www.enterprisedb.co <http://www.enterprisedb.com/>m<http://www.enterprisedb.com/> > * > Follow us on Twitter* > @EnterpriseDB > > Visit EnterpriseDB for tutorials, webinars, whitepapers<http://www.enterprisedb.com/resources-community>and more<http://www.enterprisedb.com/resources-community> > -- Best Wishes, Ashutosh Bapat EntepriseDB Corporation The Postgres Database Company |