SEO

"Marko Kreen" <markokr@gmail.com> writes:
> On 5/15/08, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> "Marko Kreen" <markokr@gmail.com> writes:
>>> Eg. how does src/backend/parser/gram.c not leak memory on syntax error?
>>
>> It's not a leak because the memory can be re-used during the next
>> command.

> I may be blind, but I don't see any static variables there.

Sorry, I was confusing bison with flex --- there are static variables
pointing at buffers within a flex scanner.

For bison it looks like defining YYSTACK_ALLOC/YYSTACK_FREE as
palloc/pfree might be a sane thing to do, but I haven't tested it.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 5/15/08, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Marko Kreen" <markokr@gmail.com> writes:
> > On 5/15/08, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> "Marko Kreen" <markokr@gmail.com> writes:
> >>> Eg. how does src/backend/parser/gram.c not leak memory on syntax error?
> >>
> >> It's not a leak because the memory can be re-used during the next
> >> command.
>
> > I may be blind, but I don't see any static variables there.
>
> Sorry, I was confusing bison with flex --- there are static variables
> pointing at buffers within a flex scanner.
>
> For bison it looks like defining YYSTACK_ALLOC/YYSTACK_FREE as
> palloc/pfree might be a sane thing to do, but I haven't tested it.

Ok, so parser.y is now fine.

Now I must admit I do the same hack in scanner.l, but because it keeps
static references, there is always call to plproxy_yylex_destroy() in
places that throw exceptions (all of flex/bison/plproxy exceptions
go through single function).

Reason for that is again the fact that I could not wrap my brain around
flex memory handling. And the hacks in src/backend/parser/scan.l are also
somethat mystery to me. When using palloc() I can be sure of the flow,
and if something goes wrong it crashes instead leaking, so it can
be fixed immidately.

But now that I think about it, the scheme fails if palloc() itself
throws exception. It can be fixed by calling following function
on parser entry:

void plproxy_yylex_startup(void)
{
#if FLXVER < 2005031
(YY_CURRENT_BUFFER) = NULL;
#else
(yy_buffer_stack) = NULL;
#endif
plproxy_yylex_destroy();
}

This is pretty hairy, but anything near flex is hairy. Such function
also would drop the requirement that plproxy_yylex_destroy() must always
be called. Then we could keep current simple always-from-scratch allocation
pattern but more robust.


Or we could go back to default malloc usage. I somewhat doubt it will
be much cleaner, it needs lot more faith in sanity of flex which I dont
have.
OTOH, considering that now here the possibility of others reviewing the
result is lot higher than before it can be attempted.

--
marko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, 15 May 2008, Marko Kreen wrote:

> On 5/15/08, Josh Berkus <josh@agliodbs.com> wrote:
>> Has PL/proxy been tested on other OSes? FreeBSD/Solaris/Windows?
>
> It definitely works on Linux and MacOS. I've seen ports for *BSD.
> I think any unix-like OS-es should work fine.

I've compiled it with a minor Makefile modification on Solaris and it
seems to work (it passes all the unit tests but the one involving
random; we might want to rethink that test so 'passes' on platforms with
different implementations of random()). However, I haven't deployed it into
a production environment.

Somewhat unrelated, I can see use-cases for replacing the call to random()
with something that allows user defined polices for RUN ON ANY.


>
> In fact, only syscalls it does on its own are gettimeofday() and poll()
> [or select()], otherwise it calls either core or libpq functions.
> So I see no reason why it shouldnt already work on Windows.
>
> The biggest portability problem thus far has been scanner.l, which at
> the beginning was written for flex 2.5.33+, but 2.5.4 is still pretty
> widespread. But this I fixed in 2.0.3.
>
> Hmm.. Now that I think about it, in my effort to remove malloc() calls
> in both scanner and parser I told bison to use alloca(). Is it portability
> concern? Easy to fix, just need to be careful not to create memleaks.
>
> --
> marko
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Steve,


> I've compiled it with a minor Makefile modification on Solaris and it
> seems to work (it passes all the unit tests but the one involving
> random; we might want to rethink that test so 'passes' on platforms with
> different implementations of random()). However, I haven't deployed it
> into a production environment.

Confirmed on my copy of Nevada 70 (opensolaris) as well.

Looks like we're good on OS support. Now time to start playing with the
semantics ...

--Josh

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 5/17/08, Steve Singer <ssinger_pg@sympatico.ca> wrote:
> Somewhat unrelated, I can see use-cases for replacing the call to random()
> with something that allows user defined polices for RUN ON ANY.

Well, thats why the RUN ON userfunc(..); exists. Also notice the function
can tag more that one partition for execution.

Or did you mean something else than partition selection by "user
defined policy"?

--
marko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sat, 17 May 2008, Marko Kreen wrote:

> On 5/17/08, Steve Singer <ssinger_pg@sympatico.ca> wrote:
>> Somewhat unrelated, I can see use-cases for replacing the call to random()
>> with something that allows user defined polices for RUN ON ANY.
>
> Well, thats why the RUN ON userfunc(..); exists. Also notice the function
> can tag more that one partition for execution.
>
> Or did you mean something else than partition selection by "user
> defined policy"?


I see RUN ON userfunc() as being for partitioning where the correctness
requires that the query be run on the result of userfunc. I see RUN ON ANY
as being for load-balancing. You might want to RUN ON ANY with a round
robin balancing, or maybe consider the load of servers for doing the
balancing.

In the case of RUN ON ANY it seems that the database the query gets sent
to doesn't matter. It might make sense for plproxy to try the next database
if it can't connect to the first one it picks. You wouldn't want this for
partitioning queries. If plproxy knows if you mean 'the query has to be run
on these partitions' versus 'run the query on any partition, using method x
to choose' then that type of things would be possible.

Steve

>
> --
> marko
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 5/18/08, Steve Singer <ssinger_pg@sympatico.ca> wrote:
> On Sat, 17 May 2008, Marko Kreen wrote:
> > On 5/17/08, Steve Singer <ssinger_pg@sympatico.ca> wrote:
> > > Somewhat unrelated, I can see use-cases for replacing the call to
> random()
> > > with something that allows user defined polices for RUN ON ANY.
> >
> > Well, thats why the RUN ON userfunc(..); exists. Also notice the function
> > can tag more that one partition for execution.
> >
> > Or did you mean something else than partition selection by "user
> > defined policy"?
>
> I see RUN ON userfunc() as being for partitioning where the correctness
> requires that the query be run on the result of userfunc. I see RUN ON ANY
> as being for load-balancing.

Here you see wrong. You should see RUN ON ANY simply as a shortcut
for RUN ON random(); The actual random() would not work as it returns
floats, but equivalent integer random();

So if you want smarter ANY, just implement your function. I don't see
any need for tunable ANY.

> You might want to RUN ON ANY with a round
> robin balancing, or maybe consider the load of servers for doing the
> balancing.
>
> In the case of RUN ON ANY it seems that the database the query gets sent to
> doesn't matter. It might make sense for plproxy to try the next database if
> it can't connect to the first one it picks. You wouldn't want this for
> partitioning queries. If plproxy knows if you mean 'the query has to be run
> on these partitions' versus 'run the query on any partition, using method x
> to choose' then that type of things would be possible.

Ok, here are 2 feature requests, that we have thought ourselves too:

RUN ON LEAST LOADED;

Sorry, this is unimplementable with current PL/Proxy design, as the
per-backend PL-s do not coordinate their usage. And this is deliberate.

If you want to implement this the design should look exactly like
PL/Proxy 1 - each PL does special connection to special pooler that
is responsible for partition selection and thus has information
about partition usage. And the complexity went through the roof...

You may achieve the same effect with smart tcp proxy or if not you
can write load-balancing feature with load check for PgBouncer.

RUN ON ANY PICK NEXT ON ERROR;

This is implementable. But we have not found an actual need for it
ourselves. So I have bothered to implement it as otherwise plproxy
would have another "implementable" and "maybe nice to have" feature
without actual reason like CONNECT, SELECT and get_cluster_config()
turned out to be.

OTOH, here we don't use read-only load balancing much. And such feature
does not make sense when partitioning is used. But it indeed makes
sense for load-balancing. So I'm not against adding it.

--
marko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, 2008-05-14 at 23:29 +0300, Marko Kreen wrote:

> There are few code beautification ideas on which I'd like to
> get feedback from wider audience:
>
> - Drop CONNECT statement. This was added to make creating
> simple dblink style calls easier, but actually its not
> maintainable compared to CLUSTER, it can be used only
> for occasional hacks. OTOH, if we want to deprecate
> dblink and replace it with PL/Proxy, it should probably stay.
>
> - Drop SELECT statement. This was added as it was simple to do
> and also to be straightforward replacement for dblink. But
> it's conceptually ugly. Also it gives impression that there
> will be UPDATE, DELETE and IF... which will not happen.

I'd also suggest one feature request

- Add COPY FROM/TO support

The way It could be done is similar to (now deprecated) SELECT

CREATE FUNCTION copy_users_to_partitions()
RETURNS VOID AS $$
CLUSTER 'userdb';
RUN ON hashtext(text::$1);
COPY users FROM stdin;
$$ LANGUAGE plproxy;

and it should be used like COPY is currently

proxydb# SELECT copy_users_to_partitions();
bob bobspwd bob@email.com
ben benspwd ben@email.com
....
amy amyspwd amy@email.com
..
copy_users_to_partitions
--------------------------

(1 row)

I am not sure how easy it is to get access to "stdin" from inside a function,
but the version of COPY which copies from filesystem can surely be done.

On slaves it will always be plain COPY with STDIN/STDOUT.

As this reintroduces "direct access to partition tables" feature removed with
SELECT, it should be a feature that can be used by superusers only.

Notice that field type should be given, as it can't be deduced from arguments.

COPY .. TO would usually (but not neccessarily) be RUN ON ALL

--------------------
Hannu




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers