Thread: Buildfarm failure analysis: penguin on 7.4 branch

Buildfarm failure analysis: penguin on 7.4 branch

From
Tom Lane
Date:
Buildfarm member penguin has never got past this point in half a year of
trying:

creating template1 database in
/home/postgres/pgfarmbuild/REL7_4_STABLE/pgsql.7701/src/test/regress/./tmp_check/data/base/1...ok
 
initializing pg_shadow... TRAP: FailedAssertion("!(StrategyEvaluationIsValid(evaluation))", File: "istrat.c", Line:
273)

Unfortunately it never will :-(, and I'd recommend removing 7.4 from its
to-do list.

The problem here appears to be the same unusual struct packing rules
that make tsearch2 fail on this machine in the 8.0 branch.  The old
"index strategy" code assumes that given this set of struct
declarations:

typedef uint16 StrategyNumber;

typedef struct StrategyOperatorData
{   StrategyNumber strategy;   bits16        flags;            /* scan qualification flags, see skey.h */
} StrategyOperatorData;

typedef struct StrategyTermData
{                                   /* conjunctive term */   uint16        degree;   StrategyOperatorData
operatorData[1];       /* VARIABLE LENGTH ARRAY */
 
} StrategyTermData;                 /* VARIABLE LENGTH STRUCTURE */

the contents of StrategyTermData are equivalent to a uint16 array
containing {degree, strategy1, flags1, strategy2, flags2, ... }.
This is true on 99% of machines out there, but the compiler penguin is
using thinks it should align StrategyOperatorData on a word boundary,
and so there is padding between the "degree" and the first array
element.

The compiler is within its rights to do this per the ANSI C spec, but
considering that we'd never seen this problem in ten years of Postgres
use and that the struct is gone (for unrelated reasons) in PG 8.0 and
up, I don't think anyone is likely to feel like messing with the 7.*
code to fix it.

(This is all extrapolation, mind you, but given what we found out about
the problem with tsearch2, I feel reasonably confident in the analysis.)
        regards, tom lane


Re: Buildfarm failure analysis: penguin on 7.4 branch

From
"Jim Buttafuoco"
Date:
Tom,

I agree with NOT fixing the tsearch2 code for this failure in 7.4.  I have left penguin building 7.4 just to see if
the
core code continues to compile.  I would be nice if the build farm code would let me exclude a contrib module if
necessary.  What do you think Andrew?

Jim


---------- Original Message -----------
From: Tom Lane <tgl@sss.pgh.pa.us>
To: jim@buttafuoco.net
Cc: Andrew Dunstan <andrew@dunslane.net>, pgsql-hackers@postgresql.org
Sent: Mon, 18 Jul 2005 01:05:09 -0400
Subject: [HACKERS] Buildfarm failure analysis: penguin on 7.4 branch

> Buildfarm member penguin has never got past this point in half a year of
> trying:
> 
> creating template1 database in 
> /home/postgres/pgfarmbuild/REL7_4_STABLE/pgsql.7701/src/test/regress/./tmp_check/data/base/1... ok 
> initializing pg_shadow... TRAP: FailedAssertion("!(StrategyEvaluationIsValid(evaluation))", File: "istrat.c",
>  Line: 273)
> 
> Unfortunately it never will :-(, and I'd recommend removing 7.4 from its
> to-do list.
> 
> The problem here appears to be the same unusual struct packing rules
> that make tsearch2 fail on this machine in the 8.0 branch.  The old
> "index strategy" code assumes that given this set of struct
> declarations:
> 
> typedef uint16 StrategyNumber;
> 
> typedef struct StrategyOperatorData
> {
>     StrategyNumber strategy;
>     bits16        flags;            /* scan qualification flags, see skey.h */
> } StrategyOperatorData;
> 
> typedef struct StrategyTermData
> {                                   /* conjunctive term */
>     uint16        degree;
>     StrategyOperatorData operatorData[1];        /* VARIABLE LENGTH ARRAY */
> } StrategyTermData;                 /* VARIABLE LENGTH STRUCTURE */
> 
> the contents of StrategyTermData are equivalent to a uint16 array
> containing {degree, strategy1, flags1, strategy2, flags2, ... }.
> This is true on 99% of machines out there, but the compiler penguin is
> using thinks it should align StrategyOperatorData on a word boundary,
> and so there is padding between the "degree" and the first array
> element.
> 
> The compiler is within its rights to do this per the ANSI C spec, but
> considering that we'd never seen this problem in ten years of Postgres
> use and that the struct is gone (for unrelated reasons) in PG 8.0 and
> up, I don't think anyone is likely to feel like messing with the 7.*
> code to fix it.
> 
> (This is all extrapolation, mind you, but given what we found out about
> the problem with tsearch2, I feel reasonably confident in the analysis.)
> 
>             regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
> 
>                http://www.postgresql.org/docs/faq
------- End of Original Message -------



Re: Buildfarm failure analysis: penguin on 7.4 branch

From
Andrew Dunstan
Date:
Jim,

There seems to be some confusion. The error Tom remarked on 7.4 is an 
initdb failure during the core check tests, not a tsearch2 failure, 
which is why I think he recommends discontinuing to build that branch on 
this machine.

Regarding the tsearch2 failure on the 8.0 branch, I am ambivalent about 
your proposed change, to say the least. Tom made a comment some time ago 
about not hiding errors/limitations, and I agree with him. This is in a 
different class from the geometry tests, where the differences were 
largely cosmetic. I don't want people to have to read the fine print to 
see what tests were excluded - if we show a machine as green it should 
mean that machine passed the whole test suite.

cheers

andrew

Jim Buttafuoco wrote:

>Tom,
>
>I agree with NOT fixing the tsearch2 code for this failure in 7.4.  I have left penguin building 7.4 just to see if
the
>core code continues to compile.  I would be nice if the build farm code would let me exclude a contrib module if
>necessary.  What do you think Andrew?
>
>Jim
>
>
>---------- Original Message -----------
>From: Tom Lane <tgl@sss.pgh.pa.us>
>To: jim@buttafuoco.net
>Cc: Andrew Dunstan <andrew@dunslane.net>, pgsql-hackers@postgresql.org
>Sent: Mon, 18 Jul 2005 01:05:09 -0400
>Subject: [HACKERS] Buildfarm failure analysis: penguin on 7.4 branch
>
>  
>
>>Buildfarm member penguin has never got past this point in half a year of
>>trying:
>>
>>creating template1 database in 
>>/home/postgres/pgfarmbuild/REL7_4_STABLE/pgsql.7701/src/test/regress/./tmp_check/data/base/1... ok 
>>initializing pg_shadow... TRAP: FailedAssertion("!(StrategyEvaluationIsValid(evaluation))", File: "istrat.c",
>> Line: 273)
>>
>>Unfortunately it never will :-(, and I'd recommend removing 7.4 from its
>>to-do list.
>>
>>The problem here appears to be the same unusual struct packing rules
>>that make tsearch2 fail on this machine in the 8.0 branch.  The old
>>"index strategy" code assumes that given this set of struct
>>declarations:
>>
>>typedef uint16 StrategyNumber;
>>
>>typedef struct StrategyOperatorData
>>{
>>    StrategyNumber strategy;
>>    bits16        flags;            /* scan qualification flags, see skey.h */
>>} StrategyOperatorData;
>>
>>typedef struct StrategyTermData
>>{                                   /* conjunctive term */
>>    uint16        degree;
>>    StrategyOperatorData operatorData[1];        /* VARIABLE LENGTH ARRAY */
>>} StrategyTermData;                 /* VARIABLE LENGTH STRUCTURE */
>>
>>the contents of StrategyTermData are equivalent to a uint16 array
>>containing {degree, strategy1, flags1, strategy2, flags2, ... }.
>>This is true on 99% of machines out there, but the compiler penguin is
>>using thinks it should align StrategyOperatorData on a word boundary,
>>and so there is padding between the "degree" and the first array
>>element.
>>
>>The compiler is within its rights to do this per the ANSI C spec, but
>>considering that we'd never seen this problem in ten years of Postgres
>>use and that the struct is gone (for unrelated reasons) in PG 8.0 and
>>up, I don't think anyone is likely to feel like messing with the 7.*
>>code to fix it.
>>
>>(This is all extrapolation, mind you, but given what we found out about
>>the problem with tsearch2, I feel reasonably confident in the analysis.)
>>
>>            regards, tom lane
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 3: Have you checked our extensive FAQ?
>>
>>               http://www.postgresql.org/docs/faq
>>    
>>
>------- End of Original Message -------
>
>
>---------------------------(end of broadcast)---------------------------
>TIP 4: Have you searched our list archives?
>
>               http://archives.postgresql.org
>
>  
>


Re: Buildfarm failure analysis: penguin on 7.4 branch

From
"Jim Buttafuoco"
Date:
must be early in the morning.  I agree with removing penguin from the 7.4 branch.  I have removed it on my end.
Please
delete it from the database



---------- Original Message -----------
From: Andrew Dunstan <andrew@dunslane.net>
To: jim@contactbda.com
Cc: Tom Lane <tgl@sss.pgh.pa.us>, jim@buttafuoco.net, pgsql-hackers@postgresql.org
Sent: Mon, 18 Jul 2005 09:38:56 -0400
Subject: Re: [HACKERS] Buildfarm failure analysis: penguin on 7.4 branch

> Jim,
> 
> There seems to be some confusion. The error Tom remarked on 7.4 is an 
> initdb failure during the core check tests, not a tsearch2 failure, 
> which is why I think he recommends discontinuing to build that branch on 
> this machine.
> 
> Regarding the tsearch2 failure on the 8.0 branch, I am ambivalent about 
> your proposed change, to say the least. Tom made a comment some time ago 
> about not hiding errors/limitations, and I agree with him. This is in a 
> different class from the geometry tests, where the differences were 
> largely cosmetic. I don't want people to have to read the fine print to 
> see what tests were excluded - if we show a machine as green it should 
> mean that machine passed the whole test suite.
> 
> cheers
> 
> andrew
> 
> Jim Buttafuoco wrote:
> 
> >Tom,
> >
> >I agree with NOT fixing the tsearch2 code for this failure in 7.4.  I have left penguin building 7.4 just to see if
the
> >core code continues to compile.  I would be nice if the build farm code would let me exclude a contrib module if
> >necessary.  What do you think Andrew?
> >
> >Jim
> >
> >
> >---------- Original Message -----------
> >From: Tom Lane <tgl@sss.pgh.pa.us>
> >To: jim@buttafuoco.net
> >Cc: Andrew Dunstan <andrew@dunslane.net>, pgsql-hackers@postgresql.org
> >Sent: Mon, 18 Jul 2005 01:05:09 -0400
> >Subject: [HACKERS] Buildfarm failure analysis: penguin on 7.4 branch
> >
> >  
> >
> >>Buildfarm member penguin has never got past this point in half a year of
> >>trying:
> >>
> >>creating template1 database in 
> >>/home/postgres/pgfarmbuild/REL7_4_STABLE/pgsql.7701/src/test/regress/./tmp_check/data/base/1... ok 
> >>initializing pg_shadow... TRAP: FailedAssertion("!(StrategyEvaluationIsValid(evaluation))", File: "istrat.c",
> >> Line: 273)
> >>
> >>Unfortunately it never will :-(, and I'd recommend removing 7.4 from its
> >>to-do list.
> >>
> >>The problem here appears to be the same unusual struct packing rules
> >>that make tsearch2 fail on this machine in the 8.0 branch.  The old
> >>"index strategy" code assumes that given this set of struct
> >>declarations:
> >>
> >>typedef uint16 StrategyNumber;
> >>
> >>typedef struct StrategyOperatorData
> >>{
> >>    StrategyNumber strategy;
> >>    bits16        flags;            /* scan qualification flags, see skey.h */
> >>} StrategyOperatorData;
> >>
> >>typedef struct StrategyTermData
> >>{                                   /* conjunctive term */
> >>    uint16        degree;
> >>    StrategyOperatorData operatorData[1];        /* VARIABLE LENGTH ARRAY */
> >>} StrategyTermData;                 /* VARIABLE LENGTH STRUCTURE */
> >>
> >>the contents of StrategyTermData are equivalent to a uint16 array
> >>containing {degree, strategy1, flags1, strategy2, flags2, ... }.
> >>This is true on 99% of machines out there, but the compiler penguin is
> >>using thinks it should align StrategyOperatorData on a word boundary,
> >>and so there is padding between the "degree" and the first array
> >>element.
> >>
> >>The compiler is within its rights to do this per the ANSI C spec, but
> >>considering that we'd never seen this problem in ten years of Postgres
> >>use and that the struct is gone (for unrelated reasons) in PG 8.0 and
> >>up, I don't think anyone is likely to feel like messing with the 7.*
> >>code to fix it.
> >>
> >>(This is all extrapolation, mind you, but given what we found out about
> >>the problem with tsearch2, I feel reasonably confident in the analysis.)
> >>
> >>            regards, tom lane
> >>
> >>---------------------------(end of broadcast)---------------------------
> >>TIP 3: Have you checked our extensive FAQ?
> >>
> >>               http://www.postgresql.org/docs/faq
> >>    
> >>
> >------- End of Original Message -------
> >
> >
> >---------------------------(end of broadcast)---------------------------
> >TIP 4: Have you searched our list archives?
> >
> >               http://archives.postgresql.org
> >
> >  
> >
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster
------- End of Original Message -------



Re: Buildfarm failure analysis: penguin on 7.4 branch

From
Andrew Dunstan
Date:

Jim Buttafuoco wrote:

>must be early in the morning.  I agree with removing penguin from the 7.4 branch.  I have removed it on my end.
Please
>delete it from the database
>  
>

No need. In 27 days it will drop off the dashboard.

cheers

andrew