Re: [PATCH] pgbench: new feature allowing to launch shell commands

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] pgbench: new feature allowing to launch shell commands
Date: 2009-09-22 01:58:14
Message-ID: c64c5f8b0909211858h279aa33chaeda32ddec196111@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Sorry for my late reply again :o)
You will find my answers on-the-line.

> > You really should be returning a value at the point since the function
> > signature defines a return type. If not the function should be void,
> > which it cannot be in this context since it is used for boolean tests
> > elsewhere. The returns in question are all part of error blocks and
> > should return false.

The function doCustom is defined with a void.
I agree that it is far cleaner to return a value but by returning in my own
code part a boolean or an integer, it will have a large impact on other
parts of the code that are dealing with the original command types of
pgbench.
By the way, the errors created by doCustom are managed through CState for
each customer, so letting it like it is is far sufficient and has no impact
on other parts of the code.

> So I get this output with and with out the shell command in there,
> against the unpatched and patched version on pgbench. The commands you
> sent will not work with this script since it is using prepared
> statements. I am using this command to run the script.

I made on my side a couple of tests to see what is happening in the code.
The problem you saw is not linked to the patch I wrote.
In fact when you are using the prepared statement mode, PQprepare seems not
to be able to manage with the parameter I put in my transaction script at
"prepare transaction" and "commit prepared" stages. This parameter is the
random ID used for prepared transaction.
If you try an end script such as:
PREPARE TRANSACTION 'T';
\shell ls -ll ~/pgsql/data
COMMIT PREPARED 'T';
It will work without any problem in both prepared and single query mode.

For 1 customer, there is no issue. However, by increasing the number of
customers, it will increase the risk for a customer to prepare a transaction
who is using a same 2pc ID, resulting in a couple of output errors.
Using the original script I wrote is OK in single query mode, as 2pc ID is
not managed by PQprepare but at pgbench level. This way pgbench will not
create errors, as it builds an individual query with a random ID one by one.
The prepared query mode prepares the same transaction for all the customers
of pgbench, and I think that PQprepare cannot manage 2pc IDs while preparing
a transaction. Parameters in SQL queries is OK, but not at prepare states.
That would explain why in this case, the system was looking for a parameter
that is not delivered initially.

Besides, you can also make tests without 2pc transactions, such as:
\shell ls -ll /home/ioltas/usr/pgsql/data
END;
In this case also it works finely in both prepared and single query mode (at
least on my side :)).

By looking at the documentation of pgbench, prepared query mode is used for
performance purposes only. The pgbench shell feature tends to slow down all
the process as it has been created for analysis purposes only, so the single
query mode is enough to my mind if you want to study the interactions of
your system and postgres.

Regards,

Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2009-09-22 02:07:49 Re: Hot Standby 0.2.1
Previous Message Tom Lane 2009-09-22 01:18:53 Re: updated hstore patch