Re: Delaying/avoiding BTreeTupleGetNAtts() call within _bt_compare()

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Peter Geoghegan <pg(at)bowt(dot)ie>
Cc: Floris Van Nee <florisvannee(at)optiver(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: Delaying/avoiding BTreeTupleGetNAtts() call within _bt_compare()
Date: 2020-02-19 00:44:42
Message-ID: CA+hUKGLGcJeEVCUbUbAR5mtSQo92QT7QBOGs=nOU=T9zOpsADw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Feb 19, 2020 at 1:35 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
> On Tue, Feb 18, 2020 at 12:55 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> > The cfbot seems to be showing "pg_regress: initdb failed" on Ubuntu,
> > with an assertion failure like this:
> >
> > #2 0x00000000008e594f in ExceptionalCondition
> > (conditionName=conditionName(at)entry=0x949098 "BTreeTupleGetNAtts(itup,
> > rel) >= key->keysz", errorType=errorType(at)entry=0x938a7d
> > "FailedAssertion", fileName=fileName(at)entry=0x949292 "nbtsearch.c",
>
> This is a legitimate bug in v1 of the patch, which was written in a
> hurry. v2 does not have the problem. Floris inadvertently created a
> separate thread for this same patch, which I responded to when posting
> v2. I've now updated the CF entry for this patch [1] to have both
> threads.
>
> BTW, I've noticed that CF Tester is wonky with patches that have
> multiple threads with at least one patch file posted to each thread.
> The deduplication patch [2] has this problem, for example. It would be
> nice if CF Tester knew to prefer one thread over another based on a
> simple rule, like "consistently look for patch files on the first
> thread connected to a CF app entry, never any other thread".

Ahh. Well I had that rule early on, and then had the problem that
some discussions move entirely to a second or third thread and left it
testing some ancient stale patch.

> Maybe you'd rather not go that way -- I guess that it would break
> other cases, such as the CF app entry for this patch (which now
> technically has one thread that supersedes the other). Perhaps a
> compromise is possible. At a minimum, CF Tester should not look for a
> patch on the (say) second thread of a CF app entry for a patch just
> because somebody posted an e-mail to that thread (an e-mail that did
> not contain a new patch). CF Tester will do this even though there is
> a more recent patch on the first thread of the CF app entry, that has
> already been accepted as passing by CFTester. I believe that CF Tester
> will actually pingpong back and forth between the same two patches on
> each thread as e-mail is sent to each thread, without anybody ever
> posting a new patch.

Yeah, for CF entries with multiple threads, it currently looks at
whichever thread has the most recent email on it, and then it finds
the most recent patch set on that thread. Perhaps it should look at
all the registered threads and find the message with the newest patch
set across all of them, as you say. I will look into that.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kohei KaiGai 2020-02-19 01:12:19 PL/Python - lifetime of variables?
Previous Message Peter Geoghegan 2020-02-19 00:35:38 Re: Delaying/avoiding BTreeTupleGetNAtts() call within _bt_compare()