Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1o37dt-00061V-8p for pgsql-hackers@arkaria.postgresql.org; Mon, 20 Jun 2022 02:59:57 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1o37dr-0007N2-Mt for pgsql-hackers@arkaria.postgresql.org; Mon, 20 Jun 2022 02:59:55 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1o37dr-0007Ms-A5 for pgsql-hackers@lists.postgresql.org; Mon, 20 Jun 2022 02:59:55 +0000 Received: from mail-yw1-x112a.google.com ([2607:f8b0:4864:20::112a]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1o37do-00064C-Nn for pgsql-hackers@lists.postgresql.org; Mon, 20 Jun 2022 02:59:54 +0000 Received: by mail-yw1-x112a.google.com with SMTP id 00721157ae682-317741c86fdso85391177b3.2 for ; Sun, 19 Jun 2022 19:59:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=O6Fv+CtYqy0H1D4uP0VDLg944dzomUjGZK1L88pOdx0=; b=FU/ejmJsRNnrPEC8ZyXSElrV3RDash4qmhGfPBWRm5Z+F9ZHEAyjJGAed6JG2jOAsL f1Mg7z1VcGFxq5b3FPwOsCyFEf4aPC48f7sur9ntILfpzfGb91wewLnSG97Lg05Ts8bn pfHXSPFvlkrKvQHOHZ1vNVc1W7b7JoN/DkxvIqZfH1wa0cOM2QJPO7VGEHhkm6NU8acJ 0P7sD1QTa5WOhWBpE/0Ml/vDNaL7dWLxFTO+kCusNb1mq0lVvPnycoTRA2WA1Ta4aVwf wtEPJwXV7MXLNZErzH/mBN12B3Ap8A4LK3nzU5M4okVOKPuyUma7VdZkrIB7ebsC+5UN Vwwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=O6Fv+CtYqy0H1D4uP0VDLg944dzomUjGZK1L88pOdx0=; b=T31WE5q58irzOuz4bEDNNGQn3UT4viEsKS8UBVGFRNt/g4dHWtwOJnjzK2RqfNxBX7 cyCkavKWEeaPrGas8AW3m5EbNeFL/vj54KgUGyPBKqDKT3G7LzoQPvvKsLUSoMT+IhdW NXQeibPld8prT+tnr7g4jVl4cToUljTzzGkb50goY3OGYNs357yf2mM04g3vzQe1ReNI m1EvmHxWmJoj+bH8fwZX2jNq/LNuDkFiqRjVYfqD+++R862uzdcToXTZ5ZthbLeyQTVh Gc+/2Q2jldrlSCsPT0wn6hoaO1M5F9eWvXGDtR8ndtyGSjgKWSUBeJtAWoekUjTPlT6X ow1Q== X-Gm-Message-State: AJIora/tpQPdOfOj/+eCU/++oF0r2Ycf8avV5Tl9BVB8/zBFxIbsIU6A RCs8LX7AWxQPdkGixWC/YWPBIfLAZ4TXxauYHTk= X-Google-Smtp-Source: AGRyM1sxUg6qCb5MraCOvQ53wyIujtqFSYOjiXeiq+i6avgVoRtGqgOYjjya2qaIa0fk+bdOUbZLnPdfVPstKCrS47M= X-Received: by 2002:a81:3a83:0:b0:317:82a1:a74 with SMTP id h125-20020a813a83000000b0031782a10a74mr17132949ywa.404.1655693990764; Sun, 19 Jun 2022 19:59:50 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Amit Kapila Date: Mon, 20 Jun 2022 08:29:39 +0530 Message-ID: Subject: Re: Perform streaming logical transactions by background workers and parallel apply To: "wangw.fnst@fujitsu.com" Cc: Masahiko Sawada , Peter Smith , "shiy.fnst@fujitsu.com" , "houzj.fnst@fujitsu.com" , PostgreSQL Hackers Content-Type: multipart/mixed; boundary="0000000000002d7c6805e1d84cdf" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --0000000000002d7c6805e1d84cdf Content-Type: text/plain; charset="UTF-8" On Fri, Jun 17, 2022 at 12:47 PM wangw.fnst@fujitsu.com wrote: > > On Wed, Jun 15, 2022 at 8:13 PM Amit Kapila wrote: > > Few questions/comments on 0001 > > =========================== > Thanks for your comments. > > > 1. > > In the commit message, I see: "We also need to allow stream_stop to > > complete by the apply background worker to avoid deadlocks because > > T-1's current stream of changes can update rows in conflicting order > > with T-2's next stream of changes." > > > > Thinking about this, won't the T-1 and T-2 deadlock on the publisher > > node as well if the above statement is true? > Yes, I think so. > I think if table's unique index/constraint of the publisher and the subscriber > are consistent, the deadlock will occur on the publisher-side. > If it is inconsistent, deadlock may only occur in the subscriber. But since we > added the check for these (see patch 0004), so it seems okay to not handle this > at STREAM_STOP. > > BTW, I made the following improvements to the code (#a, #c are improved in 0004 > patch, #b, #d and #e are improved in 0001 patch.) : > a. > I added some comments in the function apply_handle_stream_stop to explain why > we do not need to allow stream_stop to complete by the apply background worker. > I have improved the comments in this and other related sections of the patch. See attached. > > > > 3. > > + > > + > > + Setting streaming mode to apply could export invalid LSN > > + as finish LSN of failed transaction. Changing the streaming mode and making > > + the same conflict writes the finish LSN of the failed transaction in the > > + server log if required. > > + > > > > How will the user identify that this is an invalid LSN value and she > > shouldn't use it to SKIP the transaction? Can we change the second > > sentence to: "User should change the streaming mode to 'on' if they > > would instead wish to see the finish LSN on error. Users can use > > finish LSN to SKIP applying the transaction." I think we can give > > reference to docs where the SKIP feature is explained. > Improved the sentence as suggested. > You haven't answered first part of the comment: "How will the user identify that this is an invalid LSN value and she shouldn't use it to SKIP the transaction?". Have you checked what value it displays? For example, in one of the case in apply_error_callback as shown in below code, we don't even display finish LSN if it is invalid. else if (XLogRecPtrIsInvalid(errarg->finish_lsn)) errcontext("processing remote data for replication origin \"%s\" during \"%s\" in transaction %u", errarg->origin_name, logicalrep_message_type(errarg->command), errarg->remote_xid); -- With Regards, Amit Kapila. --0000000000002d7c6805e1d84cdf Content-Type: application/octet-stream; name="improve_comments_1.patch" Content-Disposition: attachment; filename="improve_comments_1.patch" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_l4m5dvwq0 ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL2xvZ2ljYWwvd29ya2VyLmMgYi9z cmMvYmFja2VuZC9yZXBsaWNhdGlvbi9sb2dpY2FsL3dvcmtlci5jCmluZGV4IGFjZjY0OTRjZDYu LmU2MjFlYjVlNDQgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL2xvZ2ljYWwv d29ya2VyLmMKKysrIGIvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vbG9naWNhbC93b3JrZXIuYwpA QCAtMzIsNyArMzIsMTIgQEAKICAqIHRyYW5zYWN0aW9uIGNvbW1pdCBpcyByZWNlaXZlZCBhbmQg YWxzbyB3YWl0IGZvciB0aGUgd29ya2VyIHRvIGZpbmlzaCBhdAogICogY29tbWl0LiBUaGlzIHBy ZXNlcnZlcyBjb21taXQgb3JkZXJpbmcgYW5kIGF2b2lkcyB3cml0aW5nIHRvIGFuZCByZWFkaW5n CiAgKiBmcm9tIGZpbGUgaW4gbW9zdCBjYXNlcy4gV2Ugc3RpbGwgbmVlZCB0byBzcGlsbCBpZiB0 aGVyZSBpcyBubyB3b3JrZXIKLSAqIGF2YWlsYWJsZS4KKyAqIGF2YWlsYWJsZS4gSXQgaXMgaW1w b3J0YW50IHRvIG1haW50YWluIGNvbW1pdCBvcmRlciB0byBhdm9pZCBmYWlsdXJlcworICogZHVl IHRvIChhKSB0cmFuc2FjdGlvbiBkZXBlbmRlbmNpZXMsIHNheSBpZiB3ZSBpbnNlcnQgYSByb3cg aW4gdGhlIGZpcnN0CisgKiB0cmFuc2FjdGlvbiBhbmQgdXBkYXRlIGl0IGluIHRoZSBzZWNvbmQg dHJhbnNhY3Rpb24gdGhlbiBhbGxvd2luZyB0byBhcHBseQorICogYm90aCBpbiBwYXJhbGxlbCBj YW4gbGVhZCB0byBmYWlsdXJlIGluIHRoZSB1cGRhdGUuIChiKSBkZWFkbG9ja3MsIGFsbG93aW5n CisgKiB0cmFuc2FjdGlvbnMgdGhhdCB1cGRhdGUgdGhlIHNhbWUgc2V0IG9mIHJvd3MvdGFibGVz IGluIG9wcG9zaXRlIG9yZGVyIHRvIGJlCisgKiBhcHBsaWVkIGluIHBhcmFsbGVsIGNhbiBsZWFk IHRvIGRlYWRsb2Nrcy4KICAqCiAgKiAyKSBXcml0ZSB0byB0ZW1wb3JhcnkgZmlsZXMgYW5kIGFw cGx5IHdoZW4gdGhlIGZpbmFsIGNvbW1pdCBhcnJpdmVzCiAgKgpAQCAtMTM2NiwxNCArMTM3MSwx MiBAQCBhcHBseV9oYW5kbGVfc3RyZWFtX3N0b3AoU3RyaW5nSW5mbyBzKQogCQljaGFyCQlhY3Rp b24gPSBMT0dJQ0FMX1JFUF9NU0dfU1RSRUFNX1NUT1A7CiAKIAkJLyoKLQkJICogVGhlcmUgaXMg bm8gbmVlZCB0byB3YWl0IGhlcmUgdG8gYWxsb3cgc3RyZWFtX3N0b3AgdG8gY29tcGxldGUgYnkK LQkJICogYmFja2dyb3VuZCB3b3JrZXIgdG8gYXZvaWQgZGVhZGxvY2tzLgotCQkgKgotCQkgKiBU aGUgZGVhZGxvY2sgcHJvYmxlbSBvbmx5IG9jY3VycyBpZiByZWxhdGlvbidzIHVuaXF1ZQotCQkg KiBpbmRleC9jb25zdHJhaW50IGlzIGRpZmZlcmVudCBiZXR3ZWVuIHB1Ymxpc2hlciBhbmQgc3Vi c2NyaWJlci4gQnV0Ci0JCSAqIGluIHRoZXNlIGNhc2VzLCB3ZSBkbyBub3QgYWxsb3cgdG8gYXBw bHkgc3RyZWFtZWQgdHJhbnNhY3Rpb24gaW4gdGhlCi0JCSAqIGFwcGx5IGJhY2tncm91bmQgd29y a2VyIChzZWUgZnVuY3Rpb24KLQkJICogYXBwbHlfYmd3b3JrZXJfcmVsYXRpb25fY2hlY2spLgor CQkgKiBVbmxpa2Ugc3RyZWFtX2NvbW1pdCwgd2UgZG9uJ3QgbmVlZCB0byB3YWl0IGhlcmUgZm9y IHN0cmVhbV9zdG9wIHRvCisJCSAqIGZpbmlzaC4gQWxsb3dpbmcgdGhlIG90aGVyIHRyYW5zYWN0 aW9uIHRvIGJlIGFwcGxpZWQgYmVmb3JlIHN0cmVhbV9zdG9wCisJCSAqIGlzIGZpbmlzaGVkIGNh biBvbmx5IGxlYWQgdG8gZmFpbHVyZXMgaWYgdGhlIHVuaXF1ZSBpbmRleC9jb25zdHJhaW50IGlz CisJCSAqIGRpZmZlcmVudCBiZXR3ZWVuIHB1Ymxpc2hlciBhbmQgc3Vic2NyaWJlci4gQnV0IGZv ciBzdWNoIGNhc2VzLCB3ZSBkb24ndAorCQkgKiBhbGxvdyBzdHJlYW1lZCB0cmFuc2FjdGlvbnMg dG8gYmUgYXBwbGllZCBpbiBwYXJhbGxlbC4gU2VlCisJCSAqIGFwcGx5X2Jnd29ya2VyX3JlbGF0 aW9uX2NoZWNrLgogCQkgKi8KIAkJYXBwbHlfYmd3b3JrZXJfc2VuZF9kYXRhKHN0cmVhbV9hcHBs eV93b3JrZXIsIDEsICZhY3Rpb24pOwogCkBAIC0yNzY0LDcgKzI3NjcsMTEgQEAgYXBwbHlfaGFu ZGxlX3N0cmVhbV9jb21taXQoU3RyaW5nSW5mbyBzKQogCQkJLyogU2VuZCBjb21taXQgbWVzc2Fn ZSAqLwogCQkJYXBwbHlfYmd3b3JrZXJfc2VuZF9kYXRhKHdzdGF0ZSwgcy0+bGVuLCBzLT5kYXRh KTsKIAotCQkJLyogV2FpdCBmb3IgYXBwbHkgYmFja2dyb3VuZCB3b3JrZXIgdG8gZmluaXNoICov CisJCQkvKgorCQkJICogV2FpdCBmb3IgYXBwbHkgYmFja2dyb3VuZCB3b3JrZXIgdG8gZmluaXNo LiBUaGlzIGlzIHJlcXVpcmVkIHRvCisJCQkgKiBtYWludGFpbiBjb21taXQgb3JkZXIgd2hpY2gg YXZvaWRzIGZhaWx1cmVzIGR1ZSB0byB0cmFuc2FjdGlvbgorCQkJICogZGVwZW5kZW5jaWVzIGFu ZCBkZWFkbG9ja3MuCisJCQkgKi8KIAkJCWFwcGx5X2Jnd29ya2VyX3dhaXRfZm9yKHdzdGF0ZSwg QVBQTFlfQkdXT1JLRVJfRklOSVNIRUQpOwogCiAJCQlwZ3N0YXRfcmVwb3J0X3N0YXQoZmFsc2Up Owo= --0000000000002d7c6805e1d84cdf--