Thread: [HACKERS] Press Release Draft - 2016-02-09 Cumulative Update

> As there are a lot of updates I did my best to consolidate some of the
> bullet points and as usual, people are directed to the release notes.
> Please let me know if there are any inaccuracies so I can fix them ASAP.

Just some minor points:

>  * Several fixes for PostgreSQL operating in hot standby mode

It sounded unnatural to me.  Maybe it is better without "PostgreSQL".

>  * Several vacuum and autovacuum fxies

Typo

>  * Several Unicode fixes

It sounded alarming to me.  I see just one related item on the release
notes.  Maybe we can clarify the problem.

>  * Sync our copy of the timezone library with IANA release tzcode2016j

This is repeated on the following sentence.

>      BEGIN;
>      DROP INDEX bad_index_name;
>      CREATE INDEX CONCURRENTLY bad_index_name ON table_name (column_name); /* replace names with your original index
definition*/
 
>      COMMIT;

Maybe you meant CREATE INDEX without CONCURRENTLY?

Hi,

Am Dienstag, den 07.02.2017, 10:37 -0500 schrieb Jonathan S. Katz:

> Below is the draft of the press release for the update this Thursday:

About the CREATE INDEX CONCURRENTLY issue, I wonder whether Peter's
amcheck extension[1] would catch that (for B-Tree indexes at least), and
if that is the case, whether we could mention that to our users as
guidance for how to check for index corruption?


Michael

[1] https://github.com/petergeoghegan/amcheck
-- 
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax:  +49 2166 9901-100
Email: michael.banck@credativ.de

credativ GmbH, HRB Mönchengladbach 12080
USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer

Michael Banck wrote:
> Hi,
> 
> Am Dienstag, den 07.02.2017, 10:37 -0500 schrieb Jonathan S. Katz:
> 
> > Below is the draft of the press release for the update this Thursday:
> 
> About the CREATE INDEX CONCURRENTLY issue, I wonder whether Peter's
> amcheck extension[1] would catch that (for B-Tree indexes at least), and
> if that is the case, whether we could mention that to our users as
> guidance for how to check for index corruption?

"it does not verify that the target index is consistent with its heap
relation"

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Jonathan S. Katz wrote:

> Below is the draft of the press release for the update this Thursday:
> 
>
https://git.postgresql.org/gitweb/?p=press.git;a=blob;f=update_releases/current/20170209updaterelease.md;h=0cccb8986c08527f65f13d704a78c36bb8de7fef;hb=afc01091dea8a1597e8e21430edc3908c581ce0c
<https://git.postgresql.org/gitweb/?p=press.git;a=blob;f=update_releases/current/20170209updaterelease.md;h=0cccb8986c08527f65f13d704a78c36bb8de7fef;hb=afc01091dea8a1597e8e21430edc3908c581ce0c>
> 
> As there are a lot of updates I did my best to consolidate some of the bullet points and as usual, people are
directedto the release notes.  Please let me know if there are any inaccuracies so I can fix them ASAP.
 

Please do post the proposed text on list for ease of review.  I wasn't
looking at the text, so I wouldn't have noticed this if Emre hadn'treplied:
 76 If you believe you have been affected by the aforementioned CREATE INDEX CONCURRENTLY bug, you will have to rebuild
theindex.  An example of rebuilding an index: 77  78     BEGIN; 79     DROP INDEX bad_index_name; 80     CREATE INDEX
CONCURRENTLYbad_index_name ON table_name (column_name); /* replace names with your original index definition */ 81
COMMIT;

This is not a good recipe, because using CREATE INDEX CONCURRENTLY in
the same transaction that grabs an exclusive lock on the table for the
DROP INDEX is pointless -- the access exclusive lock is held until the
end of the transaction, and CIC does not work inside a transaction
anyway so it'd raise an ERROR and rollback the DROP INDEX.  So the user
would probably drop the BEGIN/COMMIT sequence in order for this to work
in the first place.  (The other option is to use CREATE INDEX not
concurrent, but that would lock the table for a very long time).

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

> On Feb 7, 2017, at 11:25 AM, Emre Hasegeli <emre@hasegeli.com> wrote:
>
>> As there are a lot of updates I did my best to consolidate some of the
>> bullet points and as usual, people are directed to the release notes.
>> Please let me know if there are any inaccuracies so I can fix them ASAP.
>
> Just some minor points:
>
>> * Several fixes for PostgreSQL operating in hot standby mode
>
> It sounded unnatural to me.  Maybe it is better without "PostgreSQL".
>
>> * Several vacuum and autovacuum fxies
>
> Typo
>
>> * Several Unicode fixes
>
> It sounded alarming to me.  I see just one related item on the release
> notes.  Maybe we can clarify the problem.
>
>> * Sync our copy of the timezone library with IANA release tzcode2016j
>
> This is repeated on the following sentence.
>
>>     BEGIN;
>>     DROP INDEX bad_index_name;
>>     CREATE INDEX CONCURRENTLY bad_index_name ON table_name (column_name); /* replace names with your original index
definition*/ 
>>     COMMIT;
>
> Maybe you meant CREATE INDEX without CONCURRENTLY?

Thanks for the corrections / suggestions.  I have applied them and will push a new version shortly!

Jonathan S. Katz wrote:

> Thanks for the clarification.  I have updated the recipe along with Emre’s comments here:
>
> [updated text not included in the email]

I still don't think the recipe is a very good one because it leaves you
with a window where the affected columns are not indexed at all.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Hi,

Am Dienstag, den 07.02.2017, 15:58 -0500 schrieb Jonathan S. Katz:


>
https://git.postgresql.org/gitweb/?p=press.git;a=blob;f=update_releases/current/20170209updaterelease.txt;h=f90d4716f240dbea4cca647b099f79865545b633;hb=d85498c284275bcab4752b91476834de780648b8

It says "[...]then rows that were updated by transactions running at the
same time as the CREATE INDEX CONCURRENTLY command could have been index
incorrectly."

That sounds off to me, shouldn't it be "indexed incorrectly" or
something?


Michael

-- 
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax:  +49 2166 9901-100
Email: michael.banck@credativ.de

credativ GmbH, HRB Mönchengladbach 12080
USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer

> On Feb 7, 2017, at 4:07 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
> Jonathan S. Katz wrote:
>
>> Thanks for the clarification.  I have updated the recipe along with Emre’s comments here:
>>
>> [updated text not included in the email]
>
> I still don't think the recipe is a very good one because it leaves you
> with a window where the affected columns are not indexed at all.

Okay, so I propose two options:
1.  Does anyone have a recipe they recommend that might be better? OR2.  We just leave out the recipe altogether (which
iswhat I am leaning towards at the moment). 

Thanks!

Jonathan

> On Feb 7, 2017, at 4:39 PM, Michael Banck <michael.banck@credativ.de> wrote:
>
> Hi,
>
> Am Dienstag, den 07.02.2017, 15:58 -0500 schrieb Jonathan S. Katz:
>
>
>>
https://git.postgresql.org/gitweb/?p=press.git;a=blob;f=update_releases/current/20170209updaterelease.txt;h=f90d4716f240dbea4cca647b099f79865545b633;hb=d85498c284275bcab4752b91476834de780648b8
>
> It says "[...]then rows that were updated by transactions running at the
> same time as the CREATE INDEX CONCURRENTLY command could have been index
> incorrectly."
>
> That sounds off to me, shouldn't it be "indexed incorrectly" or
> something?

Yes, passive voice :(  I’ve made the modification on my local copy and will push it up after the resolution on the
CREATEINDEX recipe. 

Jonathan

Jonathan S. Katz wrote:
> 
> > On Feb 7, 2017, at 4:07 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > 
> > Jonathan S. Katz wrote:
> > 
> >> Thanks for the clarification.  I have updated the recipe along with Emre’s comments here:
> >> 
> >> [updated text not included in the email]
> > 
> > I still don't think the recipe is a very good one because it leaves you
> > with a window where the affected columns are not indexed at all.
> 
> Okay, so I propose two options:
> 
>     1.  Does anyone have a recipe they recommend that might be better? OR

Do the CREATE INDEX CONCURRENTLY first, then DROP INDEX CONCURRENTLY the
old index, then rename the new index to the old name.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Am 07.02.2017 um 18:44 schrieb Alvaro Herrera <alvherre@2ndquadrant.com>:
>  80     CREATE INDEX CONCURRENTLY bad_index_name ON table_name (column_name); /* replace names with your original
indexdefinition */ 

I was thinking if we could replace that "replace names with your original index definition" with something more fancy
usingpg_get_indexdef in that recipe. I ended up with quite a "REINDEX CONCURRENTLY" monster: 

\set index_name 'my_bad_index'
\set table_schema 'public'
SELECT :'index_name'||'_'||left(md5(random()::text), 5) AS index_name_tmp \gset
SELECT replace(replace(pg_get_indexdef((quote_ident(:'table_schema')||'.'||quote_ident(:'index_name'))::regclass),
'INDEX'||quote_ident(:'index_name'), 'INDEX '||quote_ident(:'index_name_tmp')), 'CREATE INDEX', 'CREATE INDEX
CONCURRENTLY')\gexec 
DROP INDEX CONCURRENTLY :"table_schema".:"index_name";
ALTER INDEX :"table_schema".:"index_name_tmp" RENAME TO :"index_name";

Probably not useable as a recipe in such an announcement but it was fun to build and to see what is actually possible
withsome psql magic :) 

Tobias

Tobias Bussmann wrote:
> Am 07.02.2017 um 18:44 schrieb Alvaro Herrera <alvherre@2ndquadrant.com>:
> >  80     CREATE INDEX CONCURRENTLY bad_index_name ON table_name (column_name); /* replace names with your original
indexdefinition */
 
> 
> I was thinking if we could replace that "replace names with your original index definition" with something more fancy
usingpg_get_indexdef in that recipe. I ended up with quite a "REINDEX CONCURRENTLY" monster:
 
> 
> \set index_name 'my_bad_index' 
> \set table_schema 'public'
> SELECT :'index_name'||'_'||left(md5(random()::text), 5) AS index_name_tmp \gset
> SELECT replace(replace(pg_get_indexdef((quote_ident(:'table_schema')||'.'||quote_ident(:'index_name'))::regclass),
'INDEX'||quote_ident(:'index_name'), 'INDEX '||quote_ident(:'index_name_tmp')), 'CREATE INDEX', 'CREATE INDEX
CONCURRENTLY')\gexec
 
> DROP INDEX CONCURRENTLY :"table_schema".:"index_name";
> ALTER INDEX :"table_schema".:"index_name_tmp" RENAME TO :"index_name";
> 
> Probably not useable as a recipe in such an announcement but it was fun to build and to see what is actually possible
withsome psql magic :)
 

Note that this is likely to fail if the original index name is close to
the 63 chars limit.  Perhaps it's enough to add substring() when
computing index_name_tmp.  (You could just not use :'index_name' there
and rely on the random md5 only, actually).  Watch out for UNIQUE too.

FWIW for previous problems we've documented them in wiki pages along
with suggested solutions, and added a link to that wiki page in the
announce.  Perhaps one thing to do is create a wiki page for this one
too (not volunteering myself).  Probably too late to add the link to the
press release now, since it's already out as "final".

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Am 08.02.2017 um 20:17 schrieb Alvaro Herrera <alvherre@2ndquadrant.com>:
> Note that this is likely to fail if the original index name is close to
> the 63 chars limit.  Perhaps it's enough to add substring() when
> computing index_name_tmp.  (You could just not use :'index_name' there
> and rely on the random md5 only, actually).  Watch out for UNIQUE too.

thank you for your valuable input! Here is a version that should take both into account - the query also could be
simplifieda bit: 

\set index_name 'my_bad_index'
\set table_schema 'public'
SELECT 'tmp_'||md5(random()::text) AS index_name_tmp \gset
SELECT replace(pg_get_indexdef((quote_ident(:'table_schema')||'.'||quote_ident(:'index_name'))::regclass), '
'||quote_ident(:'index_name')||'ON', ' CONCURRENTLY '||:'index_name_tmp'||' ON') \gexec 
DROP INDEX CONCURRENTLY :"table_schema".:"index_name";
ALTER INDEX :"table_schema".:"index_name_tmp" RENAME TO :"index_name";


> FWIW for previous problems we've documented them in wiki pages along
> with suggested solutions, and added a link to that wiki page in the
> announce.  Perhaps one thing to do is create a wiki page for this one
> too (not volunteering myself).

I'm not even remotely into the details of the CIC issue, so I'm not the right one to create a page on that topic. But I
couldput this snippet as a "REINDEX CONCURRENTLY" workaround into the Administrative Snippets category of the wiki, if
thereare no further objections about the way it works. I always have a bit of mixed feelings with these kind of string
manipulationson dynamic SQL. 

Best,
Tobias

Tobias Bussmann wrote:

> But I could put this
> snippet as a "REINDEX CONCURRENTLY" workaround into the Administrative
> Snippets category of the wiki, if there are no further objections
> about the way it works.

Sounds like a good idea.  There are further complications:

* you can't DROP indexes belonging to constraints, so this recipe
doesn't work for them.  One useful trick is to create the index first,
then ADD CONSTRAINT USING INDEX.

* For unique constraints referenced by FKs, the above doesn't work
either.  One thing you can do is create a second index and swap the
relfilenode underneath.  This is a nasty, dirty, dangerous, unsupported
trick, but it can save people's neck at times.

> I always have a bit of mixed feelings with these kind of string
> manipulations on dynamic SQL.

It may look a bit nasty, but locking tables for long periods (or being
without an important index for a period) is much worse in production
scenarios.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2/8/17 2:51 PM, Alvaro Herrera wrote:
>> I always have a bit of mixed feelings with these kind of string
>> manipulations on dynamic SQL.
> It may look a bit nasty, but locking tables for long periods (or being
> without an important index for a period) is much worse in production
> scenarios.

I think posting a recipe in the wiki is a great idea (especially if it 
handles corner cases like constraints). I'm not so keen on trying to 
code it entirely in psql though. I think it'd be a lot cleaner to have a 
plpgsql function that uses format() to construct the appropriate 
commands to run and then spit that out as text. Users can either cut and 
paste that, or they can \gset it to a variable that they then execute, 
or they can capture the output to a file which they execute.

The big advantage to this is by default you see what commands would be 
run, but you can still fully automate if you want to without much extra 
effort.
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

On 2/7/17 9:37 AM, Jonathan S. Katz wrote:
> Below is the draft of the press release for the update this Thursday:

Thanks for the work on this!
  11 There existed a race condition if CREATE INDEX CONCURRENTLY was 
called on a column that had not been indexed before, then rows that were 
updated by transactions running at the same time as the CREATE INDEX 
CONCURRENTLY command could have been indexed incorrectly.

I think that'd read better as
  11 There existed a race condition /where/ if CREATE INDEX 
CONCURRENTLY was called on a column that had not been indexed before, 
then rows that were updated by transactions running at the same time as 
the CREATE INDEX CONCURRENTLY command /may not/ have been indexed 
incorrectly.

Also, maybe we should mention that there's no way to test for this, and 
make a stronger suggestion to redo any affected indexes?
  20 These release contains several fixes to improve the stability of 
visible data and WAL logging that we wish to highlight here.

I think this sentence can just go. If we want to keep it, IMHO this is a 
better alternative: "This release contains several improvements to the 
stability of data visibility and WAL logging."
  22 Prior to this release, data could be prematurely pruned by a 
vacuum operation when a special snapshot used for catalog scans was 
presently available.

... vacuum operation even though a special catalog scan snapshot was in use.

BTW, I don't know what came out of the discussion of git references in 
release notes, but I'd find it useful to be able to at least get a 
complete list. Not hard for me to do that since I know git and our 
naming scheme, but maybe we should include directions for doing so?
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

On Wed, Feb 8, 2017 at 5:31 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
>   11 There existed a race condition /where/ if CREATE INDEX CONCURRENTLY was
> called on a column that had not been indexed before, then rows that were
> updated by transactions running at the same time as the CREATE INDEX
> CONCURRENTLY command /may not/ have been indexed incorrectly.

I think this is moot at this point, but "may not have been indexed
incorrectly" seems to have two negatives where there should be only
one.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company