amcheck on the Heroku fleet, back when I worked there, there were all
kinds of non-interesting errors that could occur that needed to be
filtered out. I want to try to make that process somewhat less painful.
--
Peter Geoghegan
t code
> has changed a bit between branches. Happy to do so on master.
The elog(), which was itself upgraded from a simple Assert by commit
d70cf811, appears in exactly the same form in 9.3+. Things did change
there, but they were kept in sync.
--
Peter Geoghegan
Fix typo in JIT README.
Author: Daniel Gustafsson
Discussion: https://postgr.es/m/3747d478-41f9-439f-8074-ac81a5c76...@yesql.se
Branch
--
master
Details
---
https://git.postgresql.org/pg/commitdiff/f6b95ff434bff28c0d9b390d5a0ff316847c4fb7
Modified Files
--
On Sat, Jun 9, 2018 at 10:55 AM, Andres Freund wrote:
> Congrats!
Thanks Andres!
--
Peter Geoghegan
Remove INCLUDE attributes section from docs.
Discussing covering indexes in a chapter that is mostly about the
behavior of B-Tree operator classes is unnecessary. The CREATE INDEX
documentation's handling of covering indexes seems sufficient.
Discussion:
Correct a comment on logtape.c's leader tape.
randomAccess parallel tuplesorts are disallowed because the leader would
try to write to its own leader tape, not because the leader would try to
write to a worker tape directly.
Cleanup from commit 9da0cc35284.
Branch
--
master
Details
---
On Sun, Feb 4, 2018 at 10:11 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> I'll be happier about it when the valgrind buildfarm animals are
> happy.
I don't know if you noticed, but I did post a patch for that on Friday.
--
Peter Geoghegan
On Sun, Feb 4, 2018 at 9:42 AM, Andres Freund <and...@anarazel.de> wrote:
> Wheee! Congrats Peter, Rushash, and everyone else involved!
Thanks!
--
Peter Geoghegan
; is not.
I think that the suppression is actually slightly better scoped than
that, since, for example, that won't just affect writes of
uninitialized bytes from the buffer. But I'll do it that way.
--
Peter Geoghegan
On Wed, Jul 11, 2018 at 4:35 PM, Peter Geoghegan wrote:
> You've convinced me that we should definitely have such a list. I've
> put it on my TODO list.
I started this Wiki page:
https://wiki.postgresql.org/wiki/Committing_checklist
I've tried to avoid being too prescriptive. This is
9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers(). (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)
Author: Peter Geoghegan
Reported-By: Luca Ferrari
9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers(). (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)
Author: Peter Geoghegan
Reported-By: Luca Ferrari
9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers(). (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)
Author: Peter Geoghegan
Reported-By: Luca Ferrari
9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers(). (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)
Author: Peter Geoghegan
Reported-By: Luca Ferrari
9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers(). (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)
Author: Peter Geoghegan
Reported-By: Luca Ferrari
9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers(). (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)
Author: Peter Geoghegan
Reported-By: Luca Ferrari
9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers(). (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)
Author: Peter Geoghegan
Reported-By: Luca Ferrari
Doc: Correct description of amcheck example query.
The amcheck documentation incorrectly claimed that its example query
verifies every catalog index in the database. In fact, the query only
verifies the 10 largest indexes (as determined by pg_class.relpages).
Adjust the description accordingly.
Doc: Correct description of amcheck example query.
The amcheck documentation incorrectly claimed that its example query
verifies every catalog index in the database. In fact, the query only
verifies the 10 largest indexes (as determined by pg_class.relpages).
Adjust the description accordingly.
Doc: Correct description of amcheck example query.
The amcheck documentation incorrectly claimed that its example query
verifies every catalog index in the database. In fact, the query only
verifies the 10 largest indexes (as determined by pg_class.relpages).
Adjust the description accordingly.
backend state as part of standard parallel initialization (Cf. commit
29d58fd3). On v11, simply disallow builds against mapped catalog
relations by deeming them parallel unsafe.
Author: Peter Geoghegan
Reported-By: "death lock"
Reviewed-By: Tom Lane, Amit Kapila
Bug: #15309
Discussi
backend state as part of standard parallel initialization (Cf. commit
29d58fd3). On v11, simply disallow builds against mapped catalog
relations by deeming them parallel unsafe.
Author: Peter Geoghegan
Reported-By: "death lock"
Reviewed-By: Tom Lane, Amit Kapila
Bug: #15309
Discussi
Correct obsolete unique index insertion comment.
Commit bc292937ae6 failed to update a comment about unique index
checking. _bt_insertonpg() is no longer responsible for finding an
insertion location while preventing conflicting insertions.
Branch
--
master
Details
---
On Mon, Jul 9, 2018 at 11:24 AM, Alvaro Herrera
wrote:
> Maybe we should add an XML comment
>
> at the end of the table :-)
+1. I made the same mistake at one point.
--
Peter Geoghegan
checklist. Still, it wouldn't hurt
> to have a wiki page of checklist entry ideas from which folks cherry-pick the
> entries they like.
You've convinced me that we should definitely have such a list. I've
put it on my TODO list.
--
Peter Geoghegan
bout pre-existing indexes.
>
> In short, this sounds like a place that did not get the memo about
> how to cope with un-upgraded indexes.
Sounds plausible.
--
Peter Geoghegan
buildfarm's not complaining --- what's the test case?
This was discovered while testing/reviewing the latest version of the
INCLUDE covering indexes patch. It now seems to be unrelated.
Sorry for the noise.
--
Peter Geoghegan
nk you for committing this.
>
> It appears that patch contains some redundant variabled. See warnings
> produced
> by gcc-7.
I also see an assertion failure within _bt_getrootheight():
TRAP: FailedAssertion("!(metad->btm_version == 3)", File:
"/home/pg/postgresql/root/bui
uggest putting this in nbtree.h instead. You can put it just after
BTREE_NONLEAF_FILLFACTOR, with a comment, in a separate block/section.
Other than that, looks good to me.
Thanks
--
Peter Geoghegan
On Tue, Apr 10, 2018 at 12:30 PM, Peter Geoghegan <p...@bowt.ie> wrote:
>> Apart from that, other changes requested are included in the patch. This
>> also takes care of Heikki's observation regarding UNLOGGED tables on the
>> other thread.
>
> Cool.
BTW,
ed the spelling of "optimization".
The comments still say "Check if the page...no split is in progress".
Despite the fact that that's just an assertion now.
--
Peter Geoghegan
On Tue, Apr 10, 2018 at 3:37 PM, Andrew Dunstan
<andrew.duns...@2ndquadrant.com> wrote:
>> The comments still say "Check if the page...no split is in progress".
>> Despite the fact that that's just an assertion now.
>>
>
> Fixed.
Thanks.
--
Peter Geoghegan
to think sensibly.
I know the feeling.
I think you can take that wording almost verbatim. Obviously it should
refer to the optimization by name, and blend into the surrounding text
in the README. I suggest putting a small section before "On-the-Fly
Deletion Of Index Tuples", but after the main discussion of deletion +
recycling. It's essentially an exception to the general rule, so that
placement makes sense to me.
Thanks
--
Peter Geoghegan
ock isn't required.
We cannot fail to detect that our hint was invalidated, because there
can only be one such page in the B-Tree at any time. It's possible
that the page will be deleted and recycled without a backend's cached
page also being detected as invalidated, but only when we happen to
recycle a page that once again becomes the rightmost leaf page.
Once those changes are made, this should be fine to commit.
--
Peter Geoghegan
I suggest using Valgrind to make sure that a patch + tests don't have
a problem like this before pushing. That's not perfect, of course, but
it's an easy way to save yourself some trouble.
--
Peter Geoghegan
On Thu, Apr 5, 2018 at 10:16 AM, Peter Geoghegan <p...@bowt.ie> wrote:
> I think you can take that wording almost verbatim. Obviously it should
> refer to the optimization by name, and blend into the surrounding text
> in the README. I suggest putting a small section before "O
ling
in the first place? We don't bother to check that when we already know
the page is rightmost within _bt_moveright() or _bt_findinsertloc().
I also noticed that favorable/favourable is misspelled "favourble".
Also, "workout" is a noun.
This patch was committed in haste, and it shows.
--
Peter Geoghegan
d, since page
deletion cannot delete the right most page among an internal page's
children, but actually omitting the P_IGNORE() doesn't seem like an
improvement. That's probably worth a comment, though.
--
Peter Geoghegan
On Wed, Mar 28, 2018 at 11:56 AM, Peter Geoghegan <p...@bowt.ie> wrote:
>> Previously, we agreed that P_IGNORE() is required. So I assume no issues
>> there. The other tests seem required too for us to conclusively decide to
>> use the cached block.
&g
regression test seems like a good idea.
--
Peter Geoghegan
ose I should have
thought of that, but didn't.
> But thank you for the leeway.
FWIW, I think that a few hours should be standard. After that, it
should be possible for allowances to be made based on extenuating
circumstances. (I don't think that it's necessary to formalize that,
though.)
--
Peter Geoghegan
ing in unsigned.
I don't want to do the whole thing in unsigned, because this ties back
fairly directly to an int8 argument from the test_bloomfilter() SQL
function interface.
I proposed the attached, which makes the buffer one byte larger, per
your suggestion.
Thanks
--
Peter Geoghegan
From 4464e134
hat isn't ideal, and
particularly hinders automated testing.
--
Peter Geoghegan
On Thu, Feb 22, 2018 at 6:32 AM, Robert Haas <robertmh...@gmail.com> wrote:
>> Attached patch does this. I cannot recreate this issue locally, but
>> this should still fix it on skink.
>
> Committed.
Looks like it worked.
--
Peter Geoghegan
points are organized in a way that makes a run
down quick and easy, even when committing a trivial patch.
--
Peter Geoghegan
ot;gitcommit" syntax.
I find it useful to have a routine checklist for things like
committing, because it frees up a little space in my head for other
things. I do the same thing when preparing for a trip.
--
Peter Geoghegan
Adjust trace_sort log messages.
The project message style guide dictates: "When citing the name of an
object, state what kind of object it is". The parallel CREATE INDEX
patch added a worker number to most of the trace_sort messages within
tuplesort.c without specifying the object type. Bring
Adjust trace_sort log messages.
The project message style guide dictates: "When citing the name of an
object, state what kind of object it is". The parallel CREATE INDEX
patch added a worker number to most of the trace_sort messages within
tuplesort.c without specifying the object type. Bring
Have BufFileSize() ereport() on FileSize() failure.
Move the responsibility for checking for and reporting a failure from
the only current BufFileSize() caller, logtape.c, to BufFileSize()
itself. Code within buffile.c is generally responsible for interfacing
with fd.c to report irrecoverable
Have BufFileSize() ereport() on FileSize() failure.
Move the responsibility for checking for and reporting a failure from
the only current BufFileSize() caller, logtape.c, to BufFileSize()
itself. Code within buffile.c is generally responsible for interfacing
with fd.c to report irrecoverable
Correct obsolete nbtree recovery comments.
Commit 40dae7ec537, which made the handling of interrupted nbtree page
splits more robust, removed an nbtree-specific end-of-recovery cleanup
step. This meant that it was no longer possible to complete an
interrupted page split during recovery.
Correct obsolete nbtree recovery comments.
Commit 40dae7ec537, which made the handling of interrupted nbtree page
splits more robust, removed an nbtree-specific end-of-recovery cleanup
step. This meant that it was no longer possible to complete an
interrupted page split during recovery.
Remove obsolete nbtree duplicate entries comment.
Remove a comment from the Berkeley days claiming that nbtree must
disambiguate duplicate keys within _bt_moveright(). There is no special
care taken around duplicates within _bt_moveright(), at least since
commit 9e85183bfc3 removed inscrutable
to make it actually painful you need a
> workload where the implied randomness of accesses isn't already a major
> bottleneck - and that's hard.
There is also the fact that in many cases you'll just have accessed
the same buffers from within _bt_check_unique() anyway.
--
Peter Geoghegan
On Wed, Mar 20, 2019 at 3:11 PM Peter Geoghegan wrote:
> On Wed, Mar 20, 2019 at 3:08 PM Tom Lane wrote:
> > And done. Should be possible to revert 7d3bf73ac, if you wish.
>
> Will do.
Actually, I'm not sure why it would be fine to revert 7d3bf73ac now.
Might the problem actual
reason why you didn't remove the heap_hot_search() wrapper
function entirely?
--
Peter Geoghegan
users, though.
--
Peter Geoghegan
On Wed, Mar 27, 2019 at 12:04 PM Andres Freund wrote:
> Congrats, this was long in the making!
+1
Buildfarm member aye-aye has a problem with this patch, though:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=aye-aye=2019-03-27%2019%3A07%3A53
--
Peter Geoghegan
it.
>
> pushed a fix
Buildfarm member snapper is still unhappy about this:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=snapper=2019-03-26%2013%3A01%3A31
--
Peter Geoghegan
99df81. I think that pruning could affect the results, for
example. Previously, it was mostly a matter of index insertion order,
which looked like it made heap TIDs among duplicates be in descending
order, though that certainly didn't happen reliably.
--
Peter Geoghegan
g forward.
> If no objections, I'll push shortly.
Sounds good.
--
Peter Geoghegan
if, for whatever reason, it seems
necessary.
Thanks
--
Peter Geoghegan
Remove dead code from nbtsplitloc.c.
It doesn't make sense to consider the possibility that there will only
be one candidate split point when choosing among split points to find
the split with the lowest penalty. This is a vestige of an earlier
version of the patch that became commit fab25024.
; array in any case, so if we're not going to sweat about OOM for the
> message then I'm not sure we need to be paranoid about the sort memory.
I agree that it isn't worth worrying about an OOM for the sort here.
--
Peter Geoghegan
that even pg_upgrade'd v3 indexes make use of this optimization.
Author: Peter Geoghegan
Reviewed-By: Heikki Linnakangas
Discussion:
https://postgr.es/m/cah2-wzkpkezjrxvr_p7vsy1b-s85e3ghytbzqzr0bkj5lrw...@mail.gmail.com
Branch
--
master
Details
---
https://git.postgresql.org/pg/
Invalidate binary search bounds consistently.
_bt_check_unique() failed to invalidate binary search bounds in the
event of a live conflict following commit e5adcb78. This resulted in
problems after waiting for the conflicting xact to commit or abort. The
subsequent call to _bt_check_unique()
Add test coverage for rootdescend verification.
Commit c1afd175, which added support for rootdescend verification to
amcheck, added only minimal regression test coverage. Address this by
making sure that rootdescend verification is run on a multi-level index.
In passing, simplify some of the
ttempt, as I currently don't have access to any such machine.
Looks like gharial still has problems:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=gharial=2019-03-29%2018%3A30%3A47
--
Peter Geoghegan
Tweak some nbtree-related code comments.
Branch
--
master
Details
---
https://git.postgresql.org/pg/commitdiff/9c7fb7e6d8d0fdcabbdc32daad2159812e538bd5
Modified Files
--
contrib/amcheck/verify_nbtree.c | 2 +-
src/backend/access/nbtree/nbtsearch.c | 3 ++-
On Wed, Mar 27, 2019 at 6:27 PM Tomas Vondra
wrote:
> It's a bit too late for pushing emergency fixes over here, so I'll do
> more testing tomorrow and then push.
The buildfarm is still almost all-red now. Can you estimate how long
it will take to push a fix?
--
Peter Geoghegan
On Thu, Mar 28, 2019 at 5:28 AM Andres Freund wrote:
> Hm, good catch. I don't like this fix very much (even if it were
> commented), but I don't have a great idea right now.
That was just a POC, to verify the problem. Not a proposal.
--
Peter Geoghegan
temporary band-aid fix.
--
Peter Geoghegan
On Thu, Mar 28, 2019 at 8:30 AM Peter Geoghegan wrote:
> I would just hard code something if there needs to be a temporary band-aid
> fix.
I also suggest that you remove the #include for heapam_xlog.h from
both nbtxlog.c and hash_xlog.c.
--
Peter Geoghegan
Fix nbtree high key "continuescan" row compare bug.
Commit 29b64d1d mishandled skipping over truncated high key attributes
during row comparisons. The row comparison key matching loop would loop
forever when a truncated attribute was encountered for a row compare
subkey. Fix by following the
for backwards scans. Backward scans continue to
opportunistically check the final non-pivot tuple, which is actually the
first non-pivot tuple on the page (not the last).
Note that even pg_upgrade'd v3 indexes make use of this optimization.
Author: Peter Geoghegan, Heikki Linnakangas
Reviewed-By
Suppress DETAIL output from an event_trigger test.
Suppress 3 lines of unstable DETAIL output from a DROP ROLE statement in
event_trigger.sql. This is further cleanup for commit dd299df8.
Note that the event_trigger test instability issue is very similar to
the recently suppressed foreign_data
Remove unneeded argument from _bt_getstackbuf().
_bt_getstackbuf() is called at exactly two points following commit
efada2b8e92 (one call site is concerned with page splits, while the
other is concerned with page deletion). The parent buffer returned by
_bt_getstackbuf() is write-locked in both
Correct obsolete nbtree page deletion comment.
Commit efada2b8e92, which made the nbtree page deletion algorithm more
robust, removed _bt_getstackbuf() calls from _bt_pagedel(). It failed
to update a comment that referenced the earlier approach. Update the
comment to explain that the
Note case where nbtree VACUUM finishes splits.
The nbtree README claims that VACUUM can never finish interrupted page
splits by design. That isn't entirely accurate, though. Note an
exception to the general rule.
Discussion:
and, so that we don't see the full absolute
> path in the diff header, which makes the diff unnecessarily verbose
> and harder to read.
This broke some of my tooling for quickly reconciling expected and
actual test outputs from my text editor.
I don't think that this was a great idea.
--
Peter Geoghegan
Correct obsolete nbtree page split WAL comment.
Commit 2c03216d831, which revamped the WAL record format, failed to
update a comment referencing the old API. Update the comment.
Branch
--
master
Details
---
Correct obsolete nbtree page split comment.
Commit 40dae7ec537, which made the nbtree page split algorithm more
robust, made _bt_insert_parent() only unlock the right child of the
parent page before inserting a new downlink into the parent. Update a
comment from the Berkeley days claiming that
to give wrong
answers to some queries, and yet not corrupt enough to allow the problem
to be detected without verifying agreement between the leaf page and the
root page, skipping at least one internal page level. The existing
bt_index_parent_check() checks never cross more than a single level.
Au
Grouping relatively similar tuples together is beneficial in
and of itself, since it reduces the number of leaf pages that must be
accessed by subsequent index scans.
Author: Peter Geoghegan
Reviewed-By: Heikki Linnakangas
Discussion:
https://postgr.es/m/cah2-wzmmolnqoj9mad78iqhfwljdszhedrazg
ker column must be used in comparisons for the
restored binary search (unlike the original _bt_check_unique() binary
search, where scankey's heap TID column must be omitted).
Author: Peter Geoghegan, Heikki Linnakangas
Reviewed-By: Heikki Linnakangas, Andrey Lepikhov
Discussion:
https://postgr.es/m/c
by an amount equal to
the space required to store an extra MAXALIGN()'d TID in a new high key
during leaf page splits. The user-facing definition of the "1/3 of a
page" restriction is already imprecise, and so does not need to be
revised. However, there should be a compatibility note in the v1
ee code used to provide mostly-reverse-insertion-order
> scan order.
That's good. I'm trying to fix it by hand right now, in the way that
Andres suggested. It is both tedious and error-prone.
--
Peter Geoghegan
On Wed, Mar 20, 2019 at 3:08 PM Tom Lane wrote:
> And done. Should be possible to revert 7d3bf73ac, if you wish.
Will do.
Thanks!
--
Peter Geoghegan
On Wed, Mar 20, 2019 at 11:30 AM Peter Geoghegan wrote:
> Your work on test stability probably eliminated 98% of the problems.
> It's still early, but the buildfarm is mostly fine.
batfish just had a similar failure, this time in foreign_data -- two
lines of "DETAIL" output ap
On Wed, Mar 20, 2019 at 10:05 AM Peter Geoghegan wrote:
> Make heap TID a tiebreaker nbtree index column.
I see that this has caused SELinux test failures on rhinoceros (the
ddl test fails). It looks like the output order is affected by the
implementation details of nbtree, likely for s
On Wed, Mar 20, 2019 at 11:09 AM Andres Freund wrote:
> FWIW, I just fix up the tests using the regression output from
> rhinoceros when that happens. Sometimes takes more than a single round,
> but it builds frequently enough...
I'll give that a go, provided Tom is okay with it.
ne to rely on the new output ordering, even
though it theoretically isn't any more stable.
--
Peter Geoghegan
Tweak nbtsearch.c function prototype order.
nbtsearch.c's static function prototypes were slightly out of order.
Make the order consistent with static function definition order.
Branch
--
master
Details
---
On Fri, Mar 22, 2019 at 10:38 AM Tom Lane wrote:
> So that means that the de-revert is probably the best option for
> now. Will you do the honors?
I'll take care of that shortly.
Thanks
--
Peter Geoghegan
thing seemed amiss to me
for these reasons.
--
Peter Geoghegan
; log could be pretty unfriendly in some contexts, too.
All of these options seem acceptable. However, the problem is unlikely
to get any worse, so going to the trouble of option 1 or 1a might not
be the best use of time.
--
Peter Geoghegan
Go back to suppressing foreign_data DETAIL test output.
This is almost a straight revert of commit fff518d, which itself was a
revert of 7d3bf73ac.
It turns out that commit 8aa9dd74, which sorted dependent objects before
deletion in DROP OWNED BY, was not sufficient to make all remaining
Revert "Suppress DETAIL output from a foreign_data test."
This should be superseded by commit 8aa9dd74.
Branch
--
master
Details
---
https://git.postgresql.org/pg/commitdiff/fff518d051285bc47e2694a349d410e01972730b
Modified Files
--
; not only that one. They should not be necessary any more, and
> they might be hiding things we need to know about, now or in
> the future. But I haven't got round to it.
Seems like a useful goal.
--
Peter Geoghegan
usting the output, rather than waiting for your
patch. Whether or not the verbosity hack can be ripped out can be
considered later, in a separate pass.
--
Peter Geoghegan
1 - 100 of 702 matches
Mail list logo
