"Kevin Grittner" wrote:
> Robert Haas wrote:
>> Updated patches attached.
>
> I have to admit I don't have my head around the extraWaits issue,
> so I can't personally vouch for that code, although I have no
> reason to doubt it, either. Everything else was something that I at
> least *think* I understand, and it looked good to me.
>
> I'm not changing the status until I get through the other patch
> file, which should be tomorrow.
Most of the procarraylock-v1.patch file was pretty straightforward,
but I have a few concerns.
Why is it OK to drop these lines from the else condition in
ProcArrayEndTransaction()?:
/* must be cleared with xid/xmin: */
proc->vacuumFlags &= ~PROC_VACUUM_STATE_MASK;
The extraWaits code still looks like black magic to me, so unless
someone can point me in the right direction to really understand
that, I can't address whether it's OK.
The need to modify flexlock_internals.h and flexlock.c seems to me to
indicate a lack of desirable modularity here. The lower level object
type shouldn't need to know about each and every implementation of a
higher level type which uses it, particularly not compiled in like
that. It would be really nice if each of the higher level types
"registered" with flexlock at runtime, so that the areas modified at
the flexlock level in this patch file could be generic. Among other
things, this could allow extensions to use specialized types, which
seems possibly useful. Does that (or some other technique to address
the concern) seem feasible?
-Kevin