25 Commits (d90aa68c96a8c4a888e019b92b9448049cbd0811)

Author	SHA1	Message	Date
Robert Haas	e3363cda9c	amcheck: In verify_heapam, allows tuples with xmin 0. ... Commit e88754a196 caused that case to be reported as corruption, but Peter Geoghegan pointed out that it can legitimately happen in the case of a speculative insertion that aborts, so we'd better not flag it as corruption after all. Back-patch to v14, like the commit that introduced the issue. Discussion: http://postgr.es/m/CAH2-WzmEabzcPTxSY-NXKH6Qt3FkAPYHGQSe2PtvGgj17ZQkCw@mail.gmail.com	3 years ago
Robert Haas	8fd5aa76c3	amcheck: Fix verify_heapam for tuples where xmin or xmax is 0. ... In such cases, get_xid_status() doesn't set its output parameter (the third argument), so we shouldn't fall through to code which will test the value of that parameter. There are five existing calls to get_xid_status(), three of which seem to already handle this case properly. This commit tries to fix the other two. If we're checking xmin and find that it is invalid (i.e. 0) just report that as corruption, similar to what's already done in the three cases that seem correct. If we're checking xmax and find that's invalid, that's fine: it just means that the tuple hasn't been updated or deleted. Thanks to Andres Freund and valgrind for finding this problem, and also to Andres for having a look at the patch. This bug seems to go all the way back to where verify_heapam was first introduced, but wasn't detected until recently, possibly because of the new test cases added for update chain verification. Back-patch to v14, where this code showed up. Discussion: http://postgr.es/m/CA+TgmoZAYzQZqyUparXy_ks3OEOfLD9-bEXt8N-2tS1qghX9gQ@mail.gmail.com	3 years ago
Andres Freund	b3a83055c2	amcheck: Fix FullTransactionIdFromXidAndCtx() for xids before epoch 0 ... 64bit xids can't represent xids before epoch 0 (see also be504a3e97). When FullTransactionIdFromXidAndCtx() was passed such an xid, it'd create a 64bit xid far into the future. Noticed while adding assertions in the course of investigating be504a3e97, as amcheck's test create such xids. To fix the issue, just return FirstNormalFullTransactionId in this case. A freshly initdb'd cluster already has a newer horizon. The most minimal version of this would make the messages for some detected corruptions differently inaccurate. To make those cases accurate, switch FullTransactionIdFromXidAndCtx() to use the 32bit modulo difference between xid and nextxid to compute the 64bit xid, yielding sensible "in the future" / "in the past" answers. Reviewed-by: Mark Dilger <mark.dilger@enterprisedb.com> Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de Backpatch: 14-, where heapam verification was introduced	3 years ago
Andres Freund	a42f515d6b	amcheck: Fix ordering bug in update_cached_xid_range() ... The initialization order in update_cached_xid_range() was wrong, calling FullTransactionIdFromXidAndCtx() before setting ->next_xid. FullTransactionIdFromXidAndCtx() uses ->next_xid. In most situations this will not cause visible issues, because the next call to update_cached_xid_range() will use a less wrong ->next_xid. It's rare that xids advance fast enough for this to be a problem. Found while adding more asserts to the 64bit xid infrastructure. Reviewed-by: Mark Dilger <mark.dilger@enterprisedb.com> Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de Backpatch: 14-, where heapam verification was introduced	3 years ago
Peter Geoghegan	e7712155ea	amcheck: Skip unlogged relations in Hot Standby. ... Have verify_heapam.c treat unlogged relations as if they were simply empty when in Hot Standby mode. This brings it in line with verify_nbtree.c, which has handled unlogged relations in the same way since bugfix commit 6754fe65a4. This was an oversight in commit 866e24d47d, which extended contrib/amcheck to check heap relations. In passing, lower the verbosity used when reporting that a relation has been skipped like this, from NOTICE to DEBUG1. This is appropriate because the skipping behavior is only an implementation detail, needed to work around the fact that unlogged tables don't have smgr-level storage for their main fork when in Hot Standby mode. Affected unlogged relations should be considered "trivially verified", not skipped over. They are verified in the same sense that a totally empty relation can be verified. This behavior seems least surprising overall, since unlogged relations on a replica will initially be empty if and when the replica is promoted and Hot Standby ends. Author: Mark Dilger <mark.dilger@enterprisedb.com> Reviewed-By: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAH2-Wzk_pukOFY7JmdiFLsrz+Pd3V8OwgC1TH2Vd5BH5ZgK4bA@mail.gmail.com Backpatch: 14-, where heapam verification was introduced.	4 years ago
Peter Geoghegan	ba05dfe943	contrib/amcheck: Add heapam CHECK_FOR_INTERRUPTS(). ... Add a CHECK_FOR_INTERRUPTS() call to make heap relation verification responsive to query cancellations. Author: Mark Dilger <mark.dilger@enterprisedb.com> Discussion: https://postgr.es/m/CAH2-Wzk-9RtQgb2QiuLv8j2O0j9tSFKPmmch5nWSZhguUxvbrw%40mail.gmail.com Backpatch: 14-, where amcheck heap verification was introduced.	4 years ago
Peter Eisentraut	97b7134186	amcheck: Fix code comments ... Code comments were claiming that verify_heapam() was checking privileges on the relation it was operating on, but it didn't actually do that. Perhaps earlier versions of the patch did that, but now the access is regulated by privileges on the function. Remove the wrong comments.	5 years ago
Tom Lane	def5b065ff	Initial pgindent and pgperltidy run for v14. ... Also "make reformat-dat-files". The only change worthy of note is that pgindent messed up the formatting of launcher.c's struct LogicalRepWorkerId, which led me to notice that that struct wasn't used at all anymore, so I just took it out.	5 years ago
Robert Haas	50529e5b4e	amcheck: Improve some confusing reports about TOAST problems. ... Don't phrase reports in terms of the number of tuples thus-far returned by the index scan, but rather in terms of the chunk_seq values found inside the tuples. Patch by me, reviewed by Mark Dilger. Discussion: http://postgr.es/m/CA+TgmoZUONCkdcdR778EKuE+f1r5Obieu63db2OgMPHaEvEPTQ@mail.gmail.com	5 years ago
Michael Paquier	7ef8b52cf0	Fix typos and grammar in comments and docs ... Author: Justin Pryzby Discussion: https://postgr.es/m/20210416070310.GG3315@telsasoft.com	5 years ago
Peter Eisentraut	59da8d9eb7	amcheck: Use correct format placeholder for TOAST chunk numbers ... Several of these were already fixed in passing in 9acaf1a621, but one was remaining inconsistent.	5 years ago
Robert Haas	9acaf1a621	amcheck: Reword some messages and fix an alignment problem. ... We don't need to mention the attribute number in these messages, because there's a dedicated column for that, but we should mention the toast value ID, because that's really useful for any follow-up troubleshooting the user wants to do. This also rewords some of the messages to hopefully read a little better. Also, use VARATT_EXTERNAL_GET_POINTER in case we're accessing a TOAST pointer that isn't aligned on a platform that's fussy about alignment, so that we don't crash while corruption-checking the user's data. Mark Dilger, reviewed by me. Discussion: http://postgr.es/m/7D3B9BF6-50D0-4C30-8506-1C1851C7F96F@enterprisedb.com	5 years ago
Robert Haas	ec7ffb8096	amcheck: fix multiple problems with TOAST pointer validation ... First, don't perform database access while holding a buffer lock. When checking a heap, we can validate that TOAST pointers are sane by performing a scan on the TOAST index and looking up the chunks that correspond to each value ID that appears in a TOAST poiner in the main table. But, to do that while holding a buffer lock at least risks causing other backends to wait uninterruptibly, and probably can cause undetected and uninterruptible deadlocks. So, instead, make a list of checks to perform while holding the lock, and then perform the checks after releasing it. Second, adjust things so that we don't try to follow TOAST pointers for tuples that are already eligible to be pruned. The TOAST tuples become eligible for pruning at the same time that the main tuple does, so trying to check them may lead to spurious reports of corruption, as observed in the buildfarm. The necessary infrastructure to decide whether or not the tuple being checked is prunable was added by commit 3b6c1259f9, but it wasn't actually used for its intended purpose prior to this patch. Mark Dilger, adjusted by me to avoid a memory leak. Discussion: http://postgr.es/m/AC5479E4-6321-473D-AC92-5EC36299FBC2@enterprisedb.com	5 years ago
Robert Haas	4573f6a9af	amcheck: Remove duplicate XID/MXID bounds checks. ... Commit 3b6c1259f9 resulted in the same xmin and xmax bounds checking being performed in both check_tuple() and check_tuple_visibility(). Remove the duplication. While at it, adjust some code comments that were overlooked in that commit. Mark Dilger Discussion: http://postgr.es/m/AC5479E4-6321-473D-AC92-5EC36299FBC2@enterprisedb.com	5 years ago
Fujii Masao	98e5bd103f	Fix typos in comments. ... Author: Masahiko Sawada Discussion: https://postgr.es/m/CAD21AoA1YL7t0nzVSEySx6zOaE7xO3r0jyu8hkitGL2_XbaMxQ@mail.gmail.com	5 years ago
Robert Haas	3b6c1259f9	amcheck: Fix verify_heapam's tuple visibility checking rules. ... We now follow the order of checks from HeapTupleSatisfies* more closely to avoid coming to erroneous conclusions. Mark Dilger and Robert Haas Discussion: http://postgr.es/m/CA+Tgmob6sii0yTvULYJ0Vq4w6ZBmj7zUhddL3b+SKDi9z9NA7Q@mail.gmail.com	5 years ago
Robert Haas	bbe0a81db6	Allow configurable LZ4 TOAST compression. ... There is now a per-column COMPRESSION option which can be set to pglz (the default, and the only option in up until now) or lz4. Or, if you like, you can set the new default_toast_compression GUC to lz4, and then that will be the default for new table columns for which no value is specified. We don't have lz4 support in the PostgreSQL code, so to use lz4 compression, PostgreSQL must be built --with-lz4. In general, TOAST compression means compression of individual column values, not the whole tuple, and those values can either be compressed inline within the tuple or compressed and then stored externally in the TOAST table, so those properties also apply to this feature. Prior to this commit, a TOAST pointer has two unused bits as part of the va_extsize field, and a compessed datum has two unused bits as part of the va_rawsize field. These bits are unused because the length of a varlena is limited to 1GB; we now use them to indicate the compression type that was used. This means we only have bit space for 2 more built-in compresison types, but we could work around that problem, if necessary, by introducing a new vartag_external value for any further types we end up wanting to add. Hopefully, it won't be too important to offer a wide selection of algorithms here, since each one we add not only takes more coding but also adds a build dependency for every packager. Nevertheless, it seems worth doing at least this much, because LZ4 gets better compression than PGLZ with less CPU usage. It's possible for LZ4-compressed datums to leak into composite type values stored on disk, just as it is for PGLZ. It's also possible for LZ4-compressed attributes to be copied into a different table via SQL commands such as CREATE TABLE AS or INSERT .. SELECT. It would be expensive to force such values to be decompressed, so PostgreSQL has never done so. For the same reasons, we also don't force recompression of already-compressed values even if the target table prefers a different compression method than was used for the source data. These architectural decisions are perhaps arguable but revisiting them is well beyond the scope of what seemed possible to do as part of this project. However, it's relatively cheap to recompress as part of VACUUM FULL or CLUSTER, so this commit adjusts those commands to do so, if the configured compression method of the table happens not to match what was used for some column value stored therein. Dilip Kumar. The original patches on which this work was based were written by Ildus Kurbangaliev, and those were patches were based on even earlier work by Nikita Glukhov, but the design has since changed very substantially, since allow a potentially large number of compression methods that could be added and dropped on a running system proved too problematic given some of the architectural issues mentioned above; the choice of which specific compression method to add first is now different; and a lot of the code has been heavily refactored. More recently, Justin Przyby helped quite a bit with testing and reviewing and this version also includes some code contributions from him. Other design input and review from Tomas Vondra, Álvaro Herrera, Andres Freund, Oleg Bartunov, Alexander Korotkov, and me. Discussion: http://postgr.es/m/20170907194236.4cefce96%40wp.localdomain Discussion: http://postgr.es/m/CAFiTN-uUpX3ck%3DK0mLEk-G_kUQY%3DSNOTeqdaNRR9FMdQrHKebw%40mail.gmail.com	5 years ago
Robert Haas	4078ce65a0	Fix a confusing amcheck corruption message. ... Don't complain about the last TOAST chunk number being different from what we expected if there are no TOAST chunks at all. In such a case, saying that the final chunk number is 0 is not really accurate, and the fact the value is missing from the TOAST table is reported separately anyway. Mark Dilger Discussion: http://postgr.es/m/AA5506CE-7D2A-42E4-A51D-358635E3722D@enterprisedb.com	5 years ago
Alvaro Herrera	8deb6b38dc	Reinstate HEAP_XMAX_LOCK_ONLY|HEAP_KEYS_UPDATED as allowed ... Commit 866e24d47d added an assert that HEAP_XMAX_LOCK_ONLY and HEAP_KEYS_UPDATED cannot appear together, on the faulty assumption that the latter necessarily referred to an update and not a tuple lock; but that's wrong, because SELECT FOR UPDATE can use precisely that combination, as evidenced by the amcheck test case added here. Remove the Assert(), and also patch amcheck's verify_heapam.c to not complain if the combination is found. Also, out of overabundance of caution, update (across all branches) README.tuplock to be more explicit about this. Author: Julien Rouhaud <rjuju123@gmail.com> Reviewed-by: Mahendra Singh Thalor <mahi6run@gmail.com> Reviewed-by: Dilip Kumar <dilipbalaut@gmail.com> Discussion: https://postgr.es/m/20210124061758.GA11756@nol	5 years ago
Bruce Momjian	ca3b37487b	Update copyright for 2021 ... Backpatch-through: 9.5	5 years ago
Michael Paquier	788dd0b839	Fix some typos ... Author: Daniel Gustafsson Discussion: https://postgr.es/m/C36ADFDF-D09A-4EE5-B186-CB46C3653F4C@yesql.se	5 years ago
Michael Paquier	8a15e735be	Fix some grammar and typos in comments and docs ... The documentation fixes are backpatched down to where they apply. Author: Justin Pryzby Discussion: https://postgr.es/m/20201031020801.GD3080@telsasoft.com Backpatch-through: 9.6	5 years ago
Tom Lane	321633e17b	Fix more portability issues in new amcheck code. ... verify_heapam() wasn't being careful to sanity-check tuple line pointers before using them, resulting in SIGBUS on alignment-picky architectures. Fix that, add some more test coverage. Mark Dilger, some tweaking by me Discussion: https://postgr.es/m/30B8E99A-2D9C-48D4-A55C-741C9D5F1563@enterprisedb.com	5 years ago
Robert Haas	8bb0c9770e	Try to avoid a compiler warning about using fxid uninitialized. ... Mark Dilger, with a couple of stray semicolons removed by me. Discussion: http://postgr.es/m/2A7DA1A8-C4AA-43DF-A985-3CA52F4DC775@enterprisedb.com	5 years ago
Robert Haas	866e24d47d	Extend amcheck to check heap pages. ... Mark Dilger, reviewed by Peter Geoghegan, Andres Freund, Álvaro Herrera, Michael Paquier, Amul Sul, and by me. Some last-minute cosmetic revisions by me. Discussion: http://postgr.es/m/12ED3DA8-25F0-4B68-937D-D907CFBF08E7@enterprisedb.com	5 years ago