On 12/18/2011 07:31 AM, Magnus Hagander wrote:
* I restructured the if statements, because I had a hard time
following the comments around that ;) I find this one easier - but I'm
happy to change back if you think your version was more readable.

That looks fine. I highlighted this because I had a feeling there was still some gain to be had here, just didn't see it myself. This works.

* The error message in pg_signal_backend breaks the abstraction,
because it specifically talks about terminating the other backend -
when it's not supposed to know about that in that function. I think we
either need to get rid of the hint completely, or we need to find a
way to issue it from the caller. Or pass it as a parameter. It's fine
for now since we only have two signals, but we might have more in the
future..

I feel that including a hint in the pg_terminate_backend case is a UI requirement. If someone has made it as far as discovering that function exists, tries calling it, and it fails, the friendly thing to do is point them toward a direction that might work better. Little things like that make a huge difference in how friendly the software appears to its users; this is even more true in cases where version improvements actually expand what can and cannot be done.

My quick and dirty side thinks that just documenting the potential future issues would be enough:

"Due to the limited number of callers of this function, the hint message here can be certain that pg_terminate_backend provides the only path to reach this point. If more callers to pg_signal_backend appear, a more generic hint mechanism might be needed here."

If you must have this more generic mechanism available, I would accept re-factoring to provide it instead. What I wouldn't want to live with is a commit of this where the hint goes away completely. It's taken a long time chopping the specification to get this feature sorted out; we might as well make what's left be the best it can be now.

--
Greg Smith   2ndQuadrant US    g...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to