[COMMITTERS] pgsql: pgbench: replace run-time string comparisons with an enum identi
pgbench: replace run-time string comparisons with an enum identifier. Minor refactoring that should yield some performance benefit. Fabien Coelho, reviewed by Aleksandr Parfenov Discussion: https://postgr.es/m/alpine.DEB.2.20.1709230538130.4999@lancre Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/f987f83de20afe3ba78be1e15db5dffe7488faa7 Modified Files -- src/bin/pgbench/pgbench.c | 62 ++- 1 file changed, 51 insertions(+), 11 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Set the metapage's pd_lower correctly in brin, gin, and spgist i
Set the metapage's pd_lower correctly in brin, gin, and spgist indexes. Previously, these index types left the pd_lower field set to the default SizeOfPageHeaderData, which is really a lie because it ought to point past whatever space is being used for metadata. The coding accidentally failed to fail because we never told xlog.c that the metapage is of standard format --- but that's not very good, because it impedes WAL consistency checking, and in some cases prevents compression of full-page images. To fix, ensure that we set pd_lower correctly, not only when creating a metapage but whenever we write it out (these apparently redundant steps are needed to cope with pg_upgrade'd indexes that don't yet contain the right value). This allows telling xlog.c that the page is of standard format. The WAL consistency check mask functions are made to mask only if pd_lower appears valid, which I think is likely unnecessary complication, since any metapage appearing in a v11 WAL stream should contain valid pd_lower. But it doesn't cost much to be paranoid. Amit Langote, reviewed by Michael Paquier and Amit Kapila Discussion: https://postgr.es/m/0d273805-0e9e-ec1a-cb84-d4da400b8...@lab.ntt.co.jp Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/81e334ce4e6d687d548e60ad8954b7dfd9e568a2 Modified Files -- src/backend/access/brin/brin.c | 4 ++-- src/backend/access/brin/brin_pageops.c | 10 +- src/backend/access/brin/brin_revmap.c | 15 +-- src/backend/access/brin/brin_xlog.c| 21 +++-- src/backend/access/gin/ginfast.c | 26 +++--- src/backend/access/gin/gininsert.c | 4 ++-- src/backend/access/gin/ginutil.c | 20 +++- src/backend/access/gin/ginxlog.c | 25 ++--- src/backend/access/spgist/spginsert.c | 4 ++-- src/backend/access/spgist/spgutils.c | 24 ++-- src/backend/access/spgist/spgxlog.c| 7 --- 11 files changed, 125 insertions(+), 35 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix float parsing in ecpg INFORMIX mode.
Fix float parsing in ecpg INFORMIX mode. Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/049dab00942165c3df3aeace9e8e86cd2679dce0 Modified Files -- src/interfaces/ecpg/ecpglib/data.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix float parsing in ecpg INFORMIX mode.
Fix float parsing in ecpg INFORMIX mode. Branch -- REL9_4_STABLE Details --- https://git.postgresql.org/pg/commitdiff/ad24e7bc28d45b180726d6ff144e58d67e73aaa4 Modified Files -- src/interfaces/ecpg/ecpglib/data.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix float parsing in ecpg INFORMIX mode.
Fix float parsing in ecpg INFORMIX mode. Branch -- REL9_3_STABLE Details --- https://git.postgresql.org/pg/commitdiff/7a35507acceb07c4ed1a7a0c82db50eee3101df3 Modified Files -- src/interfaces/ecpg/ecpglib/data.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix float parsing in ecpg INFORMIX mode.
Fix float parsing in ecpg INFORMIX mode. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/6976a4f05fc5f9d3b469869e412e0814c8c7ab2a Modified Files -- src/interfaces/ecpg/ecpglib/data.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix float parsing in ecpg INFORMIX mode.
Fix float parsing in ecpg INFORMIX mode. Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/b6d95939ed6fc8bde97eb8c9af00838dde82cc41 Modified Files -- src/interfaces/ecpg/ecpglib/data.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix float parsing in ecpg INFORMIX mode.
Fix float parsing in ecpg INFORMIX mode. Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/ee46980a73bf5b1b39c3a6be76dc01fa2a5819c7 Modified Files -- src/interfaces/ecpg/ecpglib/data.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: pg_ctl: Improve message
pg_ctl: Improve message Change message for restarting a server from a directory without a PID file. This accounts for the case where a restart happens after an initdb. The new message indicates that the start has not completed yet and might fail. Author: Jesper Pedersen Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/4b0fbfdf81e0a847b31d0b430f25f8660d5652c0 Modified Files -- src/bin/pg_ctl/pg_ctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
Robert Haas writes: > Well, my thought was that delaying this release for a week would be > better than either (a) doing an extra minor release just to get this > fix out or (b) waiting another three months to release this fix. The > former seems like fairly unnecessary work, and the latter doesn't seem > particularly responsible. Users can't reasonably expect us to fix > data-loss-causing bugs that we don't know about yet, but they can > reasonably expect us to issue fixes promptly for ones that we do know > about. Our experience with "hold the release waiting for a fix for bug X" decisions has been consistently bad. Furthermore, if we can't produce a patch we trust by Monday, I would much rather that we not do it in a rushed fashion at all. I think it would be entirely reasonable to consider making off-cadence releases in, perhaps, a month, once the dust is entirely settled. regards, tom lane -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
On Thu, Nov 2, 2017 at 10:38 PM, Tom Lane wrote: >> Personally, I think it would be best to push the release out a week. > > I would only be in favor of that if there were some reason to think that > the bug is worse now than it's been in the four years since 9.3 was > released. Otherwise, we should ship the bug fixes we have on-schedule. > I think it's a very very safe bet that there are other data-loss-causing > bugs in there, so I see no good reason for panicking over this one. Well, my thought was that delaying this release for a week would be better than either (a) doing an extra minor release just to get this fix out or (b) waiting another three months to release this fix. The former seems like fairly unnecessary work, and the latter doesn't seem particularly responsible. Users can't reasonably expect us to fix data-loss-causing bugs that we don't know about yet, but they can reasonably expect us to issue fixes promptly for ones that we do know about. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
On Thu, Nov 2, 2017 at 10:26 PM, Peter Geoghegan wrote: > On Thu, Nov 2, 2017 at 9:44 AM, Robert Haas wrote: >> The second commit (22576734b805fb0952f9be841ca8f643694ee868) is where >> I think things get a lot more dangerous. The problem (as Andres >> pointed out to me this afternoon) is that it seems possible to end up >> with a situation where there should be two HOT chains on a page, and >> because of the weakened xmin/xmax checking rules, we end up thinking >> that the second one is a continuation of the first one, which will be >> all kinds of bad news. That would be particularly likely to happen in >> releases from before we invented HEAP_XMIN_FROZEN, when there's no >> xmin/xmax matching at all, but could happen on later releases if we >> are extraordinarily unlucky (i.e. xmin of the first tuple in the >> second chain happens to be the same as the pre-freeze xmax in the old >> chain, probably because the same XID was used to update the page in >> two consecutive epochs). Fortunately, that commit is (I think) not >> released anywhere. > > FWIW, if you look at the second commit > (22576734b805fb0952f9be841ca8f643694ee868) carefully, you'll realize > that it doesn't even treat those two cases differently. It was buggy > even on its own terms. The FrozenTransactionId test used an xmin from > HeapTupleHeaderGetXmin(), not HeapTupleHeaderGetRawXmin(). Oh, wow. You seem to be correct. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
Robert Haas writes: > Just to be clear, it looks like "Fix freezing of a dead HOT-updated > tuple" (46c35116ae1acc8826705ef2a7b5d9110f9d6e84) went in before 10.0 > was stamped, but "Fix traversal of half-frozen update chains" > (22576734b805fb0952f9be841ca8f643694ee868) went in afterwards and is > therefore unreleased at present. Thanks for doing this analysis of the actual effects in 10.0. > Personally, I think it would be best to push the release out a week. I would only be in favor of that if there were some reason to think that the bug is worse now than it's been in the four years since 9.3 was released. Otherwise, we should ship the bug fixes we have on-schedule. I think it's a very very safe bet that there are other data-loss-causing bugs in there, so I see no good reason for panicking over this one. > ... and (I know you're all tired of hearing me say this) patches that > implicate the on-disk format are scary. Agreed, but how is that relevant now that the bogus patches are reverted? regards, tom lane -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] sytax errors in standby commands
I am setting up PostgreSQL standby on windows for the first time on version 9.4.14: Is the below syntax correct in the recovery.conf file restore_command = 'copy "D:\\apps\postgres\\pg_archivelog\\%f" "%p"' archive_cleanup_command = 'pg_archivecleanup "D:\\apps\postgres\pg_archivelog\\" %r' And below command is correct in syntax in portgresql.conf file archive_command = 'copy "%p" "D:\\apps\\postgres\\pg_archivelog\\%f"' I am archiving to local server -- Sent from: http://www.postgresql-archive.org/PostgreSQL-committers-f2094136.html -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Simplify new test suite handling of passwordcheck
Simplify new test suite handling of passwordcheck This changes the use of a custom configuration file to enforce the value of preload_shared_libraries to simply load the library during the tests. This removes the restriction of running installcheck on the tests, and simplifies its makefile contrary to what has been introduced in af7211e. Author: Michael Paquier Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/637a934ab9bac615af6032bb8424056e91988fb8 Modified Files -- contrib/passwordcheck/Makefile | 3 --- contrib/passwordcheck/expected/passwordcheck.out | 1 + contrib/passwordcheck/passwordcheck.conf | 1 - contrib/passwordcheck/sql/passwordcheck.sql | 2 ++ 4 files changed, 3 insertions(+), 4 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
On Thu, Nov 2, 2017 at 9:44 AM, Robert Haas wrote: > The second commit (22576734b805fb0952f9be841ca8f643694ee868) is where > I think things get a lot more dangerous. The problem (as Andres > pointed out to me this afternoon) is that it seems possible to end up > with a situation where there should be two HOT chains on a page, and > because of the weakened xmin/xmax checking rules, we end up thinking > that the second one is a continuation of the first one, which will be > all kinds of bad news. That would be particularly likely to happen in > releases from before we invented HEAP_XMIN_FROZEN, when there's no > xmin/xmax matching at all, but could happen on later releases if we > are extraordinarily unlucky (i.e. xmin of the first tuple in the > second chain happens to be the same as the pre-freeze xmax in the old > chain, probably because the same XID was used to update the page in > two consecutive epochs). Fortunately, that commit is (I think) not > released anywhere. FWIW, if you look at the second commit (22576734b805fb0952f9be841ca8f643694ee868) carefully, you'll realize that it doesn't even treat those two cases differently. It was buggy even on its own terms. The FrozenTransactionId test used an xmin from HeapTupleHeaderGetXmin(), not HeapTupleHeaderGetRawXmin(). -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix corner-case errors in brin_doupdate().
Fix corner-case errors in brin_doupdate(). In some cases the BRIN code releases lock on an index page, and later re-acquires lock and tries to check that the tuple it was working on is still there. That check was a couple bricks shy of a load. It didn't consider that the page might have turned into a "revmap" page. (The samepage code path doesn't call brin_getinsertbuffer(), so it isn't protected by the checks for revmap status there.) It also didn't check whether the tuple offset was now off the end of the linepointer array. Since commit 24992c6db the latter case is pretty common, but at least in principle it could have occurred before that. The net result is that concurrent updates of a BRIN index could fail with errors like "invalid index offnum" or "inconsistent range map". Per report from Tomas Vondra. Back-patch to 9.5, since this code is substantially the same in all versions containing BRIN. Discussion: https://postgr.es/m/10d2b9f9-f427-03b8-8ad9-6af4ecacb...@2ndquadrant.com Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/97ba7b8c87e13112fd31d936507e7d060a297aa4 Modified Files -- src/backend/access/brin/brin_pageops.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix corner-case errors in brin_doupdate().
Fix corner-case errors in brin_doupdate(). In some cases the BRIN code releases lock on an index page, and later re-acquires lock and tries to check that the tuple it was working on is still there. That check was a couple bricks shy of a load. It didn't consider that the page might have turned into a "revmap" page. (The samepage code path doesn't call brin_getinsertbuffer(), so it isn't protected by the checks for revmap status there.) It also didn't check whether the tuple offset was now off the end of the linepointer array. Since commit 24992c6db the latter case is pretty common, but at least in principle it could have occurred before that. The net result is that concurrent updates of a BRIN index could fail with errors like "invalid index offnum" or "inconsistent range map". Per report from Tomas Vondra. Back-patch to 9.5, since this code is substantially the same in all versions containing BRIN. Discussion: https://postgr.es/m/10d2b9f9-f427-03b8-8ad9-6af4ecacb...@2ndquadrant.com Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/a43cd427e7033771645abd4f9dbd2b1b2ab5ed5c Modified Files -- src/backend/access/brin/brin_pageops.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix corner-case errors in brin_doupdate().
Fix corner-case errors in brin_doupdate(). In some cases the BRIN code releases lock on an index page, and later re-acquires lock and tries to check that the tuple it was working on is still there. That check was a couple bricks shy of a load. It didn't consider that the page might have turned into a "revmap" page. (The samepage code path doesn't call brin_getinsertbuffer(), so it isn't protected by the checks for revmap status there.) It also didn't check whether the tuple offset was now off the end of the linepointer array. Since commit 24992c6db the latter case is pretty common, but at least in principle it could have occurred before that. The net result is that concurrent updates of a BRIN index could fail with errors like "invalid index offnum" or "inconsistent range map". Per report from Tomas Vondra. Back-patch to 9.5, since this code is substantially the same in all versions containing BRIN. Discussion: https://postgr.es/m/10d2b9f9-f427-03b8-8ad9-6af4ecacb...@2ndquadrant.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/62a16572d5714bfb19e2a273e61218be6682d3df Modified Files -- src/backend/access/brin/brin_pageops.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix corner-case errors in brin_doupdate().
Fix corner-case errors in brin_doupdate(). In some cases the BRIN code releases lock on an index page, and later re-acquires lock and tries to check that the tuple it was working on is still there. That check was a couple bricks shy of a load. It didn't consider that the page might have turned into a "revmap" page. (The samepage code path doesn't call brin_getinsertbuffer(), so it isn't protected by the checks for revmap status there.) It also didn't check whether the tuple offset was now off the end of the linepointer array. Since commit 24992c6db the latter case is pretty common, but at least in principle it could have occurred before that. The net result is that concurrent updates of a BRIN index could fail with errors like "invalid index offnum" or "inconsistent range map". Per report from Tomas Vondra. Back-patch to 9.5, since this code is substantially the same in all versions containing BRIN. Discussion: https://postgr.es/m/10d2b9f9-f427-03b8-8ad9-6af4ecacb...@2ndquadrant.com Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/43276abc6f7e497bfca2277b9e04585fcd6c25c5 Modified Files -- src/backend/access/brin/brin_pageops.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
On Thu, Nov 2, 2017 at 4:20 AM, Andres Freund wrote: > I think the problem is on the pruning, rather than the freezing side. We > can't freeze a tuple if it has an alive predecessor - rather than > weakining this, we should be fixing the pruning to not have the alive > predecessor. Excellent catch. > If the update xmin is actually below the cutoff we can remove the tuple > even if there's live lockers - the lockers will also be present in the > newer version of the tuple. I verified that for me that fixes the > problem. Obviously that'd require some comment work and more careful > diagnosis. I didn't even know that that was safe. > I think a5736bf754c82d8b86674e199e232096c679201d might be dangerous in > the face of previously corrupted tuple chains and pg_upgraded clusters - > it can lead to tuples being considered related, even though they they're > from entirely independent hot chains. Especially when upgrading 9.3 post > your fix, to current releases. Frankly, I'm relieved that you got to this. I was highly suspicious of a5736bf754c82d8b86674e199e232096c679201d, even beyond my specific, actionable concern about how it failed to handle the 9.3/FrozenTransactionId xmin case as special. As I went into in the "heap/SLRU verification, relfrozenxid cut-off, and freeze-the-dead bug" thread, these commits left us with a situation where there didn't seem to be a reliable way of knowing whether or not it is safe to interrogate clog for a given heap tuple using a tool like amcheck. And, it wasn't obvious that you couldn't have a codepath that failed to account for pre-cutoff non-frozen tuples -- codepaths that call TransactionIdDidCommit() despite it actually being unsafe. If I'm not mistaken, your proposed fix restores sanity there. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Remove wal_keep_segments from default configuration in PostgresN
Remove wal_keep_segments from default configuration in PostgresNode.pm This is only used in the pg_rewind tests, so only set it there. It's better if other tests run closer to a default configuration. Author: Michael Paquier Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/5eb8bf2d42676523143c1c76ba584bcdcc584f3e Modified Files -- src/bin/pg_rewind/RewindTest.pm | 5 + src/test/perl/PostgresNode.pm | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
On Thu, Nov 2, 2017 at 8:25 PM, Alvaro Herrera wrote: > Pushed the reverts. > > I noticed while doing so that REL_10_STABLE contains the bogus commits. > Does that change our opinion regarding what to do for people upgrading > to a version containing the broken commits? I don't think so, because > > 1) we hope that not many people will trust their data to 10.0 > immediately after release > 2) the bug is very low probability > 3) it doesn't look like we can do a lot about it anyway. Just to be clear, it looks like "Fix freezing of a dead HOT-updated tuple" (46c35116ae1acc8826705ef2a7b5d9110f9d6e84) went in before 10.0 was stamped, but "Fix traversal of half-frozen update chains" (22576734b805fb0952f9be841ca8f643694ee868) went in afterwards and is therefore unreleased at present. Users of 10.0 who hit the code introduced by 46c35116ae1acc8826705ef2a7b5d9110f9d6e84 will have XIDs stored in the xmax fields of tuples that predate relfrozenxid. Those tuples will be hinted-committed. That's not good, but it might not really have much in the way of consequences. *IF* the next VACUUM doesn't get confused by the old XID, then it will prune the tuple then and I think we'll be OK. And I think it won't, because it should just call HeapTupleSatisfiesVacuum() and that should see that HEAP_XMAX_COMMITTED is set and not actually try to consult the old CLOG. If that hint bit can ever get lost - or fail to propagate to a standby - then we have more trouble, but the fact that it's set by a logged operation makes me hope that can't happen. Furthermore, that follow-on VACUUM should indeed arrive in due time, because we will not have marked the page all-visible -- HeapTupleSatisfiesVacuum() will NOT have returned HEAPTUPLE_LIVE when called from lazy_scan_heap(), and therefore we will have set all_visible = false. The second commit (22576734b805fb0952f9be841ca8f643694ee868) is where I think things get a lot more dangerous. The problem (as Andres pointed out to me this afternoon) is that it seems possible to end up with a situation where there should be two HOT chains on a page, and because of the weakened xmin/xmax checking rules, we end up thinking that the second one is a continuation of the first one, which will be all kinds of bad news. That would be particularly likely to happen in releases from before we invented HEAP_XMIN_FROZEN, when there's no xmin/xmax matching at all, but could happen on later releases if we are extraordinarily unlucky (i.e. xmin of the first tuple in the second chain happens to be the same as the pre-freeze xmax in the old chain, probably because the same XID was used to update the page in two consecutive epochs). Fortunately, that commit is (I think) not released anywhere. Personally, I think it would be best to push the release out a week. I think we understand this well enough now that we can fix it relatively easily, but haste makes bugs, and (I know you're all tired of hearing me say this) patches that implicate the on-disk format are scary. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: doc: Clarify pgstattuple privileges information
doc: Clarify pgstattuple privileges information The description has gotten a bit confusing over time, so rewrite the paragraph a bit. Reported-by: Feike Steenbergen Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/0f53934164d37682fd6a6d87d57008f9ca03e3d0 Modified Files -- doc/src/sgml/pgstattuple.sgml | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Teach planner to account for HAVING quals in aggregation plan no
Teach planner to account for HAVING quals in aggregation plan nodes. For some reason, we have never accounted for either the evaluation cost or the selectivity of filter conditions attached to Agg and Group nodes (which, in practice, are always conditions from a HAVING clause). Applying our regular selectivity logic to post-grouping conditions is a bit bogus, but it's surely better than taking the selectivity as 1.0. Perhaps someday the extended-statistics mechanism can be taught to provide statistics that would help us in getting non-default estimates here. Per a gripe from Benjamin Coutu. This is surely a bug fix, but I'm hesitant to back-patch because of the prospect of destabilizing existing plan choices. Given that it took us this long to notice the bug, it's probably not hurting too many people in the field. Discussion: https://postgr.es/m/20968.1509486...@sss.pgh.pa.us Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/7b6c07547190f056b0464098bb5a2247129d7aa2 Modified Files -- src/backend/optimizer/path/costsize.c | 46 +- src/backend/optimizer/prep/prepunion.c | 2 ++ src/backend/optimizer/util/pathnode.c | 25 ++ src/include/optimizer/cost.h | 2 ++ 4 files changed, 74 insertions(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
Stephen Frost wrote: > * Tom Lane (t...@sss.pgh.pa.us) wrote: > > Andres Freund writes: > > > Do we care about people upgrading to unreleased versions? We could do > > > nothing, document it in the release notes, or ??? > > > > Do nothing. > > Agreed. Not much we can do there. Pushed the reverts. I noticed while doing so that REL_10_STABLE contains the bogus commits. Does that change our opinion regarding what to do for people upgrading to a version containing the broken commits? I don't think so, because 1) we hope that not many people will trust their data to 10.0 immediately after release 2) the bug is very low probability 3) it doesn't look like we can do a lot about it anyway. I'll experiment with Andres' proposed fix now. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Revert bogus fixes of HOT-freezing bug
Revert bogus fixes of HOT-freezing bug It turns out we misdiagnosed what the real problem was. Revert the previous changes, because they may have worse consequences going forward. A better fix is forthcoming. The simplistic test case is kept, though disabled. Discussion: https://postgr.es/m/20171102112019.33wb7g5wp4zpj...@alap3.anarazel.de Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/08ba67d596a1443c32d0b5300aaad8042729cd41 Modified Files -- src/backend/access/heap/heapam.c | 109 +++--- src/backend/access/heap/pruneheap.c | 4 +- src/backend/commands/vacuumlazy.c | 20 +++ src/backend/executor/execMain.c | 6 +- src/include/access/heapam.h | 3 - src/test/isolation/isolation_schedule | 1 - 6 files changed, 38 insertions(+), 105 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Revert bogus fixes of HOT-freezing bug
Revert bogus fixes of HOT-freezing bug It turns out we misdiagnosed what the real problem was. Revert the previous changes, because they may have worse consequences going forward. A better fix is forthcoming. The simplistic test case is kept, though disabled. Discussion: https://postgr.es/m/20171102112019.33wb7g5wp4zpj...@alap3.anarazel.de Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/7a95966bc03cd8af08825de235ea896d682d62bb Modified Files -- src/backend/access/heap/heapam.c | 109 +++--- src/backend/access/heap/pruneheap.c | 4 +- src/backend/commands/vacuumlazy.c | 20 +++ src/backend/executor/execMain.c | 6 +- src/include/access/heapam.h | 3 - src/test/isolation/isolation_schedule | 1 - 6 files changed, 38 insertions(+), 105 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Revert bogus fixes of HOT-freezing bug
Revert bogus fixes of HOT-freezing bug It turns out we misdiagnosed what the real problem was. Revert the previous changes, because they may have worse consequences going forward. A better fix is forthcoming. The simplistic test case is kept, though disabled. Discussion: https://postgr.es/m/20171102112019.33wb7g5wp4zpj...@alap3.anarazel.de Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/c6764eb3aea63f3f95582bd660785e2b0d4439f9 Modified Files -- src/backend/access/heap/heapam.c | 109 +++--- src/backend/access/heap/pruneheap.c | 4 +- src/backend/commands/vacuumlazy.c | 20 +++ src/backend/executor/execMain.c | 6 +- src/include/access/heapam.h | 3 - src/test/isolation/isolation_schedule | 1 - 6 files changed, 38 insertions(+), 105 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Revert bogus fixes of HOT-freezing bug
Revert bogus fixes of HOT-freezing bug It turns out we misdiagnosed what the real problem was. Revert the previous changes, because they may have worse consequences going forward. A better fix is forthcoming. The simplistic test case is kept, though disabled. Discussion: https://postgr.es/m/20171102112019.33wb7g5wp4zpj...@alap3.anarazel.de Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/b3888b60d3f04a27ceaf68cb60e2c2d100c6b753 Modified Files -- src/backend/access/heap/heapam.c | 105 +++--- src/backend/access/heap/pruneheap.c | 4 +- src/backend/commands/vacuumlazy.c | 16 +++--- src/backend/executor/execMain.c | 6 +- src/include/access/heapam.h | 3 - src/test/isolation/isolation_schedule | 1 - 6 files changed, 36 insertions(+), 99 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Revert bogus fixes of HOT-freezing bug
Revert bogus fixes of HOT-freezing bug It turns out we misdiagnosed what the real problem was. Revert the previous changes, because they may have worse consequences going forward. A better fix is forthcoming. The simplistic test case is kept, though disabled. Discussion: https://postgr.es/m/20171102112019.33wb7g5wp4zpj...@alap3.anarazel.de Branch -- REL9_3_STABLE Details --- https://git.postgresql.org/pg/commitdiff/f05ae2fa94b4e8c8fae4ccbc8d79cfbaa6a0e7b2 Modified Files -- src/backend/access/heap/heapam.c | 135 -- src/backend/access/heap/pruneheap.c | 4 +- src/backend/commands/vacuumlazy.c | 16 ++-- src/backend/executor/execMain.c | 6 +- src/include/access/heapam.h | 3 - src/test/isolation/isolation_schedule | 1 - 6 files changed, 60 insertions(+), 105 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Revert bogus fixes of HOT-freezing bug
Revert bogus fixes of HOT-freezing bug It turns out we misdiagnosed what the real problem was. Revert the previous changes, because they may have worse consequences going forward. A better fix is forthcoming. The simplistic test case is kept, though disabled. Discussion: https://postgr.es/m/20171102112019.33wb7g5wp4zpj...@alap3.anarazel.de Branch -- REL9_4_STABLE Details --- https://git.postgresql.org/pg/commitdiff/ef0339ee5dcf933949f3751d98771e03b8c19aa2 Modified Files -- src/backend/access/heap/heapam.c | 143 +++--- src/backend/access/heap/pruneheap.c | 4 +- src/backend/commands/vacuumlazy.c | 16 ++-- src/backend/executor/execMain.c | 6 +- src/include/access/heapam.h | 3 - src/test/isolation/isolation_schedule | 1 - 6 files changed, 60 insertions(+), 113 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Andres Freund writes: > > Do we care about people upgrading to unreleased versions? We could do > > nothing, document it in the release notes, or ??? > > Do nothing. Agreed. Not much we can do there. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
Andres Freund writes: > Do we care about people upgrading to unreleased versions? We could do > nothing, document it in the release notes, or ??? Do nothing. regards, tom lane -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: doc: Adjust name in acknowledgments
doc: Adjust name in acknowledgments per request of the named person Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/d8c435e1743773eba4e36498479ca6aef28a2d70 Modified Files -- doc/src/sgml/release-10.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: doc: Adjust name in acknowledgments
doc: Adjust name in acknowledgments per request of the named person Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/769756fb74d6a183442ae3a3b5ce0816226ce64b Modified Files -- doc/src/sgml/release-10.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
Hi, On 2017-11-02 13:49:47 +0100, Alvaro Herrera wrote: > Andres Freund wrote: > > I think the problem is on the pruning, rather than the freezing side. We > > can't freeze a tuple if it has an alive predecessor - rather than > > weakining this, we should be fixing the pruning to not have the alive > > predecessor. > > I gave a look at HTSV back then, but I didn't find what the right tweak > was, but then I only tried changing the return value to DEAD and > DELETE_IN_PROGRESS; the thought of selecting DEAD or RECENTLY_DEAD based > on OldestXmin didn't occur to me ... I was thinking that the fact that > there were live lockers meant that the tuple could not be removed, > obviously failing to notice that the subsequent versions of the tuple > would be good enough. I'll try to write up a commit based on that idea. I think there's some comment work needed too, Robert and I were both confused by a few things. I'm unfortunately travelling atm - it's evening here, and I'll flying back to the US all Saturday. I'm fairly sure I'll be able to come up with a decent patch tomorrow, but I'll need review and testing by others. > > If the update xmin is actually below the cutoff we can remove the tuple > > even if there's live lockers - the lockers will also be present in the > > newer version of the tuple. I verified that for me that fixes the > > problem. Obviously that'd require some comment work and more careful > > diagnosis. > > Sounds good. > > I'll revert those commits then, keeping the test, and then you can > commit your change. OK? Generally that sounds good - but you can't keep the testcase in without the new fix - the buildfarm would immediately turn red. I guess the best thing would be to temporarily remove it from the schedule? Do we care about people upgrading to unreleased versions? We could do nothing, document it in the release notes, or ??? Greetings, Andres Freund -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
Andres Freund wrote: > I spent some time discussing this with Robert today (with both of us > alternating between feeling the other and ourselves as stupid), and the > conclusion I think is that the problem is on the pruning, rather than > the freezing side. Thanks both for spending some more time on this. > I think the problem is on the pruning, rather than the freezing side. We > can't freeze a tuple if it has an alive predecessor - rather than > weakining this, we should be fixing the pruning to not have the alive > predecessor. I gave a look at HTSV back then, but I didn't find what the right tweak was, but then I only tried changing the return value to DEAD and DELETE_IN_PROGRESS; the thought of selecting DEAD or RECENTLY_DEAD based on OldestXmin didn't occur to me ... I was thinking that the fact that there were live lockers meant that the tuple could not be removed, obviously failing to notice that the subsequent versions of the tuple would be good enough. > If the update xmin is actually below the cutoff we can remove the tuple > even if there's live lockers - the lockers will also be present in the > newer version of the tuple. I verified that for me that fixes the > problem. Obviously that'd require some comment work and more careful > diagnosis. Sounds good. I'll revert those commits then, keeping the test, and then you can commit your change. OK? -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
On Thu, Nov 2, 2017 at 4:50 PM, Andres Freund wrote: > I think a5736bf754c82d8b86674e199e232096c679201d might be dangerous in > the face of previously corrupted tuple chains and pg_upgraded clusters - > it can lead to tuples being considered related, even though they they're > from entirely independent hot chains. Especially when upgrading 9.3 post > your fix, to current releases. I think this is a key point. If the new behavior were merely not entirely correct, we could perhaps refine it later. But it's not only not correct - it actually has the potential to create new problems that didn't exist before those commits. And if we release without reverting those commits then we can't change our mind later. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
Hi,
On 2017-09-28 14:47:53 +, Alvaro Herrera wrote:
> Fix freezing of a dead HOT-updated tuple
>
> Vacuum calls page-level HOT prune to remove dead HOT tuples before doing
> liveness checks (HeapTupleSatisfiesVacuum) on the remaining tuples. But
> concurrent transaction commit/abort may turn DEAD some of the HOT tuples
> that survived the prune, before HeapTupleSatisfiesVacuum tests them.
> This happens to activate the code that decides to freeze the tuple ...
> which resuscitates it, duplicating data.
>
> (This is especially bad if there's any unique constraints, because those
> are now internally violated due to the duplicate entries, though you
> won't know until you try to REINDEX or dump/restore the table.)
>
> One possible fix would be to simply skip doing anything to the tuple,
> and hope that the next HOT prune would remove it. But there is a
> problem: if the tuple is older than freeze horizon, this would leave an
> unfrozen XID behind, and if no HOT prune happens to clean it up before
> the containing pg_clog segment is truncated away, it'd later cause an
> error when the XID is looked up.
>
> Fix the problem by having the tuple freezing routines cope with the
> situation: don't freeze the tuple (and keep it dead). In the cases that
> the XID is older than the freeze age, set the HEAP_XMAX_COMMITTED flag
> so that there is no need to look up the XID in pg_clog later on.
I think this is the wrong fix - the assumption that ctid chains can be
validated based on the prev-xmax = cur-xmin is fairly ingrained into the
system, and we shouldn't just be breaking it. The need to later
lobotomize the checks, in a5736bf754, is some evidence of that.
I spent some time discussing this with Robert today (with both of us
alternating between feeling the other and ourselves as stupid), and the
conclusion I think is that the problem is on the pruning, rather than
the freezing side.
FWIW, I don't think the explanation in the commit message of how the
problem triggers is actually correct - the testcase you added doesn't
have any transactions concurrently committing / aborting when
crashing. Rather the problem is that the liveliness checks for freezing
is different from the ones for pruning. HTSV considers xmin
RECENTLY_DEAD when there's a multi xmax with at least one alive locker,
whereas pruning thinks it has to do something because there's the member
xid below the cutoff. No concurrent activity is needed to trigger that.
I think the problem is on the pruning, rather than the freezing side. We
can't freeze a tuple if it has an alive predecessor - rather than
weakining this, we should be fixing the pruning to not have the alive
predecessor.
The relevant logic in HTSV is:
if (tuple->t_infomask & HEAP_XMAX_IS_MULTI)
{
TransactionId xmax;
if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple),
false))
{
/* already checked above */
Assert(!HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask));
xmax = HeapTupleGetUpdateXid(tuple);
/* not LOCKED_ONLY, so it has to have an xmax */
Assert(TransactionIdIsValid(xmax));
if (TransactionIdIsInProgress(xmax))
return HEAPTUPLE_DELETE_IN_PROGRESS;
else if (TransactionIdDidCommit(xmax))
/* there are still lockers around -- can't
return DEAD here */
return HEAPTUPLE_RECENTLY_DEAD;
/* updating transaction aborted */
return HEAPTUPLE_LIVE;
with the problematic branch being the TransactionIdDidCommit()
case. Rather than unconditionally returning HEAPTUPLE_RECENTLY_DEAD, we
should test the update xid against OldestXmin and return DEAD /
RECENTLY_DEAD according to that.
If the update xmin is actually below the cutoff we can remove the tuple
even if there's live lockers - the lockers will also be present in the
newer version of the tuple. I verified that for me that fixes the
problem. Obviously that'd require some comment work and more careful
diagnosis.
I think a5736bf754c82d8b86674e199e232096c679201d might be dangerous in
the face of previously corrupted tuple chains and pg_upgraded clusters -
it can lead to tuples being considered related, even though they they're
from entirely independent hot chains. Especially when upgrading 9.3 post
your fix, to current releases.
In short, I think the two commits should be reverted, and replaced with
a fix along what I'm outlining above.
There'll be some trouble for people that upgraded to an unreleased
version, but I don't really see what we could do about that.
I could be entirely wrong - I've been travelling for the last two weeks
and my brain is somewhat more fried than usual.
Regards,
Andres
--
Sent via pgsql-committers mailing list (pgsql-committers@
