Thread: updated qCache

updated qCache

From
Neil Conway
Date:
Hi all,

Here's an updated version of the experimental qCache patch I
posted a couple days ago (which is a port of Karel Zak's 7.0
work to CVS HEAD).

Changes:

- fix segfault in EXECUTE under some circumstances (reported
  by Barry Lind)
- fix some memory leaks  (thanks to Karel Zak)
- write more regression tests (make check still won't pass)
- re-diff against CVS HEAD
- more code cleanup, minor tweaks

However, I've tentatively decided that I think the best
way to go forward is to rewrite this code. IMHO the utility of
plans cached in shared memory is fairly limited, but the
code that implements this adds a lot of complex to the patch.
I'm planning to re-implement PREPARE/EXECUTE with support only
for locally-prepared plans, using the existing patch as a
guide. The result should be a simpler patch -- once it's
in CVS we can worry about more advanced plan caching
techiques. Any complaints/comments on this plan?

Cheers,

Neil

--
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC

Attachment

Re: updated qCache

From
"Dann Corbit"
Date:
-----Original Message-----
From: Neil Conway [mailto:nconway@klamath.dyndns.org]
Sent: Wednesday, April 17, 2002 2:18 PM
To: PostgreSQL Hackers
Subject: [HACKERS] updated qCache


Hi all,

Here's an updated version of the experimental qCache patch I
posted a couple days ago (which is a port of Karel Zak's 7.0
work to CVS HEAD).

Changes:

- fix segfault in EXECUTE under some circumstances (reported by Barry Lind)
- fix some memory leaks  (thanks to Karel Zak)
- write more regression tests (make check still won't pass)
- re-diff against CVS HEAD
- more code cleanup, minor tweaks

However, I've tentatively decided that I think the best
way to go forward is to rewrite this code. IMHO the utility of
plans cached in shared memory is fairly limited, but the
code that implements this adds a lot of complex to the patch.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
DC% Why do you imagine that the utility is limited?
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

I'm planning to re-implement PREPARE/EXECUTE with support only
for locally-prepared plans, using the existing patch as a
guide. The result should be a simpler patch -- once it's
in CVS we can worry about more advanced plan caching
techiques. Any complaints/comments on this plan?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
DC% Why not allow both kinds and make it configurable...
DC% local/shared/both.
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<


Re: updated qCache

From
Neil Conway
Date:
On Wed, 17 Apr 2002 14:34:45 -0700
"Dann Corbit" <DCorbit@connx.com> wrote:
> However, I've tentatively decided that I think the best
> way to go forward is to rewrite this code. IMHO the utility of
> plans cached in shared memory is fairly limited, but the
> code that implements this adds a lot of complex to the patch.
> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> DC% Why do you imagine that the utility is limited?

(1) It's difficult to tell whether a given plan has already been   prepared: the server could have been restarted in
themean-time,   for example. We could allow app developers to check if a   given has already been prepared, but that's
inconvenient,  and the benefits seem fairly small.
 

(2) Shared memory is a bad storage location for variable-sized   data, like query plans. What happens if you're asked
to  cache a plan larger than the free space in the shared cache?   You could perhaps free up some space by removing
another  entry, but that means that every invocation of EXECUTE   needs to be prepared for the target plan to have been
 evicted from the cache -- which is irritating.
 

(3) Managing concurrent access to the shared cache may (or may   not) be a performance issue.

I'm not saying it's a bad idea, I just think I'd like to
concentrate on the locally-cached plans for now and see if
there is a need to add shared plans later.

> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
> 
> I'm planning to re-implement PREPARE/EXECUTE with support only
> for locally-prepared plans, using the existing patch as a
> guide. The result should be a simpler patch -- once it's
> in CVS we can worry about more advanced plan caching
> techiques. Any complaints/comments on this plan?
> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> DC% Why not allow both kinds and make it configurable...
> DC% local/shared/both.

That's what the current patch does.

Cheers,

Neil

-- 
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC


Re: updated qCache

From
Tom Lane
Date:
Neil Conway <nconway@klamath.dyndns.org> writes:
> I'm planning to re-implement PREPARE/EXECUTE with support only
> for locally-prepared plans, using the existing patch as a
> guide. The result should be a simpler patch -- once it's
> in CVS we can worry about more advanced plan caching
> techiques. Any complaints/comments on this plan?

That's what I wanted from day one ;-)
        regards, tom lane


Re: updated qCache

From
"Christopher Kings-Lynne"
Date:
> Neil Conway <nconway@klamath.dyndns.org> writes:
> > I'm planning to re-implement PREPARE/EXECUTE with support only
> > for locally-prepared plans, using the existing patch as a
> > guide. The result should be a simpler patch -- once it's
> > in CVS we can worry about more advanced plan caching
> > techiques. Any complaints/comments on this plan?
>
> That's what I wanted from day one ;-)

So with this scheme, people just have to be careful to use a connection pool
/ persistent connections?

Chris



Re: updated qCache

From
"Christopher Kings-Lynne"
Date:
> Neil Conway <nconway@klamath.dyndns.org> writes:
> > I'm planning to re-implement PREPARE/EXECUTE with support only
> > for locally-prepared plans, using the existing patch as a
> > guide. The result should be a simpler patch -- once it's
> > in CVS we can worry about more advanced plan caching
> > techiques. Any complaints/comments on this plan?
>
> That's what I wanted from day one ;-)

You know, if we had a threaded backend, we wouldn't have any of these
problems :)

Chris



Re: updated qCache

From
Karel Zak
Date:
On Wed, Apr 17, 2002 at 05:17:51PM -0400, Neil Conway wrote:
> Hi all,
> 
> Here's an updated version of the experimental qCache patch I
> posted a couple days ago (which is a port of Karel Zak's 7.0
> work to CVS HEAD).
> 
> Changes:
> 
> - fix segfault in EXECUTE under some circumstances (reported
>   by Barry Lind)
> - fix some memory leaks  (thanks to Karel Zak)
> - write more regression tests (make check still won't pass)
> - re-diff against CVS HEAD
> - more code cleanup, minor tweaks
> 
> However, I've tentatively decided that I think the best
> way to go forward is to rewrite this code. IMHO the utility of
> plans cached in shared memory is fairly limited, but the
> code that implements this adds a lot of complex to the patch.
> I'm planning to re-implement PREPARE/EXECUTE with support only
> for locally-prepared plans, using the existing patch as a
> guide. The result should be a simpler patch -- once it's
> in CVS we can worry about more advanced plan caching
> techiques. Any complaints/comments on this plan?
I agree too :-) I think remove the shared memory code from this patchis easy and local memory storage is there already
done.
       Karel

-- Karel Zak  <zakkr@zf.jcu.cz>http://home.zf.jcu.cz/~zakkr/C, PostgreSQL, PHP, WWW, http://docs.linux.cz,
http://mape.jcu.cz


Re: updated qCache

From
Karel Zak
Date:
On Wed, Apr 17, 2002 at 06:05:59PM -0400, Neil Conway wrote:
> On Wed, 17 Apr 2002 14:34:45 -0700
> 
> I'm not saying it's a bad idea, I just think I'd like to
> concentrate on the locally-cached plans for now and see if
> there is a need to add shared plans later.
Yes, later we can use shared memory buffer as "pipe" betweenbackends:
Backend A:                            Backend B:local-memory-query-plan --> shmem --> local-memory-query-planIn this
ideais in the shared memory one query-plan only and backends use it for plan copying from "A" to "B".
 
It require persistent backends of course.
       Karel
PS. it's idea only and nothing other, the original qcache was idea    only too :-)
-- Karel Zak  <zakkr@zf.jcu.cz>http://home.zf.jcu.cz/~zakkr/C, PostgreSQL, PHP, WWW, http://docs.linux.cz,
http://mape.jcu.cz


Re: updated qCache

From
Karel Zak
Date:
On Wed, Apr 17, 2002 at 05:17:51PM -0400, Neil Conway wrote:
> Hi all,
> 
> Here's an updated version of the experimental qCache patch I
> posted a couple days ago (which is a port of Karel Zak's 7.0
> work to CVS HEAD).
I have a question, what the Dllist and malloc()? I think it'snothing nice for local-memory cache too.
There is needful destory cached planns by one call of MemoryContextDelete(). I hope that Neil wants still use original
"context-per-plan"cache.
 
     Karel

-- Karel Zak  <zakkr@zf.jcu.cz>http://home.zf.jcu.cz/~zakkr/C, PostgreSQL, PHP, WWW, http://docs.linux.cz,
http://mape.jcu.cz