Re: logical replication: could not create file "state.tmp": File exists

On Sat, Nov 30, 2019 at 03:09:39PM +0300, Grigory Smolkin wrote:
> I`ve digged a bit into this problem, and it`s turned out that in
> SaveSlotToPath() temp file for replication slot is opened with 'O_CREAT |
> O_EXCL' flags, which makes this routine as not very reentrant.

What did you see as I/O problem before facing the actual error
reported here?  Was it just ENOSPC, a fsync failure, or just a failure
in closing the fd?  The first pattern is mostly what I guess happened,
still a fsync failure would not trigger a PANIC here (actually we
really should do that!), but I am raising a different thread about
that issue.

> Since an exclusive lock is taken before temp file creation, I think it
> should be safe to replace O_EXCL with O_TRUNC.
> Script to reproduce and patch are attached.

Agreed.  I prefer the O_TRUNC option because that's less code churn.  
Also, as it can still be useful to have a look at the temporary state
file after a crash or a failure, doing unlink() in the error code
paths is no good option IMO.

Have others thoughts or objections to share?
--
Michael

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAl3klKMACgkQnvQgOdby
QH2wog//UTidu8V4DCBjseT33aq9QzYbONJklaqmNIdsZIe+3j7ctT2qhmBpcbzC
DDAVJmAIuU/Ji21Vo3jDVMVDuDEU3e+Wk3qvs3PgKKRQ16zaMTeFj5FqPL9JpXU+
mF/UidKttzihS1o0+ZR/bIJStL6p563Llkz7RBVIBLk9zPAcIC4dTHMPy6/EZfZg
odQe2z0qnFGuEj30rXn655IJwZs48TSWRMSmz7pfSK/hyXGBjhYIb7x5dzkKDvCK
aqxWWN5xALbY5nSLiKU6qfJw/FRzuOQv1vBYEqI4xAxwGFcHr6kWUzgt5nuobhNY
ZDro2jCfvCE04wD/NyP8HAAAtG5LekGGsG9cBubqE+ie3pzgrEZZnQYBvJV8EANT
6TP6zVL3YN/ZYVQ5avX9ehemS0dxNpx96+swBhzthEoRc/9qXWpzNk/57tmNDpNJ
lJxW3VVVI35RtBzoWieAZTp06dBf93JEAH2XO+ek9ZD7P8rVZPEOgVqK3XsJn8xR
APNn83bQwAOiV0G1pY8ctsMcq2XczSmatu5h+5rIK+pHAElHzcbCHwAhFVbJM8rs
MvZBszg6rBVX96U/3eZcapjAHYgckQXucgLaJiomejDNQxK2gWRxdKzx1QN2m78L
1PSg178Qz4OYlCbg5MWChujlSm4PKahC01cwoXCVHAgplaPDV2I=
=J11v
-----END PGP SIGNATURE-----