Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:
> On 22.06.2011 17:28, Tom Lane wrote:
>> Why is the call in ExecSeqScan at all, and not in the node
>> startup function?
>
> It makes sense to delay it until the scan is actually started, so
> that you don't get unnecessary serialization failures if the scan
> is never actually executed. I don't know if that was Kevin's
> original reason for putting it there, but that's why I left it
> there.
I honestly can't remember whether that was a factor. I went through
the README files and source code comments and set breakpoints at the
low level heap reads in gdb and captured stack traces from as many
execution plans as I knew how to generate, and went looking through
those for likely places to put the predicate locking calls. I
reasoned through the alternatives as best I could coming in cold and
having been discouraged from asking questions. It would not shock
me if those with greater familiarity with the code and a deeper
understanding of how the pieces fit together can improve on my work
there.
I certainly won't take offense at any improvements made there; but I
do have some concern over making changes this late in the release
cycle unless they are clearly safe. Anssi Kääriäinen put in days of
testing with real production data and software, and YAMAMOTO Takashi
put in what appears to have been weeks of solid run time with I
don't know what testing setup, but one which was really good at
exposing race conditions. I get nervous about invalidating those
efforts if they won't be repeated before release.
By the way, I didn't miss the concern about the lossy bitmaps in
bitgetpage() -- I'm trying to work my way through that now. What's
a good way to generate a plan which uses lossy bitmaps? I'd like to
try to generate a failing test. That's often very useful to me
during coding, and tends to make a good addition to the test suite.
-Kevin