Menu
▾
▴
postgres-xc-developers
|
From: Pavan D. <pav...@gm...> - 2014-03-05 08:39:32
|
Hi All,
My apologies if I am raising a point which is well settled in other
discussion. I know this topic has been discussed at length, but I haven't
followed that discussion well. But the this statement in the commit log
still worries me:
" Otherwise ctid is used, like we were using previously."
This can never be safe for replicated tables. So why don't we just throw an
error if no primary key or unique index is defined ? Updating tables based
on ctid would definitely lead to data corruption. So why do that ?
Thanks,
Pavan
---------- Forwarded message ----------
From: <ap...@us...>
Date: Wed, Mar 5, 2014 at 1:51 PM
Subject: [Postgres-xc-committers] Postgres-XC branch, REL1_2_STABLE,
updated. XC1_0_BETA1_PG9_1-3999-ge5466f7
To: pos...@li...
Project "Postgres-XC".
The branch, REL1_2_STABLE has been updated
via e5466f780b3da3e44c5b65e245417db1af822af2 (commit)
from 45f8541d30cfd9f7385cd83c43bcdca813b7f83a (commit)
- Log -----------------------------------------------------------------
http://postgres-xc.git.sourceforge.net/git/gitweb.cgi?p=postgres-xc/postgres-xc;a=commitdiff;h=e5466f780b3da3e44c5b65e245417db1af822af2
commit 22fdd3506d5b3dd6a899b0e2134eb813b424c5db
Author: Koichi Suzuki <koi...@gm...>
Date: Wed Mar 5 17:18:44 2014 +0900
A patch to improve replicated table update/delete handling.
This commit performs updates/deletes to replicated tables
based on either primary key or unique index under the following
conditions
1. The replicated table has either a primary key or a unique index
defined.
2. The query is not changing the primary key itself.
Otherwise ctid is used, like we were using previously.
--
Pavan Deolasee
http://www.linkedin.com/in/pavandeolasee
|
|
From: Koichi S. <koi...@gm...> - 2014-03-05 09:02:35
|
I'd like Abbas to answer why it is practical at this point ... >From the discussion of the thread, I understand that it is highly recommended to have a primary key in replicated tables. This will be in the release note and the reference document. Original patch from Mason requires a primary key, which causes some of regression test to fail and the short-term fix will make regression very dirty. In a long run, as discussed, I believe system primary key will be a solution, if no primary key is defined as user column. Regards; --- Koichi Suzuki 2014-03-05 17:39 GMT+09:00 Pavan Deolasee <pav...@gm...>: > Hi All, > > My apologies if I am raising a point which is well settled in other > discussion. I know this topic has been discussed at length, but I haven't > followed that discussion well. But the this statement in the commit log > still worries me: > > " Otherwise ctid is used, like we were using previously." > > This can never be safe for replicated tables. So why don't we just throw an > error if no primary key or unique index is defined ? Updating tables based > on ctid would definitely lead to data corruption. So why do that ? > > Thanks, > Pavan > > > ---------- Forwarded message ---------- > From: <ap...@us...> > Date: Wed, Mar 5, 2014 at 1:51 PM > Subject: [Postgres-xc-committers] Postgres-XC branch, REL1_2_STABLE, > updated. XC1_0_BETA1_PG9_1-3999-ge5466f7 > To: pos...@li... > > > Project "Postgres-XC". > > The branch, REL1_2_STABLE has been updated > via e5466f780b3da3e44c5b65e245417db1af822af2 (commit) > from 45f8541d30cfd9f7385cd83c43bcdca813b7f83a (commit) > > > - Log ----------------------------------------------------------------- > http://postgres-xc.git.sourceforge.net/git/gitweb.cgi?p=postgres-xc/postgres-xc;a=commitdiff;h=e5466f780b3da3e44c5b65e245417db1af822af2 > > commit 22fdd3506d5b3dd6a899b0e2134eb813b424c5db > Author: Koichi Suzuki <koi...@gm...> > Date: Wed Mar 5 17:18:44 2014 +0900 > > A patch to improve replicated table update/delete handling. > > This commit performs updates/deletes to replicated tables > based on either primary key or unique index under the following > conditions > > 1. The replicated table has either a primary key or a unique index > defined. > 2. The query is not changing the primary key itself. > > Otherwise ctid is used, like we were using previously. > > > > > -- > Pavan Deolasee > http://www.linkedin.com/in/pavandeolasee > > ------------------------------------------------------------------------------ > Subversion Kills Productivity. Get off Subversion & Make the Move to > Perforce. > With Perforce, you get hassle-free workflows. Merge that actually works. > Faster operations. Version large binaries. Built-in WAN optimization and > the > freedom to use Git, Perforce or both. Make the move to Perforce. > http://pubads.g.doubleclick.net/gampad/clk?id=122218951&iu=/4140/ostg.clktrk > _______________________________________________ > Postgres-xc-developers mailing list > Pos...@li... > https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers > |
|
From: Pavan D. <pav...@gm...> - 2014-03-05 17:19:24
|
On Wed, Mar 5, 2014 at 2:32 PM, Koichi Suzuki <koi...@gm...> wrote: > I'd like Abbas to answer why it is practical at this point ... > > Sure. I would wait for him to explain. > From the discussion of the thread, I understand that it is highly > recommended to have a primary key in replicated tables. This will be > in the release note and the reference document. Original patch from > Mason requires a primary key, which causes some of regression test to > fail and the short-term fix will make regression very dirty. > > I am sorry, but I think data inconsistency problem should take higher priority that passing the regression tests. The regression tests are there to find bugs, not hide them under the carpet. Even if its must for some reason to temporarily circumvent the problem for the regression tests, a possible way would be to test with a patched server. But I would strongly argue against releasing anything which has a known and a very common data corruption bug. > In a long run, as discussed, I believe system primary key will be a > solution, if no primary key is defined as user column. > > Yeah, that would work. It should not be very complicated too to implement. Thanks, Pavan -- Pavan Deolasee http://www.linkedin.com/in/pavandeolasee |
|
From: Koichi S. <koi...@gm...> - 2014-03-06 00:52:46
|
What we may be able to do for 1.2 is to add new GUC, say, require_replicated_table_pkey and set default to ON. In regression, we set it OFF as we used to do for TEMP object with enforce_two_phase_commit. This will be used when we support system pkey. Abbas, any input on this? --- Koichi Suzuki 2014-03-06 2:18 GMT+09:00 Pavan Deolasee <pav...@gm...>: > On Wed, Mar 5, 2014 at 2:32 PM, Koichi Suzuki <koi...@gm...> wrote: >> >> I'd like Abbas to answer why it is practical at this point ... >> > > Sure. I would wait for him to explain. > >> >> From the discussion of the thread, I understand that it is highly >> recommended to have a primary key in replicated tables. This will be >> in the release note and the reference document. Original patch from >> Mason requires a primary key, which causes some of regression test to >> fail and the short-term fix will make regression very dirty. >> > > I am sorry, but I think data inconsistency problem should take higher > priority that passing the regression tests. The regression tests are there > to find bugs, not hide them under the carpet. Even if its must for some > reason to temporarily circumvent the problem for the regression tests, a > possible way would be to test with a patched server. But I would strongly > argue against releasing anything which has a known and a very common data > corruption bug. > >> >> In a long run, as discussed, I believe system primary key will be a >> solution, if no primary key is defined as user column. >> > > Yeah, that would work. It should not be very complicated too to implement. > > Thanks, > Pavan > > -- > Pavan Deolasee > http://www.linkedin.com/in/pavandeolasee |
|
From: Abbas B. <abb...@en...> - 2014-03-06 02:30:25
|
On Thu, Mar 6, 2014 at 5:52 AM, Koichi Suzuki <koi...@gm...> wrote: > What we may be able to do for 1.2 is to add new GUC, say, > require_replicated_table_pkey and set default to ON. In regression, > we set it OFF as we used to do for TEMP object with > enforce_two_phase_commit. This will be used when we support system > pkey. > > Abbas, any input on this? > I agree with the GUC suggestion. > > --- > Koichi Suzuki > > > 2014-03-06 2:18 GMT+09:00 Pavan Deolasee <pav...@gm...>: > > On Wed, Mar 5, 2014 at 2:32 PM, Koichi Suzuki <koi...@gm...> > wrote: > >> > >> I'd like Abbas to answer why it is practical at this point ... > >> > > > > Sure. I would wait for him to explain. > > > >> > >> From the discussion of the thread, I understand that it is highly > >> recommended to have a primary key in replicated tables. This will be > >> in the release note and the reference document. Original patch from > >> Mason requires a primary key, which causes some of regression test to > >> fail and the short-term fix will make regression very dirty. > >> > > > > I am sorry, but I think data inconsistency problem should take higher > > priority that passing the regression tests. The regression tests are > there > > to find bugs, not hide them under the carpet. Even if its must for some > > reason to temporarily circumvent the problem for the regression tests, a > > possible way would be to test with a patched server. But I would strongly > > argue against releasing anything which has a known and a very common data > > corruption bug. > > > >> > >> In a long run, as discussed, I believe system primary key will be a > >> solution, if no primary key is defined as user column. > >> > > > > Yeah, that would work. It should not be very complicated too to > implement. > > > > Thanks, > > Pavan > > > > -- > > Pavan Deolasee > > http://www.linkedin.com/in/pavandeolasee > > > ------------------------------------------------------------------------------ > Subversion Kills Productivity. Get off Subversion & Make the Move to > Perforce. > With Perforce, you get hassle-free workflows. Merge that actually works. > Faster operations. Version large binaries. Built-in WAN optimization and > the > freedom to use Git, Perforce or both. Make the move to Perforce. > > http://pubads.g.doubleclick.net/gampad/clk?id=122218951&iu=/4140/ostg.clktrk > _______________________________________________ > Postgres-xc-developers mailing list > Pos...@li... > https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers > -- -- *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> |
|
From: Abbas B. <abb...@en...> - 2014-03-06 02:27:42
|
On Wed, Mar 5, 2014 at 10:18 PM, Pavan Deolasee <pav...@gm...>wrote: > On Wed, Mar 5, 2014 at 2:32 PM, Koichi Suzuki <koi...@gm...>wrote: > >> I'd like Abbas to answer why it is practical at this point ... >> >> > Sure. I would wait for him to explain. > It was done as a quick fix for backward compatibility, later we may use cursors or system generated primary key. We tried using cursors but that work could not be completed in the allotted time. It was also decided that since updates to replicated tables without unique keys can cause issues we will add detailed information in docs/wiki/release notes. > >> >From the discussion of the thread, I understand that it is highly >> recommended to have a primary key in replicated tables. This will be >> in the release note and the reference document. Original patch from >> Mason requires a primary key, which causes some of regression test to >> fail and the short-term fix will make regression very dirty. >> >> > I am sorry, but I think data inconsistency problem should take higher > priority that passing the regression tests. The regression tests are there > to find bugs, not hide them under the carpet. Even if its must for some > reason to temporarily circumvent the problem for the regression tests, a > possible way would be to test with a patched server. But I would strongly > argue against releasing anything which has a known and a very common data > corruption bug. > > >> In a long run, as discussed, I believe system primary key will be a >> solution, if no primary key is defined as user column. >> >> > Yeah, that would work. It should not be very complicated too to implement. > > > Thanks, > Pavan > > -- > Pavan Deolasee > http://www.linkedin.com/in/pavandeolasee > > > ------------------------------------------------------------------------------ > Subversion Kills Productivity. Get off Subversion & Make the Move to > Perforce. > With Perforce, you get hassle-free workflows. Merge that actually works. > Faster operations. Version large binaries. Built-in WAN optimization and > the > freedom to use Git, Perforce or both. Make the move to Perforce. > > http://pubads.g.doubleclick.net/gampad/clk?id=122218951&iu=/4140/ostg.clktrk > _______________________________________________ > Postgres-xc-developers mailing list > Pos...@li... > https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers > > -- -- *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> |
|
From: Pavan D. <pav...@gm...> - 2014-03-06 06:01:47
|
> On 06-Mar-2014, at 7:57 am, Abbas Butt <abb...@en...> wrote: > > > > >> On Wed, Mar 5, 2014 at 10:18 PM, Pavan Deolasee <pav...@gm...> wrote: >>> On Wed, Mar 5, 2014 at 2:32 PM, Koichi Suzuki <koi...@gm...> wrote: >>> I'd like Abbas to answer why it is practical at this point ... >> >> Sure. I would wait for him to explain. > > It was done as a quick fix for backward compatibility, IMHO it's a bug that needs to be fixed and not a compatibility that needs to be backward supported. Let's just throw an error for now until we have time and resources to fix the bug. Thanks, Pavan |
|
From: Abbas B. <abb...@en...> - 2014-03-08 14:22:39
Attachments:
2_guc.patch
|
Hi, Attached please find patch to add a GUC called require_replicated_table_pkey which is true by default. The purpose of the GUC is to error out for non-FQS UPDATE / DELETE to a replicated table. By setting the GUC to false the error can be by passed and this is done to run regression tests. make check runs fine after the patch. Test cases are added to check the added functionality. Best Regards On Thu, Mar 6, 2014 at 9:00 AM, Pavan Deolasee <pav...@gm...>wrote: > > > On 06-Mar-2014, at 7:57 am, Abbas Butt <abb...@en...> > wrote: > > > > > On Wed, Mar 5, 2014 at 10:18 PM, Pavan Deolasee <pav...@gm...>wrote: > >> On Wed, Mar 5, 2014 at 2:32 PM, Koichi Suzuki <koi...@gm...>wrote: >> >>> I'd like Abbas to answer why it is practical at this point ... >>> >>> >> Sure. I would wait for him to explain. >> > > It was done as a quick fix for backward compatibility, > > > IMHO it's a bug that needs to be fixed and not a compatibility that needs > to be backward supported. Let's just throw an error for now until we have > time and resources to fix the bug. > > Thanks, > Pavan > > -- -- *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> |
|
From: Ashutosh B. <ash...@en...> - 2014-03-10 08:38:30
|
On Sat, Mar 8, 2014 at 7:52 PM, Abbas Butt <abb...@en...>wrote: > Hi, > Attached please find patch to add a GUC called require_replicated_table_pkey > which is true by default. > The purpose of the GUC is to error out for non-FQS UPDATE / DELETE to a > replicated table. > Here is another reason why we shouldn't add requirement for primary key in replicated tables. Thanks Abbas for pointing it out. If the UPDATE/DELETE statement gets FQSed, we don't need a primary key for replicated table and if it doesn't, then we need a primary key. It will be difficult to explain users why some DMLs on replicated tables error out and why some of them don't. Again we come back to the backward compatibility. Before anyone suggests, disabling FQS for replicated tables, is not a solution here. It will affect performance badly. > By setting the GUC to false the error can be by passed and this is done to > run regression tests. > make check runs fine after the patch. > Test cases are added to check the added functionality. > Best Regards > > > > On Thu, Mar 6, 2014 at 9:00 AM, Pavan Deolasee <pav...@gm...>wrote: > >> >> >> On 06-Mar-2014, at 7:57 am, Abbas Butt <abb...@en...> >> wrote: >> >> >> >> >> On Wed, Mar 5, 2014 at 10:18 PM, Pavan Deolasee <pav...@gm... >> > wrote: >> >>> On Wed, Mar 5, 2014 at 2:32 PM, Koichi Suzuki <koi...@gm...>wrote: >>> >>>> I'd like Abbas to answer why it is practical at this point ... >>>> >>>> >>> Sure. I would wait for him to explain. >>> >> >> It was done as a quick fix for backward compatibility, >> >> >> IMHO it's a bug that needs to be fixed and not a compatibility that needs >> to be backward supported. Let's just throw an error for now until we have >> time and resources to fix the bug. >> >> Thanks, >> Pavan >> >> > > > -- > -- > *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> > > > ------------------------------------------------------------------------------ > Subversion Kills Productivity. Get off Subversion & Make the Move to > Perforce. > With Perforce, you get hassle-free workflows. Merge that actually works. > Faster operations. Version large binaries. Built-in WAN optimization and > the > freedom to use Git, Perforce or both. Make the move to Perforce. > > http://pubads.g.doubleclick.net/gampad/clk?id=122218951&iu=/4140/ostg.clktrk > _______________________________________________ > Postgres-xc-developers mailing list > Pos...@li... > https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers > > -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company |
|
From: Pavan D. <pav...@gm...> - 2014-03-10 09:25:24
|
> On 10-Mar-2014, at 2:08 pm, Ashutosh Bapat <ash...@en...> wrote: > > Here is another reason why we shouldn't add requirement for primary key in replicated tables. Thanks Abbas for pointing it out. > > If the UPDATE/DELETE statement gets FQSed, we don't need a primary key for replicated table and if it doesn't, then we need a primary key. It will be difficult to explain users why some DMLs on replicated tables error out and why some of them don't. We should error out irrespective of whether the query can be FQSed or not. > Again we come back to the backward compatibility. Again, it's a bug > Before anyone suggests, disabling FQS for replicated tables, is not a solution here. It will affect performance badly. IMHO correctness takes precedence over performance, especially now that we are moving from being an experimental product to being production ready FWIW I am ok with adding a non documented guc for now Thanks, Pavan > >> |
|
From: Ashutosh B. <ash...@en...> - 2014-03-10 11:00:24
|
On Mon, Mar 10, 2014 at 2:54 PM, Pavan Deolasee <pav...@gm...>wrote: > > > > > On 10-Mar-2014, at 2:08 pm, Ashutosh Bapat < > ash...@en...> wrote: > > Here is another reason why we shouldn't add requirement for primary key in > replicated tables. Thanks Abbas for pointing it out. > > If the UPDATE/DELETE statement gets FQSed, we don't need a primary key for > replicated table and if it doesn't, then we need a primary key. It will be > difficult to explain users why some DMLs on replicated tables error out and > why some of them don't. > > > We should error out irrespective of whether the query can be FQSed or not. > > That's not what the patch is doing. Hence, my objection. Anyway, there is no reason for an FQSable DML not to be FQSed since there is no data corruption happening in that case. > Again we come back to the backward compatibility. > > > Again, it's a bug > > My objection is for the hasty fixes. We should fix the bug in a clean way without having any bad impacts. > Before anyone suggests, disabling FQS for replicated tables, is not a > solution here. It will affect performance badly. > > > IMHO correctness takes precedence over performance, especially now that we > are moving from being an experimental product to being production ready > > Well, try doing that, and people wouldn't use Postgres-XC :) > FWIW I am ok with adding a non documented guc for now > > Thanks, > Pavan > > > >> >> -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company |
|
From: Michael P. <mic...@gm...> - 2014-03-10 10:32:55
|
On Mon, Mar 10, 2014 at 6:24 PM, Pavan Deolasee <pav...@gm...> wrote: > IMHO correctness takes precedence over performance, especially now that we > are moving from being an experimental product to being production ready +1. > FWIW I am ok with adding a non documented guc for now But -1 for that. It is better to be transparent to the user, if you do not document it even developers might forget about it and what it does. -- Michael |
|
From: Mason S. <ms...@tr...> - 2014-03-10 12:52:42
|
On Mon, Mar 10, 2014 at 6:32 AM, Michael Paquier <mic...@gm...>wrote: > On Mon, Mar 10, 2014 at 6:24 PM, Pavan Deolasee > <pav...@gm...> wrote: > > IMHO correctness takes precedence over performance, especially now that > we > > are moving from being an experimental product to being production ready > +1. > +1 -- Mason Sharp TransLattice - http://www.translattice.com Distributed and Clustered Database Solutions |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X