Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
I see this in pgbench.c: /* return false iff client should be disconnected */ static bool doCustom(CState *st, instr_time *conn_time) I think you need to increase the verbosity of the error messages when you're working on this code, because when I compile I get a slew of these errors pointing to the problem too: pgbench.c:1009: warning: return with no value, in function returning non-void The fix is that when there's an error, you need to do this: return clientDone(st, false); Thanks for the tip. In fact I based my patch on postgres 8.4.1 and not on the head of the git repository. This explains why I did not go through the error messages returned by doCustom. The new version of the patch attached to this email has been made directly from the git repository. It looks definitely cleaner this time. I tried to clean the patch so as to pass the rest of the command line also, so as not to have to do it later. A short update of the setshell feature is also available on PostgreSQL's wiki at the page pgbench shell command. About the potential examples, I can also write a short script and put that on the wiki. As you said previously, the Pareto example is possible, but also why not thinking about other statistical distributions? a Gaussian or a Poisson distribution? There are many possibilities. Regards, -- Michael Paquier NTT OSSC pgbenchsetshell.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
On Tue, 22 Sep 2009, Michael Paquier wrote: The function doCustom is defined with a void. I see this in pgbench.c: /* return false iff client should be disconnected */ static bool doCustom(CState *st, instr_time *conn_time) I think you need to increase the verbosity of the error messages when you're working on this code, because when I compile I get a slew of these errors pointing to the problem too: pgbench.c:1009: warning: return with no value, in function returning non-void The fix is that when there's an error, you need to do this: return clientDone(st, false); To indicate to DoCustom that things have gone badly and it shouldn't try executing anything else in the script. Your patch didn't apply cleanly either, but I think that's not your fault. There's a lot of code that looks quite similar in this area and both git apply and patch seemed confused. Probably needs more context around the diff next time. See attached a patch of this setshell feature. If you use in a script file something like: /setshell param_set setshellparam.sh pgbench reads from the shell script setshellparam.sh the first output value, verifies if it is an integer, then manages it as a pgbench parameter. I did not take into account you 4th and 5th arguments, so it is just a basic implementation. That part looks fine so far, but what actually needs to happen here is that the rest of the line after the name of the script should be passed to the script as part of its command line. This patch is moving in the right direction, it still needs some work. So far my list of concerns is: * Make the patch diff apply cleanly to HEAD (I'm past this) * Update the return logic (already fixed in my copy, just need to update all the return statements with no value) * Make setshell pass through the rest of the command line (I could fix this, but don't really want to) * We need a much simpler example how this patch can be helpful and used (this I will do, I have a couple of ideas) * The documentation needs to be updated with that example and the rest of the details on what you've added. If you plan to submit more patches in the future, you probably want to get familiar with this eventually, and I encourage you to take a shot at it. I have some other pgbench docs fixes to apply anyway soon, so I wouldn't mind taking care of this too once we're close to code that can be committed. We're trying to close up this CommitFest, so I'm going to mark this one returned with feedback for now. I've got it sitting in my personal git repo how and can help out with the details. I'll be glad to work with you to get it into shape for the next CommitFest, where I think it can be a useful feature to add. -- * Greg Smith gsm...@gregsmith.com http://www.gregsmith.com Baltimore, MD -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
On Tue, 22 Sep 2009, Michael Paquier wrote: Besides, you can also make tests without 2pc transactions, such as: \shell ls -ll /home/ioltas/usr/pgsql/data END; I think that demonstrating the pgbench shell feature with this 2PC example is working against your patch being even considered, much less accepted. It's way too complicated, and the idea you're pouplarizing with it that the script should save whatever it does in the external filesystem is messy. You'll need a simpler one that still accomplishes something useful that can go into the docs no matter what, the problems with prepared transactions you're reporting in your messages are just the beginning of issues that come from presenting this as the hello, world of how to use the shell feature. Most of the cases I can think of for situations where I'd like to call the shell in a pgbench script require doing something useful with the result that is returned. For example, when I'm picking a row at random, I often want to skew which row that is, to simulate the common real-world situation where a fraction of customers generate most of the transactions (the Pareto principle). What I'd consider closer to a useful implementation of this feature is that you could call a shell script and use the first line of its output to set one of the pgbench internal variables. The range of return codes is way too limited for those to help here. Here's what that might look like, assuming skewedrand is our script that does that given the range it needs to operate over: \set nbranches :scale \set ntellers 10 * :scale \set naccounts 10 * :scale \setshell aid skewedrand 1 :naccounts SELECT abalance FROM pgbench_accounts WHERE aid = :aid; (I'm not sure if setshell would need to interpret the output as an integer for this to work right, there may be some int vs. string considerations here) If you got something like that working first before moving onto these more complicated examples and I think you'll have an easier time of things. -- * Greg Smith gsm...@gregsmith.com http://www.gregsmith.com Baltimore, MD -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
See attached a patch of this setshell feature.
If you use in a script file something like:
/setshell param_set setshellparam.sh
pgbench reads from the shell script setshellparam.sh the first output value,
verifies if it is an integer, then manages it as a pgbench parameter.
I did not take into account you 4th and 5th arguments, so it is just a basic
implementation.
Depending on the statistical model you want to use for you pgbench tests, it
is possible to add other parameters as on the call prototype you proposed.
Regards,
Michael
On Wed, Sep 23, 2009 at 8:17 AM, Greg Smith gsm...@gregsmith.com wrote:
On Tue, 22 Sep 2009, Michael Paquier wrote:
Besides, you can also make tests without 2pc transactions, such as:
\shell ls -ll /home/ioltas/usr/pgsql/data
END;
I think that demonstrating the pgbench shell feature with this 2PC example
is working against your patch being even considered, much less accepted.
It's way too complicated, and the idea you're pouplarizing with it that the
script should save whatever it does in the external filesystem is messy.
You'll need a simpler one that still accomplishes something useful that can
go into the docs no matter what, the problems with prepared transactions
you're reporting in your messages are just the beginning of issues that come
from presenting this as the hello, world of how to use the shell feature.
Most of the cases I can think of for situations where I'd like to call the
shell in a pgbench script require doing something useful with the result
that is returned. For example, when I'm picking a row at random, I often
want to skew which row that is, to simulate the common real-world situation
where a fraction of customers generate most of the transactions (the Pareto
principle).
What I'd consider closer to a useful implementation of this feature is that
you could call a shell script and use the first line of its output to set
one of the pgbench internal variables. The range of return codes is way too
limited for those to help here. Here's what that might look like, assuming
skewedrand is our script that does that given the range it needs to
operate over:
\set nbranches :scale
\set ntellers 10 * :scale
\set naccounts 10 * :scale
\setshell aid skewedrand 1 :naccounts
SELECT abalance FROM pgbench_accounts WHERE aid = :aid;
(I'm not sure if setshell would need to interpret the output as an integer
for this to work right, there may be some int vs. string considerations
here)
If you got something like that working first before moving onto these more
complicated examples and I think you'll have an easier time of things.
--
* Greg Smith gsm...@gregsmith.com http://www.gregsmith.com Baltimore, MD
--
Michael Paquier
NTT OSSC
--- postgresql-8.4.0.orig/contrib/pgbench/pgbench.c 2009-09-22 10:09:44.0 +0900
+++ postgresql-8.4.0/contrib/pgbench/pgbench.c 2009-09-23 11:03:27.0 +0900
@@ -123,6 +123,7 @@ typedef struct
} Variable;
#define MAX_FILES 128 /* max number of SQL script files allowed */
+#define SHELL_COMMAND_SIZE 256 /* maximum size allowed for shell command */
/*
* structures used in custom query mode
@@ -987,7 +988,99 @@ top:
st-listen = 1;
}
+ else if (pg_strcasecmp(argv[0], setshell) == 0)
+ {
+ int retval;
+ char res[64],
+resfin[64];
+ FILE *respipe = NULL;
+ /* Data treatment */
+ /* prototype: /setshell aid skewerand */
+ respipe = popen(argv[2],r);
+ if (respipe == NULL)
+ {
+fprintf(stderr, %s: error launching shell script, argv[0]);
+st-ecnt++;
+return;
+ }
+ if (fgets(res, 64, respipe) == NULL)
+ {
+fprintf(stderr, %s: error getting parameter, argv[0]);
+st-ecnt++;
+return;
+ }
+
+ retval = pclose(respipe);
+ if (retval == -1)
+ {
+fprintf(stderr, %s: error closing shell script, argv[0]);
+st-ecnt++;
+return;
+ }
+ /* Transform the parameter into an integer */
+ retval = atoi(res);
+ if (retval == 0)
+ {
+fprintf(stderr, %s: error input integer, argv[0]);
+st-ecnt++;
+return;
+ }
+ /* ready to put the variable */
+ snprintf(resfin, sizeof(resfin), %d, retval);
+
+ if (putVariable(st, argv[1], resfin) == false)
+ {
+fprintf(stderr, %s: out of memory\n, argv[0]);
+st-ecnt++;
+return;
+ }
+#ifdef DEBUG
+ printf(shell parameter name: %s, value: %s, argv[1], resfin);
+#endif
+ st-listen = 1;
+ }
+ else if (pg_strcasecmp(argv[0], shell) == 0)
+ {
+ int j,
+retval,
+retvalglob;
+ char commandLoc[SHELL_COMMAND_SIZE];
+
+ retval = snprintf(commandLoc,SHELL_COMMAND_SIZE-1,%s,argv[1]);
+ if (retval 0
+|| retval SHELL_COMMAND_SIZE-1)
+ {
+fprintf(stderr, Error launching shell command: too many characters\n);
+st-ecnt++;
+return;
+ }
+ retvalglob = retval;
+
+ for (j = 2; j argc; j++)
+ {
+char *commandLoc2 = strdup(commandLoc);
+retval = snprintf(commandLoc,SHELL_COMMAND_SIZE-1,%s %s,
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
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
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
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.
OK I got your point, I missed the part managing with CState, which is tested
after doCustom.
Another version of the patch is attached with this email.
I must be doing something wrong when I am running this script. It is
still throwing errors. Would you mind showing me the pgbench command you
used to run it?
Of course, here it is the list of commands I use:
pgbench -i dbname (in case your database is called dbname)
pgbench -c 10 -t 10 -f transaction_file_name.data dbname (customer and
transaction numbers can also bet set as you want).
Regards,
--
Michael Paquier
NTT OSSC
--- postgresql-8.4.0.orig/contrib/pgbench/pgbench.c 2009-09-17 09:07:24.0 +0900
+++ postgresql-8.4.0/contrib/pgbench/pgbench.c 2009-09-18 13:24:33.0 +0900
@@ -120,6 +120,7 @@ typedef struct
} Variable;
#define MAX_FILES 128 /* max number of SQL script files allowed */
+#define SHELL_COMMAND_SIZE 256 /* maximum size allowed for shell command */
/*
* structures used in custom query mode
@@ -984,7 +985,47 @@ top:
st-listen = 1;
}
+ else if (pg_strcasecmp(argv[0], shell) == 0)
+ {
+ int j,
+retval,
+retvalglob;
+ char commandLoc[SHELL_COMMAND_SIZE];
+
+ retval = snprintf(commandLoc,SHELL_COMMAND_SIZE-1,%s,argv[1]);
+ if (retval 0
+|| retval SHELL_COMMAND_SIZE-1)
+ {
+fprintf(stderr, Error launching shell command: too many characters\n);
+st-ecnt++;
+return;
+ }
+ retvalglob = retval;
+ for (j = 2; j argc; j++)
+ {
+char *commandLoc2 = strdup(commandLoc);
+retval = snprintf(commandLoc,SHELL_COMMAND_SIZE-1,%s %s, commandLoc2, argv[j]);
+retvalglob += retval;
+if (retval 0
+ || retvalglob SHELL_COMMAND_SIZE-1)
+{
+ fprintf(stderr, Error launching shell command: too many characters\n);
+ free(commandLoc2);
+ st-ecnt++;
+ return;
+}
+free(commandLoc2);
+ }
+ retval = system(commandLoc);
+ if (retval 0)
+ {
+fprintf(stderr, Error launching shell command: command not launched);
+st-ecnt++;
+return;
+ }
+ st-listen = 1;
+ }
goto top;
}
}
@@ -1280,6 +1321,14 @@ process_commands(char *buf)
fprintf(stderr, %s: extra argument \%s\ ignored\n,
my_commands-argv[0], my_commands-argv[j]);
}
+ else if (pg_strcasecmp(my_commands-argv[0], shell) == 0)
+ {
+ if (my_commands-argc 1)
+ {
+fprintf(stderr, %s: missing command\n, my_commands-argv[0]);
+return NULL;
+ }
+ }
else
{
fprintf(stderr, Invalid command %s\n, my_commands-argv[0]);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
On Fri, Sep 18, 2009 at 03:10:14PM +0900, Michael Paquier wrote: 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. OK I got your point, I missed the part managing with CState, which is tested after doCustom. Another version of the patch is attached with this email. I must be doing something wrong when I am running this script. It is still throwing errors. Would you mind showing me the pgbench command you used to run it? Of course, here it is the list of commands I use: pgbench -i dbname (in case your database is called dbname) pgbench -c 10 -t 10 -f transaction_file_name.data dbname (customer and transaction numbers can also bet set as you want). Regards, -- Michael Paquier NTT OSSC OK, thank you for sending me the patch and the relivent commands. You're still not returning false where you should be, but that wasn't the issue I am seeing. I believe there is an issue in either the custom script you posted on the wiki or pgbench itself. Here is the script I am running, you might recongnize it :) \set nbranches :scale \set ntellers 10 * :scale \set naccounts 10 * :scale \setrandom aid 1 :naccounts \setrandom bid 1 :nbranches \setrandom tid 1 :ntellers \setrandom delta -5000 5000 \setrandom txidrand 0 1 START TRANSACTION; UPDATE pgbench_accounts SET abalance = abalance + :delta WHERE aid = :aid; SELECT abalance FROM pgbench_accounts WHERE aid = :aid; SELECT abalance FROM pgbench_accounts WHERE aid = :aid; UPDATE pgbench_tellers SET tbalance = tbalance + :delta WHERE tid = :tid; UPDATE pgbench_branches SET bbalance = bbalance + :delta WHERE bid = :bid; INSERT INTO pgbench_history (tid, bid, aid, delta, mtime) VALUES (:tid, :bid, :aid, :delta, CURRENT_TIMESTAMP); PREPARE TRANSACTION ':txidrand'; \shell ls -ll /tmp/log_data`date '+%Y%m%d'`_`date '+%k%M'` COMMIT PREPARED ':txidrand'; 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. ./contrib/pgbench/pgbench -c 10 -j 2 -M prepared -f custom.script test From which, I get the following output: starting vacuum...end. Client 0 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 1 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 2 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 5 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 8 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 6 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 7 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 3 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 4 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 9 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 transaction type: Custom query scaling factor: 1 query mode: prepared number of clients: 10 number of threads: 2 number of transactions per client: 10 number of transactions actually processed: 0/100 tps = 0.00 (including connections establishing) tps = 0.00 (excluding connections establishing) Since it occurs on both versions, I'm having some trouble identifying whether this is a bug in the script or a bug in pgbench. Any help is appreciated. -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
On Thu, Sep 17, 2009 at 09:56:44AM +0900, Michael Paquier wrote: Hi all, Sorry for my late reply. There is no other update for this patch since the 13th of August, at least until today. The new patch is attached By the way I worked on the comments that Dan and Gabriel pointed out. I added a check on system such as to prevent an error from this side. By the way, I noticed that the way return values are managed in doCustom and in process_commands is different. Such as to make an homogeneous code, return values are managed the same way in both functions in the patch, explaining why I did not return a specific value when file commands are treated in doCustom. 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. Here is also as wanted a simple transaction that permits to use this function: \set nbranches :scale \set naccounts 10 * :scale \setrandom aid 1 :naccounts \setrandom bid 1 :nbranches \setrandom delta -5000 5000 \setrandom txidrand 0 1 START TRANSACTION; UPDATE pgbench_accounts SET abalance = abalance + :delta WHERE aid = :aid; SELECT abalance FROM pgbench_accounts WHERE aid = :aid; UPDATE pgbench_branches SET bbalance = bbalance + :delta WHERE bid = :bid; PREPARE TRANSACTION ':txidrand'; \shell ls -ll $PGDATA/pg_twophase COMMIT PREPARED ':txidrand'; I must be doing something wrong when I am running this script. It is still throwing errors. Would you mind showing me the pgbench command you used to run it? -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
Hi all,
Sorry for my late reply.
There is no other update for this patch since the 13th of August, at least
until today. The new patch is attached
By the way I worked on the comments that Dan and Gabriel pointed out.
I added a check on system such as to prevent an error from this side.
By the way, I noticed that the way return values are managed in doCustom and
in process_commands is different. Such as to make an homogeneous code,
return values are managed the same way in both functions in the patch,
explaining why I did not return a specific value when file commands are
treated in doCustom.
Here is also as wanted a simple transaction that permits to use this
function:
\set nbranches :scale
\set naccounts 10 * :scale
\setrandom aid 1 :naccounts
\setrandom bid 1 :nbranches
\setrandom delta -5000 5000
\setrandom txidrand 0 1
START TRANSACTION;
UPDATE pgbench_accounts SET abalance = abalance + :delta WHERE aid = :aid;
SELECT abalance FROM pgbench_accounts WHERE aid = :aid;
UPDATE pgbench_branches SET bbalance = bbalance + :delta WHERE bid = :bid;
PREPARE TRANSACTION ':txidrand';
\shell ls -ll $PGDATA/pg_twophase
COMMIT PREPARED ':txidrand';
The shell command I use here permits to scan the 2PC state files written in
pg_twophase.
You will notice that in this case files have a size of 540B. Also please do
not forget to set PGDATA.
The new functionnality proposed here is just for analysis purposes. Even if
it decreased the performance of pgbench, it is interesting to have a simple
benchmark that permits to analyse precisely the system while transaction are
being run.
Regards,
--- postgresql-8.4.0.orig/contrib/pgbench/pgbench.c 2009-09-17 09:07:24.0 +0900
+++ postgresql-8.4.0/contrib/pgbench/pgbench.c 2009-09-17 09:03:35.0 +0900
@@ -120,6 +120,7 @@ typedef struct
} Variable;
#define MAX_FILES 128 /* max number of SQL script files allowed */
+#define SHELL_COMMAND_SIZE 256 /* maximum size allowed for shell command */
/*
* structures used in custom query mode
@@ -984,7 +985,44 @@ top:
st-listen = 1;
}
+ else if (pg_strcasecmp(argv[0], shell) == 0)
+ {
+ int j,
+retval,
+retvalglob;
+ char commandLoc[SHELL_COMMAND_SIZE];
+
+ retval = snprintf(commandLoc,SHELL_COMMAND_SIZE-1,%s,argv[1]);
+ if (retval 0
+|| retval SHELL_COMMAND_SIZE-1)
+ {
+fprintf(stderr, Error launching shell command: too many characters\n);
+return;
+ }
+ retvalglob = retval;
+ for (j = 2; j argc; j++)
+ {
+char *commandLoc2 = strdup(commandLoc);
+retval = snprintf(commandLoc,SHELL_COMMAND_SIZE-1,%s %s, commandLoc2, argv[j]);
+retvalglob += retval;
+if (retval 0
+ || retvalglob SHELL_COMMAND_SIZE-1)
+{
+ fprintf(stderr, Error launching shell command: too many characters\n);
+ free(commandLoc2);
+ return;
+}
+free(commandLoc2);
+ }
+ retval = system(commandLoc);
+ if (retval 0)
+ {
+fprintf(stderr, Error launching shell command: command not launched);
+return;
+ }
+ st-listen = 1;
+ }
goto top;
}
}
@@ -1280,6 +1318,14 @@ process_commands(char *buf)
fprintf(stderr, %s: extra argument \%s\ ignored\n,
my_commands-argv[0], my_commands-argv[j]);
}
+ else if (pg_strcasecmp(my_commands-argv[0], shell) == 0)
+ {
+ if (my_commands-argc 1)
+ {
+fprintf(stderr, %s: missing command\n, my_commands-argv[0]);
+return NULL;
+ }
+ }
else
{
fprintf(stderr, Invalid command %s\n, my_commands-argv[0]);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
Michael, I just wanted to follow-up on your pgbench patch. The latest version that I see is from August 13th. Is that the correct patch to be reviewing? Do you have any other updates on it? Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
On Tue, Sep 15, 2009 at 09:53:11PM -0400, Stephen Frost wrote: Michael, I just wanted to follow-up on your pgbench patch. The latest version that I see is from August 13th. Is that the correct patch to be reviewing? Do you have any other updates on it? Thanks! Stephen Hi! Some comments about this patch: - You have DOS-style carriage returns, which interfere with the patch application on Unix systems. - We'd like to see return specifically return a value (lines 1008 and 1022 in the patched version) - We'd like to see something done with the return value from system (line 1026 in patched version) - pg_bench functions as expected, however, your example script given on the wiki page for this patch fails (http://wiki.postgresql.org/wiki/Pgbench:_shell_command). Can we have an example that works so we can check it out? It's not really clear to us how this will be useful to others. Thanks! Gabrielle Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
Thanks a lot for all of your pieces of advice. I modified the name of the page as well as I deleted the parts linked to the -P option. It just consisted in deleting the right parts. Here is the lighted version. -- Michael Paquier NTT OSSC postgresql-8.4.0-pgbenchshell2.0.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
Sorry I forgot to attach the the patch. Regards, Michael On Fri, Aug 7, 2009 at 12:23 PM, Michael Paquier michael.paqu...@gmail.comwrote: Hi all, Here is a short patch implementing a new feature in pgbench so as to allow shell commands to be launched in a transaction file of pgbench. the user has just to add at the beginning of the command line in his transaction file \shell + the command wanted. As an example of transaction: Begin; [Transaction instructions] Prepare transaction ‘TXID’; \shell ls ~/pg_twophase; Commit prepared ‘TXID’; This patch was particularly useful in order to determine the size of state files flushed to disk for prepared but not committed transactions. As an addition, I added a new default transaction in the code that permits to launch a 2PC transaction with a random prepare identifier of the format Txxx. I also created a page in postgresql's wiki about this feature. Please refer to this link: http://wiki.postgresql.org/wiki/Pgbench:_shell_command Regards, -- Michael Paquier NTT OSSC -- Michael Paquier NTT OSSC postgresql-8.4.0-pgbenchshell.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
On Thu, Aug 6, 2009 at 11:26 PM, Michael Paquiermichael.paqu...@gmail.com wrote: Sorry I forgot to attach the the patch. Please add your patches at https://commitfest.postgresql.org/action/commitfest_view/open ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
Michael Paquier michael.paqu...@gmail.com wrote: Here is a short patch implementing a new feature in pgbench so as to allow shell commands to be launched in a transaction file of pgbench. \shell ls ~/pg_twophase; +1 for \shell command itself, but does the performance fit for your purpose? Spawning a new process is not so cheap, no? -1 for -P option because it is too narrow purpose and 'ls' and '/tmp/' is not portable. We don't need to include your workload because you can use -f FILENAME to run your benchmark script. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
Michael Paquier escribió: I also created a page in postgresql's wiki about this feature. Please refer to this link: http://wiki.postgresql.org/wiki/Pgbench:_shell_command Please don't use colons in wiki page names. Pgbench_shell_command should be fine. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pgbench: new feature allowing to launch shell commands
Yes it dramatically decreases the transaction flow. This function has not been implemented at all for performance but for analysis purposes. I used it mainly to have a look at state files size in pg_twophase for transactions that are prepared but not committed. Regards On Fri, Aug 7, 2009 at 12:55 PM, Itagaki Takahiro itagaki.takah...@oss.ntt.co.jp wrote: Michael Paquier michael.paqu...@gmail.com wrote: Here is a short patch implementing a new feature in pgbench so as to allow shell commands to be launched in a transaction file of pgbench. \shell ls ~/pg_twophase; +1 for \shell command itself, but does the performance fit for your purpose? Spawning a new process is not so cheap, no? -1 for -P option because it is too narrow purpose and 'ls' and '/tmp/' is not portable. We don't need to include your workload because you can use -f FILENAME to run your benchmark script. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Michael Paquier NTT OSSC
