10 Commits (29ef2b310da9892fda075ff9ee12da7f92d5da6e)

Author	SHA1	Message	Date
Alvaro Herrera	8d1c1ca70b	Reduce cost of test_decoding's new oldest_xmin test ... Change a whole-database VACUUM into doing just pg_attribute, which is the portion that verifies what we want it to do. The original formulation wastes a lot of CPU time, which leads the test to fail when runtime exceeds isolationtester timeout when it's super-slow, such as under CLOBBER_CACHE_ALWAYS. Per buildfarm member friarbird. It turns out that the previous shape of the test doesn't always detect the condition it is supposed to detect (on unpatched reorderbuffer code): the reason is that there is a good chance of encountering a xl_running_xacts record (logged every 15 seconds) before the checkpoint -- and because we advance the xmin when we receive that WAL record, and we *don't* advance the xmin twice consecutively without receiving a client message in between, that means the xmin is not advanced enough for the tuple to be pruned from pg_attribute by VACUUM. So the test would spuriously pass. The reason this test deficiency wasn't detected earlier is that HOT pruning removes the tuple anyway, even if vacuum leaves it in place, so the test correctly fails (detecting the coding mistake), but for the wrong reason. To fix this mess, run the s0_get_changes step twice before vacuum instead of once: this seems to cause the xmin to be advanced reliably, wreaking havoc with more certainty. Author: Arseny Sher Discussion: https://postgr.es/m/87h8lkuxoa.fsf@ars-thinkpad	7 years ago
Alvaro Herrera	f49a80c481	Fix "base" snapshot handling in logical decoding ... Two closely related bugs are fixed. First, xmin of logical slots was advanced too early. During xl_running_xacts processing, xmin of the slot was set to the oldest running xid in the record, but that's wrong: actually, snapshots which will be used for not-yet-replayed transactions might consider older txns as running too, so we need to keep xmin back for them. The problem wasn't noticed earlier because DDL which allows to delete tuple (set xmax) while some another not-yet-committed transaction looks at it is pretty rare, if not unique: e.g. all forms of ALTER TABLE which change schema acquire ACCESS EXCLUSIVE lock conflicting with any inserts. The included test case (test_decoding's oldest_xmin) uses ALTER of a composite type, which doesn't have such interlocking. To deal with this, we must be able to quickly retrieve oldest xmin (oldest running xid among all assigned snapshots) from ReorderBuffer. To fix, add another list of ReorderBufferTXNs to the reorderbuffer, where transactions are sorted by base-snapshot-LSN. This is slightly different from the existing (sorted by first-LSN) list, because a transaction can have an earlier LSN but a later Xmin, if its first record does not obtain an xmin (eg. xl_xact_assignment). Note this new list doesn't fully replace the existing txn list: we still need that one to prevent WAL recycling. The second issue concerns SnapBuilder snapshots and subtransactions. SnapBuildDistributeNewCatalogSnapshot never assigned a snapshot to a transaction that is known to be a subtxn, which is good in the common case that the top-level transaction already has one (no point in doing so), but a bug otherwise. To fix, arrange to transfer the snapshot from the subtxn to its top-level txn as soon as the kinship gets known. test_decoding's snapshot_transfer verifies this. Also, fix a minor memory leak: refcount of toplevel's old base snapshot was not decremented when the snapshot is transferred from child. Liberally sprinkle code comments, and rewrite a few existing ones. This part is my (Álvaro's) contribution to this commit, as I had to write all those comments in order to understand the existing code and Arseny's patch. Reported-by: Arseny Sher <a.sher@postgrespro.ru> Diagnosed-by: Arseny Sher <a.sher@postgrespro.ru> Co-authored-by: Arseny Sher <a.sher@postgrespro.ru> Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Antonin Houska <ah@cybertec.at> Discussion: https://postgr.es/m/87lgdyz1wj.fsf@ars-thinkpad	7 years ago
Peter Eisentraut	325f2ec555	Handle heap rewrites even better in logical decoding ... Logical decoding should not publish anything about tables created as part of a heap rewrite during DDL. Those tables don't exist externally, so consumers of logical decoding cannot do anything sensible with that information. In ab28feae2b, we worked around this for built-in logical replication, but that was hack. This is a more proper fix: We mark such transient heaps using the new field pg_class.relwrite, linking to the original relation OID. By default, we ignore them in logical decoding before they get to the output plugin. Optionally, a plugin can register their interest in getting such changes, if they handle DDL specially, in which case the new field will help them get information about the actual table. Reviewed-by: Craig Ringer <craig@2ndquadrant.com>	8 years ago
Andres Freund	955a684e04	Fix race condition leading to hanging logical slot creation. ... The snapshot assembly during the creation of logical slots relied waiting for transactions in xl_running_xacts to end, by checking for their commit/abort records. Unfortunately, despite locking, it is possible to see an xl_running_xact record listing transactions as ready, that have already WAL-logged an commit/abort record, as the locking just prevents the ProcArray to be adjusted, and the commit record has to be logged first. That lead to either delayed or hanging snapshot creation, because snapbuild.c would wait "forever" to see commit/abort records for some transactions. That hang resolved only if a xl_running_xacts record without any running transactions happened to be logged, far from certain on a busy server. It's impractical to prevent that via more heavyweight locking, the likelihood of deadlocks and significantly increased contention would be too big. Instead change the initial snapshot creation to be solely based on tracking the oldest running transaction via xl_running_xacts->oldestRunningXid - that actually ends up significantly simplifying the code. That has two disadvantages: 1) Because we cannot fully "trust" the contents of xl_running_xacts, we cannot use it to build the initial snapshot. Instead we have to wait twice for all running transactions to finish. 2) Previously a slot, unless the race occurred, could be created when the all transaction perceived as running based on commit/abort records, now we have to wait for the next xl_running_xacts record. To address that, trigger logging new xl_running_xacts record from within snapbuild.c exactly when necessary. Unfortunately snabuild.c's SnapBuild is stored on disk, one of the stupider ideas of a certain Mr Freund, so we can't change it in a minor release. As this is going to be backpatched, we have to hack around a bit to keep on-disk compatibility. A later commit will rejigger that on master. Author: Andres Freund, based on a quite different patch from Petr Jelinek Analyzed-By: Petr Jelinek Reviewed-By: Petr Jelinek Discussion: https://postgr.es/m/f37e975c-908f-858e-707f-058d3b1eb214@2ndquadrant.com Backpatch: 9.4-, where logical decoding has been introduced	8 years ago
Andres Freund	1986c3c440	Force synchronous_commit=on in test_decoding's concurrent_ddl_dml.spec. ... Otherwise running installcheck-force on a server with synchronous_commit=off will result in the tests failing. All the other tests already do so... Backpatch: 9.4, where logical decoding was added	10 years ago
Robert Haas	5243a35825	Remove bogus step from test_decoding isolation tests. ... Commit 43b4a16817 made the isolation tester reject duplicate step names, and it turns out that the test_decoding module's concurrent_ddl_dml isolation test has a duplicate name. I think the second definition isn't actually getting used, so just remove it. Per buildfarm.	10 years ago
Heikki Linnakangas	4fc72cc7bb	Collection of typo fixes. ... Use "a" and "an" correctly, mostly in comments. Two error messages were also fixed (they were just elogs, so no translation work required). Two function comments in pg_proc.h were also fixed. Etsuro Fujita reported one of these, but I found a lot more with grep. Also fix a few other typos spotted while grepping for the a/an typos. For example, "consists out of ..." -> "consists of ...". Plus a "though"/ "through" mixup reported by Euler Taveira. Many of these typos were in old code, which would be nice to backpatch to make future backpatching easier. But much of the code was new, and I didn't feel like crafting separate patches for each branch. So no backpatching.	10 years ago
Andres Freund	ec5896aed3	Fix several weaknesses in slot and logical replication on-disk serialization. ... Heikki noticed in 544E23C0.8090605@vmware.com that slot.c and snapbuild.c were missing the FIN_CRC32 call when computing/checking checksums of on disk files. That doesn't lower the the error detection capabilities of the checksum, but is inconsistent with other usages. In a followup mail Heikki also noticed that, contrary to a comment, the 'version' and 'length' struct fields of replication slot's on disk data where not covered by the checksum. That's not likely to lead to actually missed corruption as those fields are cross checked with the expected version and the actual file length. But it's wrong nonetheless. As fixing these issues makes existing on disk files unreadable, bump the expected versions of on disk files for both slots and logical decoding historic catalog snapshots. This means that loading old files will fail with ERROR: "replication slot file ... has unsupported version 1" and ERROR: "snapbuild state file ... has unsupported version 1 instead of 2" respectively. Given the low likelihood of anybody already using these new features in a production setup that seems acceptable. Fixing these issues made me notice that there's no regression test covering the loading of historic snapshot from disk - so add one. Backpatch to 9.4 where these features were introduced.	11 years ago
Andres Freund	d6fa44fce7	Add skip-empty-xacts option to test_decoding for use in the regression tests. ... The regression tests for contrib/test_decoding regularly failed on postgres instances that were very slow. Either because the hardware itself was slow or because very expensive debugging options like CLOBBER_CACHE_ALWAYS were used. The reason they failed was just that some additional transactions were decoded. Analyze and vacuum, triggered by autovac. To fix just add a option to test_decoding to only display transactions in which a change was actually displayed. That's not pretty because it removes information from the tests; but better than constantly failing tests in very likely harmless ways. Backpatch to 9.4 where logical decoding was introduced. Discussion: 20140629142511.GA26930@awork2.anarazel.de	11 years ago
Robert Haas	b89e151054	Introduce logical decoding. ... This feature, building on previous commits, allows the write-ahead log stream to be decoded into a series of logical changes; that is, inserts, updates, and deletes and the transactions which contain them. It is capable of handling decoding even across changes to the schema of the effected tables. The output format is controlled by a so-called "output plugin"; an example is included. To make use of this in a real replication system, the output plugin will need to be modified to produce output in the format appropriate to that system, and to perform filtering. Currently, information can be extracted from the logical decoding system only via SQL; future commits will add the ability to stream changes via walsender. Andres Freund, with review and other contributions from many other people, including Álvaro Herrera, Abhijit Menon-Sen, Peter Gheogegan, Kevin Grittner, Robert Haas, Heikki Linnakangas, Fujii Masao, Abhijit Menon-Sen, Michael Paquier, Simon Riggs, Craig Ringer, and Steve Singer.	12 years ago