On 03/01/2014 12:06 PM, Simon Riggs wrote:
> On 27 February 2014 08:48, Simon Riggs <si...@2ndquadrant.com> wrote:
>> On 26 February 2014 15:25, Andres Freund <and...@2ndquadrant.com> wrote:
>>> On 2014-02-26 15:15:00 +0000, Simon Riggs wrote:
>>>> On 26 February 2014 13:38, Andres Freund <and...@2ndquadrant.com> wrote:
>>>>> Hi,
>>>>>
>>>>> On 2014-02-26 07:32:45 +0000, Simon Riggs wrote:
>>>>>>> * This definitely should include isolationtester tests actually
>>>>>>> performing concurrent ALTER TABLEs. All that's currently there is
>>>>>>> tests that the locklevel isn't too high, but not that it actually
>>>>>>> works.
>>>>>> There is no concurrent behaviour here, hence no code that would be
>>>>>> exercised by concurrent tests.
>>>>> Huh? There's most definitely new concurrent behaviour. Previously no
>>>>> other backends could have a relation open (and locked) while it got
>>>>> altered (which then sends out relcache invalidations). That's something
>>>>> that should be tested.
>>>> It has been. High volume concurrent testing has been performed, per
>>>> Tom's original discussion upthread, but that's not part of the test
>>>> suite.
>>> Yea, that's not what I am looking for.
>>>
>>>> For other tests I have no guide as to how to write a set of automated
>>>> regression tests. Anything could cause a failure, so I'd need to write
>>>> an infinite set of tests to prove there is no bug *somewhere*. How
>>>> many tests are required? 0, 1, 3, 30?
>>> I think some isolationtester tests for the most important changes in
>>> lock levels are appropriate. Say, create a PRIMARY KEY, DROP INHERIT,
>>> ... while a query is in progress in a nother session.
>> OK, I'll work on some tests.
>>
>> v18 attached, with v19 coming soon
> v19 complete apart from requested comment additions
I've started to look at this patch and re-read the thread. The first
thing I noticed is what seems like an automated replace error. The docs
say "This form requires only an SHARE x EXCLUSIVE lock." where the "an"
was not changed to "a".
Attached is a patch-on-patch to fix this. A more complete review will
come later.
--
Vik
*** a/doc/src/sgml/ref/alter_table.sgml
--- b/doc/src/sgml/ref/alter_table.sgml
***************
*** 157,163 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
table to change.
</para>
<para>
! This form requires only an <literal>SHARE ROW EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
--- 157,163 ----
table to change.
</para>
<para>
! This form requires only a <literal>SHARE ROW EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
***************
*** 188,194 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
<xref linkend="planner-stats">.
</para>
<para>
! This form requires only an <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
--- 188,194 ----
<xref linkend="planner-stats">.
</para>
<para>
! This form requires only a <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
***************
*** 223,229 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
planner, refer to <xref linkend="planner-stats">.
</para>
<para>
! This form requires only an <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
--- 223,229 ----
planner, refer to <xref linkend="planner-stats">.
</para>
<para>
! This form requires only a <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
***************
*** 344,350 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
created. Currently only foreign key constraints may be altered.
</para>
<para>
! This form requires only an <literal>SHARE ROW EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
--- 344,350 ----
created. Currently only foreign key constraints may be altered.
</para>
<para>
! This form requires only a <literal>SHARE ROW EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
***************
*** 366,372 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
does not prevent normal write commands against the table while it runs.
</para>
<para>
! This form requires only an <literal>SHARE UPDATE EXCLUSIVE</literal> lock
on the table being altered. If the constraint is a foreign key then
a <literal>ROW SHARE</literal> lock is also required on
the table referenced by the constraint.
--- 366,372 ----
does not prevent normal write commands against the table while it runs.
</para>
<para>
! This form requires only a <literal>SHARE UPDATE EXCLUSIVE</literal> lock
on the table being altered. If the constraint is a foreign key then
a <literal>ROW SHARE</literal> lock is also required on
the table referenced by the constraint.
***************
*** 411,417 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
fire regardless of the current replication mode.
</para>
<para>
! This form requires only an <literal>SHARE ROW EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
--- 411,417 ----
fire regardless of the current replication mode.
</para>
<para>
! This form requires only a <literal>SHARE ROW EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
***************
*** 439,445 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
operations. It does not actually re-cluster the table.
</para>
<para>
! This form requires only an <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
--- 439,445 ----
operations. It does not actually re-cluster the table.
</para>
<para>
! This form requires only a <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
***************
*** 454,460 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
future cluster operations that don't specify an index.
</para>
<para>
! This form requires only an <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
--- 454,460 ----
future cluster operations that don't specify an index.
</para>
<para>
! This form requires only a <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
***************
*** 504,510 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
of <command>ALTER TABLE</> that forces a table rewrite.
</para>
<para>
! This form requires only an <literal>SHARE ROW EXCLUSIVE</literal> lock.
</para>
<note>
--- 504,510 ----
of <command>ALTER TABLE</> that forces a table rewrite.
</para>
<para>
! This form requires only a <literal>SHARE ROW EXCLUSIVE</literal> lock.
</para>
<note>
***************
*** 529,535 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
needed to update the table entirely.
</para>
<para>
! This form requires only an <literal>SHARE ROW EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
--- 529,535 ----
needed to update the table entirely.
</para>
<para>
! This form requires only a <literal>SHARE ROW EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
***************
*** 560,566 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
this might change in the future.
</para>
<para>
! This form requires only an <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
--- 560,566 ----
this might change in the future.
</para>
<para>
! This form requires only a <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
***************
*** 575,581 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
from the target table.
</para>
<para>
! This form requires only an <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
--- 575,581 ----
from the target table.
</para>
<para>
! This form requires only a <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
***************
*** 593,599 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
definition.
</para>
<para>
! This form requires only an <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
--- 593,599 ----
definition.
</para>
<para>
! This form requires only a <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
***************
*** 605,611 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
This form dissociates a typed table from its type.
</para>
<para>
! This form requires only an <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
--- 605,611 ----
This form dissociates a typed table from its type.
</para>
<para>
! This form requires only a <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
</para>
</listitem>
</varlistentry>
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
