Re: BUG #16329: Valgrind detects an invalid read when building a gist index with buffering
x4mmm@yandex-team.ru
From: Andrey Borodin <x4mmm@yandex-team.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Pavel Borisov <pashkin.elfe@gmail.com>,
Alexander Lakhin <exclusion@gmail.com>,
pgsql-bugs@lists.postgresql.org
Date: 2020-10-24T16:40:35Z
Lists: pgsql-bugs
Attachments
- 0001-Make-GiST-deterministic.patch (application/octet-stream) patch 0001
> 22 окт. 2020 г., в 19:01, Tom Lane <tgl@sss.pgh.pa.us> написал(а): > > Andrey Borodin <x4mmm@yandex-team.ru> writes: >>> 13 окт. 2020 г., в 03:16, Tom Lane <tgl@sss.pgh.pa.us> написал(а): >>> 2. The test exposed the gistRelocateBuildBuffersOnSplit bug only about >>> one time in ten for me. I suppose this is due to the random split >>> choices gistchoose makes for equally-good tuples, so it's not terribly >>> surprising; but it is problematic for a test that we're hoping to use >>> to provide reliable code coverage. >>> >>> I'm not really sure what we could do about #2. Perhaps, instead of >>> relying on random(), we could make gistchoose() use our own PRNG and >>> then invent a debugging function that forces the seed to a known value. >>> (GEQO already does something similar, although I'd not go as far as >>> exposing the seed as a GUC. Resetting it via some quick-hack C >>> function in regress.c would be enough IMO.) Or perhaps gist.sql could >>> be adjusted so that the test data is less full of equally-good tuples. > >> I think we should use not entropy-based tie breaker in GiST. We can extract some randomness from tuples using hash. >> I'd be much happier if GiST behaviour was deterministic. > > If we started using our own PRNG, we could very easily make the "random" > sequence be the same in every GiST build --- again, see GEQO for prior > art. I'm a little suspicious of trying to pull some entropy out of the > tuples themselves: that seems like it'd be tremendously prone to cross- > platform and cross-version behavioral differences. PFA copy of GEQO approach to GiST. I haven't found proper place to document this: current random tie breaker seems undocumented. Will describing GUC here [0] be enough? Thanks! Best regards, Andrey Borodin. [0] https://www.postgresql.org/docs/13/runtime-config-developer.html
Commits
-
Fix dereference of dangling pointer in GiST index buffering build.
- d2a1d4b190ec 12.15 landed
- b5c6776c1127 11.20 landed
- a1904c9ce56a 14.8 landed
- 8e5eef50c5b4 16.0 landed
- 2dc77adc768e 15.3 landed
- 2adb6adad5f8 13.11 landed
-
Fix GiST buffering build to work when there are included columns.
- 962ab473ec3d 13.1 landed
- 371668a8389d 14.0 landed
- 12945874ebd6 12.5 landed
-
Re-allow testing of GiST buffered builds.
- 78c0b6ed273a 14.0 landed
-
Add support for building GiST index by sorting.
- 16fa9b2b30a3 14.0 cited