Content-Type: multipart/mixed; boundary="------------P6qpeRkCgbxiSlnCGeNDdnWh"
Message-ID: <b7c8d68f-5bda-4c33-adeb-ac3cef7c77ad@bluegap.ch>
Date: Fri, 21 Feb 2025 09:06:04 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Reset the output buffer after sending from WalSndWriteData
To: Masahiko Sawada <sawada.mshk@gmail.com>,
  Markus Wanner <markus.wanner@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <7c9c2f8d-ff29-48a6-bf1f-53fb278bc1ad@enterprisedb.com>
 <CAD21AoAD_B2yjDwpg5Xvi_evmO_Lm=MinpbZT-soWxuqRWomKQ@mail.gmail.com>
Content-Language: en-US
From: Markus Wanner <markus@bluegap.ch>
Autocrypt: addr=markus@bluegap.ch; keydata=
 xsNNBEyA6xoBIAD2b/0sj830m1pqjX0LXY6ICB/oE4/UMxij07neWCHdGwCNGdXroz9BRucA
 cjRnVKnt/Xe10PjfgBMo41d20g0vBtGQnRcZzI5r98cfbwkCoND3M2Svh+/NZV9rPjfa5dBo
 Ihd73e/qBpmAc+YM9qGfhzgSXXg8N5lHCLo0CQxN7WrYhhB+G/mPvLjyQEenGi793R9uT5GA
 MLB2ZjbJXIRtdB5Nn+OILwELp08mrQrc6UwN60KmsrH1+1OxJUviMmCSjF573egAmhxdYOSQ
 /3JAgg5rPEAflwLodSUGFd6VQ5AaaOElqJjXTWT23YLN7r6Qq5EbF4VvsMB7ClVvXRr3vzXU
 dwzdukOsnXSv5Do0KL4gQvgNuBj5z//VMpHXoAQJvZpNgtdd4UFe4hJx4esJX0iGtqytl2xc
 QV8aZxdfksuTgzkMT+QcdcP5lB3VngxwObqyKl8NwJcDS46clNDc8qfBz5x0cCU0x4MIb179
 rf9CZ84bVrTbe1zOqeu5rnb+sqxfzSX2IDaKqZEXEUz5s1+pHo3SuxkXVm/xbqJPUIFtrAgQ
 /lt/0UEoNyXm3wxokPTrb9C3QJNyIb3IpQhGWcmBFMNVd+Kb74gjS3IQAdjETGdVYwn1BBEk
 UhMQ07RnFoojFX9hfMyVALCcmuU8mdfJqdO0DfoO9Wo5VZ5CTa8094Kzg7X0oLF1/AMZoB61
 uM2h3eBkzoHsSvgJVOGOZzPUD56Q+wjgJt9QMx7CyMRg2JyrLxmxDxB6e1GUyDK1QDMjq7iB
 nrBI4w4ivMPNC+UH15m0Kb8UmeBpKTcPnLat/2WHVeYcnm9oad9Jp6iEmCPcNsZIOkJcOxLu
 1RJ+Xfeb+1owUS7oCDSwzlM6bzG1TIVcEmXFIdwC3r0Rp9U822JDDHUTapKmRSdLpO59cuZT
 vX9E9k0TU81WtWNsy++MfNC1ed9SrcyTKijnNGz90lb9ewB0IATzyUJpKe2VGAsvj67t9dJB
 pXjg5iiKlQe0ULneg+63YovSVqQpk9E5CCLzchJ3mKCHQwc1qkXOkPC715sE/2uGtdKD3ne8
 rrJvSbz1bMq5G69MJ+QWenDtoFDDiz4LFwEH4uTM3tQtwtEjVnp4UeFErEshM1ZzJG2GP1E4
 CWmGRBcKvpQKy5yg4i4LDApWwa6hhLzkwzRpSl+wCGDSJpo4cuDDnVzuT1LRhX0t80mrRZVi
 a1DY+70tFAGVHweVLWC+LHcQAIxBx3uRdOXPQzzJVM5lIxgd+Slp1aRGqmcxNCIOTofYp9rS
 SIggvq/JgvibJx9WHTPk46X5SZop2Fnm+lzube/fPF8MK7z830wPl+7atB+WarLayo7pMSMU
 OaPqmQPMoY4XABEBAAHNIU1hcmt1cyBXYW5uZXIgPG1hcmt1c0BibHVlZ2FwLmNoPsLDnAQT
 AQgARgIbAwcLCQgKBwQDCBUKCQgLAwIBBRYCAwEAAh4BAheAAhkBFiEE7WdiNgeE4zHiUwPW
 Alr+layd8xsFAlrhndcFCSEstb0ACgkQAlr+layd8xtjXx/9GhTKwxa1Los78B+3y9kmD5kI
 N97tpCd/vJG7oFT13EIe4OXXJiYYBClpaWw6nj/bLkmIZHo4Otc8wLGisRWE+s3L5eXHiBt+
 RKlXmxamVFKGCQDhxPbUBrAsTlI5WWMkbj8J0pOYf2zfWJVRahSE+5840Hc/TzEW3gYF0oFK
 ZqblJO3wlgKs5MJzzsSnite9a1B40TJ7YjknHDDv9fUlr9WIoR/XIwaaphibI0tSGMYOSqSB
 Nu9haQv57eC2RyDz/DP8J3y5RwZJ/lfD1l9AxGCGkwFAX/VyzWjSmDvNz1kSqbs7nn57su4N
 aRl9TtSN/51B7j9WxIsbuyNPtmd26+KDgSteZT9DDNg3e455CZRRv2zo0LaOwpvLfsRh6F5c
 jnOaHYR6fBdjk8Zdw8yN1aLm2Ls0HGWzHSttBLLy7Wa75hIwrHYqz7NykMmy5bDJUvKCjmug
 aE5dJZ5mFsawYTV8PBxhOp+m8584pacBa0wcBrHpQr9+tNZJe8lQGLvxVVsYV+WEDUwc1MCJ
 tFpjS/tq420F5hLVQRCzypi8iXlohGwnlN+qRWE6NuIXBdYYuLQzed9THeNZgMd8zptGByrl
 FwJHPBlDMPLcyQPhJMq/4NksL+6F+iKpbclNpq3gq7soHjlqMQwUTK14LUAQcfzRkExuO7go
 vwtJBf286WY+LU3qG5ErdeNP1OciTKVsC4lH9GevZ0q94OJfFppPIenUK6Co7DLsM4/NKaEI
 JCn9LV2S1sktVLCtNsIiOjAsRS3PRcHVmSZcWDI6QIfINBj5omBiigtR9r38dCPdce+0aYXB
 T4RCleeyqlN/nYFQcMzRdZ/q3LbfGmhDrHUE795q4RwU8qvLIR41NwEi4dYyTKWQphA1iVaa
 D4clk70NrqNZtMacE99iG6k+0PQoTEw2O8BnXTcAXlu3wKaq1QsLTrEhrReqhNRiy4kAMORT
 dlerMgTmh4nbjBUdrhARARxruPp11Dkq3CiytSWwIbXvmIhffGBSV3mQq4iw/d5CWpzBuOzq
 1b87Tnqh36yyUc9l4PkXw6vWgRdwf+0PQRl1VGzuO0j3PY4J/H6RfLMaw8y1VFGqY76c6whw
 XnV5QxzdBw0gl7ekvTYOz/PzDistB6yzcAqdfUZfwIOmD+ewmDzpkzZgClHzrW9tM6L3dO9o
 u8DkPdMDt2CgFYz6gdSPpN3KTI4ASY8A+X5u4SHhMksqm/1M7XpwGAULa77hDidI74MtVd0H
 WVVJjfMcg8SHyOddPDKmwHWCMVLYB5IzdCZ8ZUNplMcbVlEH0K97p/e7IVH4Bwp/aaW0+n2o
 ZoJZ0vjmvjdUQoCpxtOSWIYDYtDymsgSXMnK4/Xc3joZQ844BGN9KYUSCisGAQQBl1UBBQEB
 B0BQ7XndauIOu0Owz1dPCUbA3va75F2YNBody5GaoZI5DgMBCAfCw3wEGAEIACYWIQTtZ2I2
 B4TjMeJTA9YCWv6VrJ3zGwUCY30phQIbDAUJBaOagAAKCRACWv6VrJ3zG+GxH/9PXFilXAQY
 CMiplBKCzAZUkHqmWvkFiC+rwQiUrr2GVLcYY3QC0pTb9Lq6sSJjgqrfrvmBgcG8Gy9PdfM6
 SUb1gi6T2D6wZnWSc2lwiCIJ1sJYl9MN+c0JbkDTybFJ+17IgjUVocousBh+pN4637f9qVVB
 b6FIV77sw4HfH9OVGR0iAUHHqLfbA/3thlz7BjB5mbSUfu/G87kUq9IVX5Nk9MNsT4+x+Gmg
 WqAfcyPwhok2RlSm7UnZjjwiK4so5w1tiKqV6fwNsAntVX0ikT538gSIkB4oQINeQJ6OACVP
 CaB9YqysAZyEiZZazNaU5a1lXa0AUBL+kwmNOMxB/kArDvwcxZNtzKNpW6B9Xug8sawLgD8n
 v/FTI4j6CDZnNw9HrZi1eQ6piEI0qqvGGGQ/aXtjyGKlzkE3xZa7FzjkJQIMsitZ7ZLAgFSP
 +Oj9wLD0mtBEMVWA8Cx/B/DiiEjMUOfg7yRmd3NKVYLfUkQVdQf6fDLXKgiQ0+Fb+c8gWWGF
 HZq0aBOJZ6QcG0CXZEjDtUbxQZALrinKPrjfZWPsOM2GcYxgeLbp51f8sdMg0nzwB9UfoW1o
 5HXhvcqtwehm5Ld5oHLCcJ7aUV04zvSCe0SgRpEfLtOKNVRhG+8skymV4ztjHncV0p+RtJxo
 J2gJiSWk3VTikdEC0V/oM65wt9VEY7LBi3CLu+jczjYyZrExZXKMfTPjyhmK2pPsW/rQS8Le
 e+BEOhEqlt200FhBySPzNX0/DMx0KKg6mKK6/G6DNEcSptqbGB4MsgBlKG9mzSsNd1E/nzqM
 MME/+UM+efSxQdTt1tTFF5cTofbCUmOcIoH/BXS3xl/K2tTUoa23pdpWLvJfM8Y4CzgmwIEi
 uLyWCtzqVjpqhm9f6jJr6qnSZ6hrscoV1c+rK2ok8e/tda1kPTjd3OCzP7akO+kkS7sEzvp3
 XLpJm/FoWyb1j9OGDAvNyBMjZz/WJ2XbFsnt2PdvUW9Kp0oqjatwYh8kaeLZg5PTgCEYtzeo
 qefA3eEB5RxZgyJgjDa3wXkX9n/CczHpXxUc/qFb4FyC5CrzV+EI6eO46PYHfx478C0XurSW
 gmPaSsZ/uwJtEaVHjl+tWIUbpzX0gcUAG9o3Tap6bjecgJFTm4SnJaXfp5JcYJNTPPsuVWCB
 nI5X12lr6Cbc1wKW/uKXe6lHxEHNkflHoYvxwlkWsi3rkzyIRy1F/fwDmD8ZgySD0/AeyFoM
 ISHwIoDr7mqBMgvuWBrsoibaH+vYmo7aYyGsEmJ10EHFckg1MFnmmul0UN35GjyWzICKQVt5
 rXX2QBDETwRSb3B+cb7tmevoHmYn61xM7Z4Aglt2l39NG4mLdrGQzHBRelRo
In-Reply-To: 
 <CAD21AoAD_B2yjDwpg5Xvi_evmO_Lm=MinpbZT-soWxuqRWomKQ@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/b7c8d68f-5bda-4c33-adeb-ac3cef7c77ad%40bluegap.ch>
Precedence: bulk

This is a multi-part message in MIME format.
--------------P6qpeRkCgbxiSlnCGeNDdnWh
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 2/21/25 07:17, Masahiko Sawada wrote:
> On Thu, Feb 20, 2025 at 12:50 PM Markus Wanner
> <markus.wanner@enterprisedb.com> wrote:
>>
>> Hi,
>>
>> I recently stumbled over an issue with an unintentional re-transmission.
>> While this clearly was our fault in the output plugin code, I think the
>> walsender's exposed API could easily be hardened to prevent the bad
>> consequence from this mistake.
>>
>> Does anything speak against the attached one line patch?
> 
> According to the documentation[1], OutputPluginPrepareWrite() has to
> be called before OutputPluginWrite().

Sure, that's exactly what we forgot. My complaint is that it's a mistake 
that's unnecessarily hard to spot and the API could be improved. There 
is no good reason to keep the sent data in the ctx->out buffer. But 
clearly, there's some danger to it.

(Note that it wasn't quite as simple as you may think, though. With an 
error involved in the send/recv loop of the walsender invoked from 
OutputPluginWrite. The error handling code in PG_CATCH then attempting 
to flush the queue with OutputPluginWrite.)

Arguably, one could even say that replacing the call to resetStringInfo 
in WalSndPrepareWrite with an Assert(ctx->out->len == 0) would catch a 
variant of improper use. And another Assert in WalSndWriteData to check 
OutputPluginPrepareWrite had properly been invoked before can't hurt, 
either.

Best Regards

Markus
--------------P6qpeRkCgbxiSlnCGeNDdnWh
Content-Type: text/x-patch; charset=UTF-8;
 name="0002-Add-asserts-to-guide-to-proper-API-usage.patch"
Content-Disposition: attachment;
 filename="0002-Add-asserts-to-guide-to-proper-API-usage.patch"
Content-Transfer-Encoding: base64

RnJvbSA1YjlkNjI4NTI1MjIwNGQ3MTA5MDAxZmM0MjMyM2FkZDAwYWM3NzNmIE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBNYXJrdXMgV2FubmVyIDxtYXJrdXMud2FubmVyQGVu
dGVycHJpc2VkYi5jb20+CkRhdGU6IEZyaSwgMjEgRmViIDIwMjUgMDk6MDA6MjggKzAxMDAK
U3ViamVjdDogW1BBVENIXSBBZGQgYXNzZXJ0cyB0byBndWlkZSB0byBwcm9wZXIgQVBJIHVz
YWdlCgotLS0KIHNyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL3dhbHNlbmRlci5jIHwgNCArKyst
CiAxIGZpbGUgY2hhbmdlZCwgMyBpbnNlcnRpb25zKCspLCAxIGRlbGV0aW9uKC0pCgpkaWZm
IC0tZ2l0IGEvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vd2Fsc2VuZGVyLmMgYi9zcmMvYmFj
a2VuZC9yZXBsaWNhdGlvbi93YWxzZW5kZXIuYwppbmRleCA5Njg4NGNlMTUyZi4uYjVkN2Jh
ZjgzYWQgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL3dhbHNlbmRlci5j
CisrKyBiL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL3dhbHNlbmRlci5jCkBAIC0xNTIzLDcg
KzE1MjMsNyBAQCBXYWxTbmRQcmVwYXJlV3JpdGUoTG9naWNhbERlY29kaW5nQ29udGV4dCAq
Y3R4LCBYTG9nUmVjUHRyIGxzbiwgVHJhbnNhY3Rpb25JZCB4aQogCWlmICghbGFzdF93cml0
ZSkKIAkJbHNuID0gSW52YWxpZFhMb2dSZWNQdHI7CiAKLQlyZXNldFN0cmluZ0luZm8oY3R4
LT5vdXQpOworCUFzc2VydChjdHgtPm91dC0+bGVuID09IDApOwogCiAJcHFfc2VuZGJ5dGUo
Y3R4LT5vdXQsICd3Jyk7CiAJcHFfc2VuZGludDY0KGN0eC0+b3V0LCBsc24pOwkvKiBkYXRh
U3RhcnQgKi8KQEAgLTE1NDksNiArMTU0OSw4IEBAIFdhbFNuZFdyaXRlRGF0YShMb2dpY2Fs
RGVjb2RpbmdDb250ZXh0ICpjdHgsIFhMb2dSZWNQdHIgbHNuLCBUcmFuc2FjdGlvbklkIHhp
ZCwKIHsKIAlUaW1lc3RhbXBUeiBub3c7CiAKKwlBc3NlcnQoY3R4LT5vdXQtPmxlbiA+PSAx
ICsgMyAqIHNpemVvZihpbnQ2NCk7CisKIAkvKgogCSAqIEZpbGwgdGhlIHNlbmQgdGltZXN0
YW1wIGxhc3QsIHNvIHRoYXQgaXQgaXMgdGFrZW4gYXMgbGF0ZSBhcyBwb3NzaWJsZS4KIAkg
KiBUaGlzIGlzIHNvbWV3aGF0IHVnbHksIGJ1dCB0aGUgcHJvdG9jb2wgaXMgc2V0IGFzIGl0
J3MgYWxyZWFkeSB1c2VkIGZvcgotLSAKMi4zOS41Cgo=

--------------P6qpeRkCgbxiSlnCGeNDdnWh--