SEO

Tom Lane wrote:
> Since Simon seems intent on hacking something in there, here is a patch
> that I think is actually sane for improving operator lookup speed.
> This patch caches all lookups, exact or ambiguous (since even the exact
> ones require multiple cache searches in common cases); and behaves sanely
> in the presence of search_path, pg_operator, or pg_cast changes.
>
> I see about a 45% speedup (2110 vs 1445 tps) on Guillame Smet's test case.
> On straight pgbench --- which has no ambiguous operators, plus it's not
> read-only --- it's hard to measure any consistent speedup, but I can say
> that it's not slower. Some other test cases would be nice.
>
> I went through the code that's being bypassed in some detail, to see what
> dependencies were being skipped over. I think that as long as we assume
> that no *existing* type changes its domain base type, typtype, array
> status, type category, or preferred-type status, we don't need to flush
> the cache on pg_type changes. This is a good thing since pg_type changes
> frequently (eg, at temp table create or drop).
>
> The only case that I believe to be unhandled is that the cache doesn't pay
> attention to ALTER TABLE ... INHERIT / NO INHERIT events. This means it
> is theoretically possible to return the wrong operator if an operator
> takes a complex type as input and the calling situation involves another
> complex type whose inheritance relationship to that one changes. That's
> sufficiently far out of the normal case that I'm not very worried about it
> (in fact, we probably have bugs in that area even without this patch,
> since for instance cached plans don't respond to such changes either).
> We could plug the hole by forcing a system-wide cache reset during ALTER
> TABLE ... INHERIT / NO INHERIT, if anyone insists.
>
> I'm not entirely happy about applying a patch like this so late in
> the beta cycle, but I'd much rather do this than than any of the
> less-than-half-baked ideas that have been floated in the discussion
> so far.

Thanks for the patch. I can see it is clearly of significant size.

I also noted that you found that the case of:

SELECT col FROM tab WHERE text_col = 'ABC';

also took 37% of CPU in January, I think meaning we had this problem in
8.2.

On the one hand we have a pretty significant patch that we might apply.
It gives us a major speedup (+30%) for a common query type. I assume
8.3 was slightly slower than 8.2 only because we have a few more
pg_catalog entries in 8.3 than 8.2. (I am still baffled how a lookup
function could take so much CPU compared to what else is done for a
query.)

We are also talking about catlog changes for 8.3. Are we comfortable
doing catalog changes between the beta and RC? I am wondering if the
right plan is to have someone else review your patch, apply it, make the
catalog changes, and release another beta this weekend. Give the beta
one week of testing and go for RC. That gives us testing of the patch,
and testing of the catalog changes before going to RC1.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Bruce Momjian <bruce@momjian.us> writes:
> We are also talking about catlog changes for 8.3. Are we comfortable
> doing catalog changes between the beta and RC?

The catalog changes in question seem entirely safe ... certainly much
more so than this patch ...

I do see your point that another beta might be prudent, but on the other
hand I'm not sure it's really needed. The only difference between a
beta and an RC is that we try not to change the code anymore after RC.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > We are also talking about catlog changes for 8.3. Are we comfortable
> > doing catalog changes between the beta and RC?
>
> The catalog changes in question seem entirely safe ... certainly much
> more so than this patch ...
>
> I do see your point that another beta might be prudent, but on the other
> hand I'm not sure it's really needed. The only difference between a
> beta and an RC is that we try not to change the code anymore after RC.

To me RC means we think this might be the release candidate and I would
like to get some testing in of this in beta before hitting that point.

And an additional beta might encourage more testing too.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Bruce Momjian wrote:
> Tom Lane wrote:
>> Bruce Momjian <bruce@momjian.us> writes:
>>> We are also talking about catlog changes for 8.3. Are we comfortable
>>> doing catalog changes between the beta and RC?
>> The catalog changes in question seem entirely safe ... certainly much
>> more so than this patch ...
>>
>> I do see your point that another beta might be prudent, but on the other
>> hand I'm not sure it's really needed. The only difference between a
>> beta and an RC is that we try not to change the code anymore after RC.
>
> To me RC means we think this might be the release candidate and I would
> like to get some testing in of this in beta before hitting that point.
>
> And an additional beta might encourage more testing too.

I agree with Bruce here. If you want to apply that operator lookup cache
patch, I would have another beta. (And I am not personally against it,
because I feel major performance fixes may sometimes slip in as "bug fixes".)
If you all decide against that patch, we might as well just go for RC1. The
catalog changes seem rather trivial, and just a required initdb is no
reason for calling it another beta, IMHO.

Great work on that patch, btw.!

Best Regards
Michael Paesold

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

On Nov 27, 2007 6:34 AM, Bruce Momjian <bruce@momjian.us> wrote:
> And an additional beta might encourage more testing too.

I'm not that sure of this point. I'm really worried about the lack of
people testing 8.3 at the moment. We have really too little feedback.
Perhaps they didn't meet any problem but even that could be good to
know.

That said, if this patch is applied, another beta is the reasonable
way to go. Not sure it's worth it though.

--
Guillaume

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match