Re: dsm_unpin_segment

On Sun, Aug 21, 2016 at 7:54 PM, Thomas Munro
<thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> Here's the rationale I'm using: if it's helpful to programmers of
> client code, especially client code that might include extensions, and
> nowhere near a hot code path, then why not use elog rather than
> Assert? I took inspiration for that from the pre-existing "debugging
> cross-check" in dsm_attach that does elog(ERROR, "can't attach the
> same segment more than once"). On that basis, this new version
> retains the elog you mentioned, and now also uses elog for the
> you-tried-to-unpin-a-handle-I-couldn't-find case. But I kept Assert
> in places that detect bugs in *this* code, rather than client code.

Hmm, well said. I've never thought about it in exactly that way, but
I think that is a useful distinction. I've sometimes phrased it this
way: an Assert() is good if the path is performance-critical, or if
the Assert() is checking something that is "nearby" in the code, but
an elog() is better if we're checking for a condition that could
happen as a result of some code that is far-removed from the place
where the elog() is. If an assertion fails, you're not necessarily
going to realize right away that the calling code needs to be checked
for errors. That could be mitigated, of course, by adding a comment
right before the Assert() saying "if this Assertion fails, you
probably did X, and you shouldn't do that". But an elog() can state
the exact problem right away.

Also, of course, elog() is the right tool if we want to perform the
check even in production builds where asserts are not enabled. That's
not so relevant here, but it matters in some other cases, like when
checking for a case that shouldn't happen normally but could be the
result of data corruption.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company