Date: Fri, 27 Jul 2018 13:15:42 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: bossartn@amazon.com, andres@anarazel.de,
	pgsql-hackers@lists.postgresql.org, robertmhaas@gmail.com,
	schnjere@amazon.com, pgsql-hackers@postgresql.org,
	lalbin@scharp.org
Subject: Re: BUG #15182: Canceling authentication due to timeout aka Denial
 of Service Attack
Message-ID: <20180727041542.GH1754@paquier.xyz>
References: <20180726204127.GA1754@paquier.xyz>
 <3D4F2733-92AE-4B74-8912-3C611B027721@amazon.com>
 <20180727023123.GE1754@paquier.xyz>
 <20180727.123702.77822416.horiguchi.kyotaro@lab.ntt.co.jp>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="cf0hFtnykp6aONGL"
Content-Disposition: inline
In-Reply-To: <20180727.123702.77822416.horiguchi.kyotaro@lab.ntt.co.jp>
Precedence: bulk


--cf0hFtnykp6aONGL
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Jul 27, 2018 at 12:37:02PM +0900, Kyotaro HORIGUCHI wrote:
> I intended the same as Bossart said. It is not reasonable that
> VACUUM and VACUUM FULL behaves differently on rejection for the
> same reason. The objective of the first (or early) check is
> rejecting (non-superuser's) unprivileged VACUUM FULL. Any
> possible modifications on a syscache tuple before taking a lock
> doesn't seem to harm in the point of view. Anyway the lock
> acquired in expand_vacuum_rel is not held until vacuum_rel.

Thinking about it harder, it is possible to pass down a status pointer
that the callback of RangeVarGetRelidExtended could fill if we pass it
down using the argument list.  This would need special handling for the
case where expand_vacuum_rel() is NIL but that would be workable.  If
vacuum_check_rel() does not need to work with elevel >=3D ERROR, then the
set of log messages remains the same.

> Apart from the discussion above, it is uneasy for me that the
> messages seem heavily affected by the callers context.

Sure, the patch for vacuum is still heavily WIP.  I am not much happy
about that either.

>> Regarding those patches, I am pretty happy how things turn out for
>> TRUNCATE and REINDEX, way less for VACUUM, so getting 0001 and 0003
>> committed first makes the most sense to me as their logic is rather
>> straight-forward (well way of speaking ;p ).
>=20
> About 0001, I'm not sure what the "session-level" means but it
> looks fine and works as expected. 0003 looks fine overall.

What I mean here is checking the interactions with other backends,
truncate_check_session is the best name I could come up with.
truncate_check_activity was another.  I am not attached to a single
name, if you have an ideaof course please feel free.

> I think I heard that gender-free wording is appreciated but it
> might be only about documentation.

Sure.
--
Michael

--cf0hFtnykp6aONGL
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAltanG4ACgkQnvQgOdby
QH24YxAAh6HHX5GWitzQtHTxUmyzW4yztxrEhqKRfvyK7bFAjsGsAM/c+4mcQM7l
wZkDYTZMyOZEU/hWoejYVoxD+3bOWbvE02lDiEECdj1GVzkkCg4w+PP2amFCD3+0
F+r5P4Tog66HXuvUgU+C7DSKMsApCJMszMtZ8tiqsbOg8EebtL1DTqDGXF0lYUbl
rWK6RE3AorIzJpkwIR6IllV0NM4bMePg4ocXeH1WBQ4AT0/7WK0klAX3UWGF4ONr
+thzAwpyGVD7yR/pjMzj2EriYBvOG8nstqoH8xN0bhlwmtUMXxhLewek9jkVwtVG
OlgIIV7/3mY+GNtABXtCg+BzwKgMnyjr0CucOScxlK/9eGUzrGV0eQm7NSjszZfP
BcjGc4Fg3oRMuNlp8CLn3QNtCoqCjCL45QgaNY37uNmT3mq9WcoZgn//9mDLYhBa
9nbFla9Kxh/OMgVIpTpHhGzqsm+dyamqc0KPCT95nZjYQCgC9s6MFIxncoGgncjz
zv8wzrRAmBme3ON8FDF5ao6qrGqGURwONTGTrh62y5SsnRRngo5IzPV4wmvaSJ4q
B1iaDyDf0O9ENYPUpljK3mMcpqRz+BWn8Um3LnnNdfwIJv5Imh7JdWfD2NdWeXRv
2zexmUEI3BgcBz7EeJRplfUqyhOi9Nlr8Gc3qOP8dV3IMt8c/SE=
=S8HQ
-----END PGP SIGNATURE-----

--cf0hFtnykp6aONGL--