ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
> Here is a patch to log conflicted queries on deadlocks. Queries are dumped
> at CONTEXT in the same sorting order as DETAIL messages. Those queries are
> picked from pg_stat_get_backend_activity, as same as pg_stat_activity,
> so that users cannot see other user's queries.
Applied with revisions. It's a cute idea --- I first thought it
couldn't work reliably because of race conditions, but actually we
haven't aborted our own transaction at this point, so everyone else
involved in the deadlock is still waiting and it should be safe to
grab their activity strings.
However there was still a big implementation problem, which is that
looking at pg_stat_activity could deliver very stale results, not only
about other backends' queries but even our own. The data for that
comes from a snapshot that might have been taken much earlier in our
transaction. I replaced the code you were using with a new pgstat.c
entry point that delivers up-to-date info directly from the shared
memory array.
> (It might be better to log all queries in the server log and mask them
> in the client response, but I'm not sure how to do it...)
Yeah, that would be cute, but we don't have any way to deliver a
different CONTEXT string to the client than we do to the log. We could
generate duplicate messages that go only to the log but that seemed
pretty ugly. In practice this definition is probably good enough.
One thing that I worried about for a little bit is that you can imagine
privilege-escalation scenarios. Suppose that the user is allowed to
call some SECURITY DEFINER function that runs as superuser, and a
deadlock occurs inside that. As the patch stands, every query involved
in the deadlock will be reported, which might be undesirable. We could
make the test use the outermost session user's ID instead of current
ID, but that might only move the security risks someplace else.
Thoughts?
regards, tom lane
