From 6471045230f5d891ad724c54d406e2214f3c96d9 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 19 Aug 2016 17:32:59 -0400 Subject: Allow empty queries in pgbench. This might have been too much of a foot-gun before 9.6, but with the new commands-end-at-semicolons parsing rule, the only way to get an empty query into a script is to explicitly write an extra ";". So we may as well allow the case. Fabien Coelho Patch: --- src/bin/pgbench/pgbench.c | 1 + 1 file changed, 1 insertion(+) (limited to 'src/bin/pgbench') diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 87fb006d87..8027955121 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -1898,6 +1898,7 @@ top: { case PGRES_COMMAND_OK: case PGRES_TUPLES_OK: + case PGRES_EMPTY_QUERY: break; /* OK */ default: fprintf(stderr, "client %d aborted in state %d: %s", -- cgit v1.2.3 From 9daec77e165de461fca9d5bc3ece86a91aba5804 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 30 Aug 2016 17:02:02 -0400 Subject: Simplify correct use of simple_prompt(). The previous API for this function had it returning a malloc'd string. That meant that callers had to check for NULL return, which few of them were doing, and it also meant that callers had to remember to free() the string later, which required extra logic in most cases. Instead, make simple_prompt() write into a buffer supplied by the caller. Anywhere that the maximum required input length is reasonably small, which is almost all of the callers, we can just use a local or static array as the buffer instead of dealing with malloc/free. A fair number of callers used "pointer == NULL" as a proxy for "haven't requested the password yet". Maintaining the same behavior requires adding a separate boolean flag for that, which adds back some of the complexity we save by removing free()s. Nonetheless, this nets out at a small reduction in overall code size, and considerably less code than we would have had if we'd added the missing NULL-return checks everywhere they were needed. In passing, clean up the API comment for simple_prompt() and get rid of a very-unnecessary malloc/free in its Windows code path. This is nominally a bug fix, but it does not seem worth back-patching, because the actual risk of an OOM failure in any of these places seems pretty tiny, and all of them are client-side not server-side anyway. This patch is by me, but it owes a great deal to Michael Paquier who identified the problem and drafted a patch for fixing it the other way. Discussion: --- contrib/oid2name/oid2name.c | 13 ++++++----- contrib/vacuumlo/vacuumlo.c | 17 +++++++++------ src/bin/initdb/initdb.c | 23 ++++++++------------ src/bin/pg_basebackup/nls.mk | 1 + src/bin/pg_basebackup/streamutil.c | 14 ++++++------ src/bin/pg_dump/pg_backup_db.c | 35 ++++++++++++------------------ src/bin/pg_dump/pg_dumpall.c | 17 +++++++++------ src/bin/pgbench/pgbench.c | 10 +++++---- src/bin/psql/command.c | 25 +++++++++++----------- src/bin/psql/startup.c | 16 ++++++++------ src/bin/scripts/common.c | 34 +++++++++++------------------ src/bin/scripts/createuser.c | 21 +++++++++++------- src/bin/scripts/dropuser.c | 7 +++++- src/include/port.h | 3 ++- src/port/sprompt.c | 44 +++++++++++++++----------------------- 15 files changed, 138 insertions(+), 142 deletions(-) (limited to 'src/bin/pgbench') diff --git a/contrib/oid2name/oid2name.c b/contrib/oid2name/oid2name.c index e5eeec21c1..5a2aa1dd0e 100644 --- a/contrib/oid2name/oid2name.c +++ b/contrib/oid2name/oid2name.c @@ -261,7 +261,8 @@ PGconn * sql_conn(struct options * my_opts) { PGconn *conn; - char *password = NULL; + bool have_password = false; + char password[100]; bool new_pass; /* @@ -282,7 +283,7 @@ sql_conn(struct options * my_opts) keywords[2] = "user"; values[2] = my_opts->username; keywords[3] = "password"; - values[3] = password; + values[3] = have_password ? password : NULL; keywords[4] = "dbname"; values[4] = my_opts->dbname; keywords[5] = "fallback_application_name"; @@ -302,17 +303,15 @@ sql_conn(struct options * my_opts) if (PQstatus(conn) == CONNECTION_BAD && PQconnectionNeedsPassword(conn) && - password == NULL) + !have_password) { PQfinish(conn); - password = simple_prompt("Password: ", 100, false); + simple_prompt("Password: ", password, sizeof(password), false); + have_password = true; new_pass = true; } } while (new_pass); - if (password) - free(password); - /* check to see that the backend connection was successfully made */ if (PQstatus(conn) == CONNECTION_BAD) { diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c index 769c805a84..0a9328dc0e 100644 --- a/contrib/vacuumlo/vacuumlo.c +++ b/contrib/vacuumlo/vacuumlo.c @@ -65,13 +65,17 @@ vacuumlo(const char *database, const struct _param * param) long matched; long deleted; int i; - static char *password = NULL; bool new_pass; bool success = true; + static bool have_password = false; + static char password[100]; /* Note: password can be carried over from a previous call */ - if (param->pg_prompt == TRI_YES && password == NULL) - password = simple_prompt("Password: ", 100, false); + if (param->pg_prompt == TRI_YES && !have_password) + { + simple_prompt("Password: ", password, sizeof(password), false); + have_password = true; + } /* * Start the connection. Loop until we have a password if requested by @@ -91,7 +95,7 @@ vacuumlo(const char *database, const struct _param * param) keywords[2] = "user"; values[2] = param->pg_user; keywords[3] = "password"; - values[3] = password; + values[3] = have_password ? password : NULL; keywords[4] = "dbname"; values[4] = database; keywords[5] = "fallback_application_name"; @@ -110,11 +114,12 @@ vacuumlo(const char *database, const struct _param * param) if (PQstatus(conn) == CONNECTION_BAD && PQconnectionNeedsPassword(conn) && - password == NULL && + !have_password && param->pg_prompt != TRI_NO) { PQfinish(conn); - password = simple_prompt("Password: ", 100, false); + simple_prompt("Password: ", password, sizeof(password), false); + have_password = true; new_pass = true; } } while (new_pass); diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 54d338d013..94074928cb 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -1557,8 +1557,8 @@ setup_auth(FILE *cmdfd) static void get_su_pwd(void) { - char *pwd1, - *pwd2; + char pwd1[100]; + char pwd2[100]; if (pwprompt) { @@ -1567,14 +1567,13 @@ get_su_pwd(void) */ printf("\n"); fflush(stdout); - pwd1 = simple_prompt("Enter new superuser password: ", 100, false); - pwd2 = simple_prompt("Enter it again: ", 100, false); + simple_prompt("Enter new superuser password: ", pwd1, sizeof(pwd1), false); + simple_prompt("Enter it again: ", pwd2, sizeof(pwd2), false); if (strcmp(pwd1, pwd2) != 0) { fprintf(stderr, _("Passwords didn't match.\n")); exit_nicely(); } - free(pwd2); } else { @@ -1587,7 +1586,6 @@ get_su_pwd(void) * for now. */ FILE *pwf = fopen(pwfilename, "r"); - char pwdbuf[MAXPGPATH]; int i; if (!pwf) @@ -1596,7 +1594,7 @@ get_su_pwd(void) progname, pwfilename, strerror(errno)); exit_nicely(); } - if (!fgets(pwdbuf, sizeof(pwdbuf), pwf)) + if (!fgets(pwd1, sizeof(pwd1), pwf)) { if (ferror(pwf)) fprintf(stderr, _("%s: could not read password from file \"%s\": %s\n"), @@ -1608,15 +1606,12 @@ get_su_pwd(void) } fclose(pwf); - i = strlen(pwdbuf); - while (i > 0 && (pwdbuf[i - 1] == '\r' || pwdbuf[i - 1] == '\n')) - pwdbuf[--i] = '\0'; - - pwd1 = pg_strdup(pwdbuf); - + i = strlen(pwd1); + while (i > 0 && (pwd1[i - 1] == '\r' || pwd1[i - 1] == '\n')) + pwd1[--i] = '\0'; } - superuser_password = pwd1; + superuser_password = pg_strdup(pwd1); } /* diff --git a/src/bin/pg_basebackup/nls.mk b/src/bin/pg_basebackup/nls.mk index ec466dcaa2..a34ca3d268 100644 --- a/src/bin/pg_basebackup/nls.mk +++ b/src/bin/pg_basebackup/nls.mk @@ -2,3 +2,4 @@ CATALOG_NAME = pg_basebackup AVAIL_LANGUAGES = de es fr it ko pl pt_BR ru zh_CN GETTEXT_FILES = pg_basebackup.c pg_receivexlog.c pg_recvlogical.c receivelog.c streamutil.c ../../common/fe_memutils.c +GETTEXT_TRIGGERS = simple_prompt diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c index 72d8657004..595eaff46a 100644 --- a/src/bin/pg_basebackup/streamutil.c +++ b/src/bin/pg_basebackup/streamutil.c @@ -41,7 +41,8 @@ char *dbport = NULL; char *replication_slot = NULL; char *dbname = NULL; int dbgetpassword = 0; /* 0=auto, -1=never, 1=always */ -static char *dbpassword = NULL; +static bool have_password = false; +static char password[100]; PGconn *conn = NULL; /* @@ -141,24 +142,23 @@ GetConnection(void) } /* If -W was given, force prompt for password, but only the first time */ - need_password = (dbgetpassword == 1 && dbpassword == NULL); + need_password = (dbgetpassword == 1 && !have_password); do { /* Get a new password if appropriate */ if (need_password) { - if (dbpassword) - free(dbpassword); - dbpassword = simple_prompt(_("Password: "), 100, false); + simple_prompt("Password: ", password, sizeof(password), false); + have_password = true; need_password = false; } /* Use (or reuse, on a subsequent connection) password if we have it */ - if (dbpassword) + if (have_password) { keywords[i] = "password"; - values[i] = dbpassword; + values[i] = password; } else { diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c index d2a3de3c5d..3b9cd89b4a 100644 --- a/src/bin/pg_dump/pg_backup_db.c +++ b/src/bin/pg_dump/pg_backup_db.c @@ -134,6 +134,7 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser) const char *newdb; const char *newuser; char *password; + char passbuf[100]; bool new_pass; if (!reqdb) @@ -149,13 +150,12 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser) ahlog(AH, 1, "connecting to database \"%s\" as user \"%s\"\n", newdb, newuser); - password = AH->savedPassword ? pg_strdup(AH->savedPassword) : NULL; + password = AH->savedPassword; if (AH->promptPassword == TRI_YES && password == NULL) { - password = simple_prompt("Password: ", 100, false); - if (password == NULL) - exit_horribly(modulename, "out of memory\n"); + simple_prompt("Password: ", passbuf, sizeof(passbuf), false); + password = passbuf; } initPQExpBuffer(&connstr); @@ -201,16 +201,14 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser) fprintf(stderr, "Connecting to %s as %s\n", newdb, newuser); - if (password) - free(password); - if (AH->promptPassword != TRI_NO) - password = simple_prompt("Password: ", 100, false); + { + simple_prompt("Password: ", passbuf, sizeof(passbuf), false); + password = passbuf; + } else exit_horribly(modulename, "connection needs password\n"); - if (password == NULL) - exit_horribly(modulename, "out of memory\n"); new_pass = true; } } while (new_pass); @@ -225,8 +223,6 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser) free(AH->savedPassword); AH->savedPassword = pg_strdup(PQpass(newConn)); } - if (password) - free(password); termPQExpBuffer(&connstr); @@ -258,18 +254,18 @@ ConnectDatabase(Archive *AHX, { ArchiveHandle *AH = (ArchiveHandle *) AHX; char *password; + char passbuf[100]; bool new_pass; if (AH->connection) exit_horribly(modulename, "already connected to a database\n"); - password = AH->savedPassword ? pg_strdup(AH->savedPassword) : NULL; + password = AH->savedPassword; if (prompt_password == TRI_YES && password == NULL) { - password = simple_prompt("Password: ", 100, false); - if (password == NULL) - exit_horribly(modulename, "out of memory\n"); + simple_prompt("Password: ", passbuf, sizeof(passbuf), false); + password = passbuf; } AH->promptPassword = prompt_password; @@ -309,9 +305,8 @@ ConnectDatabase(Archive *AHX, prompt_password != TRI_NO) { PQfinish(AH->connection); - password = simple_prompt("Password: ", 100, false); - if (password == NULL) - exit_horribly(modulename, "out of memory\n"); + simple_prompt("Password: ", passbuf, sizeof(passbuf), false); + password = passbuf; new_pass = true; } } while (new_pass); @@ -332,8 +327,6 @@ ConnectDatabase(Archive *AHX, free(AH->savedPassword); AH->savedPassword = pg_strdup(PQpass(AH->connection)); } - if (password) - free(password); /* check for version mismatch */ _check_database_version(AH); diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index 54a9f48200..b5efb46019 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -1884,13 +1884,17 @@ connectDatabase(const char *dbname, const char *connection_string, bool new_pass; const char *remoteversion_str; int my_version; - static char *password = NULL; const char **keywords = NULL; const char **values = NULL; PQconninfoOption *conn_opts = NULL; + static bool have_password = false; + static char password[100]; - if (prompt_password == TRI_YES && !password) - password = simple_prompt("Password: ", 100, false); + if (prompt_password == TRI_YES && !have_password) + { + simple_prompt("Password: ", password, sizeof(password), false); + have_password = true; + } /* * Start the connection. Loop until we have a password if requested by @@ -1970,7 +1974,7 @@ connectDatabase(const char *dbname, const char *connection_string, values[i] = pguser; i++; } - if (password) + if (have_password) { keywords[i] = "password"; values[i] = password; @@ -1998,11 +2002,12 @@ connectDatabase(const char *dbname, const char *connection_string, if (PQstatus(conn) == CONNECTION_BAD && PQconnectionNeedsPassword(conn) && - password == NULL && + !have_password && prompt_password != TRI_NO) { PQfinish(conn); - password = simple_prompt("Password: ", 100, false); + simple_prompt("Password: ", password, sizeof(password), false); + have_password = true; new_pass = true; } } while (new_pass); diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 8027955121..56c37d537e 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -773,8 +773,9 @@ static PGconn * doConnect(void) { PGconn *conn; - static char *password = NULL; bool new_pass; + static bool have_password = false; + static char password[100]; /* * Start the connection. Loop until we have a password if requested by @@ -794,7 +795,7 @@ doConnect(void) keywords[2] = "user"; values[2] = login; keywords[3] = "password"; - values[3] = password; + values[3] = have_password ? password : NULL; keywords[4] = "dbname"; values[4] = dbName; keywords[5] = "fallback_application_name"; @@ -815,10 +816,11 @@ doConnect(void) if (PQstatus(conn) == CONNECTION_BAD && PQconnectionNeedsPassword(conn) && - password == NULL) + !have_password) { PQfinish(conn); - password = simple_prompt("Password: ", 100, false); + simple_prompt("Password: ", password, sizeof(password), false); + have_password = true; new_pass = true; } } while (new_pass); diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 4aaf657cce..38038d415f 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -1089,11 +1089,11 @@ exec_command(const char *cmd, /* \password -- set user password */ else if (strcmp(cmd, "password") == 0) { - char *pw1; - char *pw2; + char pw1[100]; + char pw2[100]; - pw1 = simple_prompt("Enter new password: ", 100, false); - pw2 = simple_prompt("Enter it again: ", 100, false); + simple_prompt("Enter new password: ", pw1, sizeof(pw1), false); + simple_prompt("Enter it again: ", pw2, sizeof(pw2), false); if (strcmp(pw1, pw2) != 0) { @@ -1139,9 +1139,6 @@ exec_command(const char *cmd, if (opt0) free(opt0); } - - free(pw1); - free(pw2); } /* \prompt -- prompt and set variable */ @@ -1173,7 +1170,10 @@ exec_command(const char *cmd, opt = arg1; if (!pset.inputfile) - result = simple_prompt(prompt_text, 4096, true); + { + result = (char *) pg_malloc(4096); + simple_prompt(prompt_text, result, 4096, true); + } else { if (prompt_text) @@ -1747,20 +1747,19 @@ exec_command(const char *cmd, static char * prompt_for_password(const char *username) { - char *result; + char buf[100]; if (username == NULL) - result = simple_prompt("Password: ", 100, false); + simple_prompt("Password: ", buf, sizeof(buf), false); else { char *prompt_text; prompt_text = psprintf(_("Password for user %s: "), username); - result = simple_prompt(prompt_text, 100, false); + simple_prompt(prompt_text, buf, sizeof(buf), false); free(prompt_text); } - - return result; + return pg_strdup(buf); } static bool diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index 111593cd9d..7ce05fbe4a 100644 --- a/src/bin/psql/startup.c +++ b/src/bin/psql/startup.c @@ -103,7 +103,8 @@ main(int argc, char *argv[]) { struct adhoc_opts options; int successResult; - char *password = NULL; + bool have_password = false; + char password[100]; char *password_prompt = NULL; bool new_pass; @@ -210,7 +211,10 @@ main(int argc, char *argv[]) options.username); if (pset.getPassword == TRI_YES) - password = simple_prompt(password_prompt, 100, false); + { + simple_prompt(password_prompt, password, sizeof(password), false); + have_password = true; + } /* loop until we have a password if requested by backend */ do @@ -226,7 +230,7 @@ main(int argc, char *argv[]) keywords[2] = "user"; values[2] = options.username; keywords[3] = "password"; - values[3] = password; + values[3] = have_password ? password : NULL; keywords[4] = "dbname"; /* see do_connect() */ values[4] = (options.list_dbs && options.dbname == NULL) ? "postgres" : options.dbname; @@ -244,16 +248,16 @@ main(int argc, char *argv[]) if (PQstatus(pset.db) == CONNECTION_BAD && PQconnectionNeedsPassword(pset.db) && - password == NULL && + !have_password && pset.getPassword != TRI_NO) { PQfinish(pset.db); - password = simple_prompt(password_prompt, 100, false); + simple_prompt(password_prompt, password, sizeof(password), false); + have_password = true; new_pass = true; } } while (new_pass); - free(password); free(password_prompt); if (PQstatus(pset.db) == CONNECTION_BAD) diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c index 7c1ebe059f..a71cc64a8c 100644 --- a/src/bin/scripts/common.c +++ b/src/bin/scripts/common.c @@ -68,19 +68,19 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport, const char *progname, bool fail_ok, bool allow_password_reuse) { PGconn *conn; - static char *password = NULL; bool new_pass; + static bool have_password = false; + static char password[100]; if (!allow_password_reuse) + have_password = false; + + if (!have_password && prompt_password == TRI_YES) { - if (password) - free(password); - password = NULL; + simple_prompt("Password: ", password, sizeof(password), false); + have_password = true; } - if (password == NULL && prompt_password == TRI_YES) - password = simple_prompt("Password: ", 100, false); - /* * Start the connection. Loop until we have a password if requested by * backend. @@ -97,7 +97,7 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport, keywords[2] = "user"; values[2] = pguser; keywords[3] = "password"; - values[3] = password; + values[3] = have_password ? password : NULL; keywords[4] = "dbname"; values[4] = dbname; keywords[5] = "fallback_application_name"; @@ -123,9 +123,8 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport, prompt_password != TRI_NO) { PQfinish(conn); - if (password) - free(password); - password = simple_prompt("Password: ", 100, false); + simple_prompt("Password: ", password, sizeof(password), false); + have_password = true; new_pass = true; } } while (new_pass); @@ -275,22 +274,15 @@ yesno_prompt(const char *question) for (;;) { - char *resp; + char resp[10]; - resp = simple_prompt(prompt, 1, true); + simple_prompt(prompt, resp, sizeof(resp), true); if (strcmp(resp, _(PG_YESLETTER)) == 0) - { - free(resp); return true; - } - else if (strcmp(resp, _(PG_NOLETTER)) == 0) - { - free(resp); + if (strcmp(resp, _(PG_NOLETTER)) == 0) return false; - } - free(resp); printf(_("Please answer \"%s\" or \"%s\".\n"), _(PG_YESLETTER), _(PG_NOLETTER)); } diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c index e88879dc19..f13d9a047a 100644 --- a/src/bin/scripts/createuser.c +++ b/src/bin/scripts/createuser.c @@ -66,6 +66,8 @@ main(int argc, char *argv[]) char *conn_limit = NULL; bool pwprompt = false; char *newpassword = NULL; + char newuser_buf[128]; + char newpassword_buf[100]; /* Tri-valued variables. */ enum trivalue createdb = TRI_DEFAULT, @@ -188,7 +190,11 @@ main(int argc, char *argv[]) if (newuser == NULL) { if (interactive) - newuser = simple_prompt("Enter name of role to add: ", 128, true); + { + simple_prompt("Enter name of role to add: ", + newuser_buf, sizeof(newuser_buf), true); + newuser = newuser_buf; + } else { if (getenv("PGUSER")) @@ -200,18 +206,17 @@ main(int argc, char *argv[]) if (pwprompt) { - char *pw1, - *pw2; + char pw2[100]; - pw1 = simple_prompt("Enter password for new role: ", 100, false); - pw2 = simple_prompt("Enter it again: ", 100, false); - if (strcmp(pw1, pw2) != 0) + simple_prompt("Enter password for new role: ", + newpassword_buf, sizeof(newpassword_buf), false); + simple_prompt("Enter it again: ", pw2, sizeof(pw2), false); + if (strcmp(newpassword_buf, pw2) != 0) { fprintf(stderr, _("Passwords didn't match.\n")); exit(1); } - newpassword = pw1; - free(pw2); + newpassword = newpassword_buf; } if (superuser == 0) diff --git a/src/bin/scripts/dropuser.c b/src/bin/scripts/dropuser.c index 31fa28f7cd..ebcc15209c 100644 --- a/src/bin/scripts/dropuser.c +++ b/src/bin/scripts/dropuser.c @@ -46,6 +46,7 @@ main(int argc, char *argv[]) enum trivalue prompt_password = TRI_DEFAULT; bool echo = false; bool interactive = false; + char dropuser_buf[128]; PQExpBufferData sql; @@ -108,7 +109,11 @@ main(int argc, char *argv[]) if (dropuser == NULL) { if (interactive) - dropuser = simple_prompt("Enter name of role to drop: ", 128, true); + { + simple_prompt("Enter name of role to drop: ", + dropuser_buf, sizeof(dropuser_buf), true); + dropuser = dropuser_buf; + } else { fprintf(stderr, _("%s: missing required argument role name\n"), progname); diff --git a/src/include/port.h b/src/include/port.h index 455f72338c..b81fa4a89e 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -203,7 +203,8 @@ extern char *pgwin32_setlocale(int category, const char *locale); #endif /* WIN32 */ /* Portable prompt handling */ -extern char *simple_prompt(const char *prompt, int maxlen, bool echo); +extern void simple_prompt(const char *prompt, char *destination, size_t destlen, + bool echo); #ifdef WIN32 #define PG_SIGNAL_COUNT 32 diff --git a/src/port/sprompt.c b/src/port/sprompt.c index fd6f16ed30..15ca7a6005 100644 --- a/src/port/sprompt.c +++ b/src/port/sprompt.c @@ -12,33 +12,31 @@ * *------------------------------------------------------------------------- */ +#include "c.h" + +#ifdef HAVE_TERMIOS_H +#include +#endif /* * simple_prompt * * Generalized function especially intended for reading in usernames and - * password interactively. Reads from /dev/tty or stdin/stderr. + * passwords interactively. Reads from /dev/tty or stdin/stderr. * - * prompt: The prompt to print - * maxlen: How many characters to accept + * prompt: The prompt to print, or NULL if none (automatically localized) + * destination: buffer in which to store result + * destlen: allocated length of destination * echo: Set to false if you want to hide what is entered (for passwords) * - * Returns a malloc()'ed string with the input (w/o trailing newline). + * The input (without trailing newline) is returned in the destination buffer, + * with a '\0' appended. */ -#include "c.h" - -#ifdef HAVE_TERMIOS_H -#include -#endif - -extern char *simple_prompt(const char *prompt, int maxlen, bool echo); - -char * -simple_prompt(const char *prompt, int maxlen, bool echo) +void +simple_prompt(const char *prompt, char *destination, size_t destlen, bool echo) { int length; - char *destination; FILE *termin, *termout; @@ -48,14 +46,10 @@ simple_prompt(const char *prompt, int maxlen, bool echo) #else #ifdef WIN32 HANDLE t = NULL; - LPDWORD t_orig = NULL; + DWORD t_orig = 0; #endif #endif - destination = (char *) malloc(maxlen + 1); - if (!destination) - return NULL; - #ifdef WIN32 /* @@ -118,11 +112,10 @@ simple_prompt(const char *prompt, int maxlen, bool echo) if (!echo) { /* get a new handle to turn echo off */ - t_orig = (LPDWORD) malloc(sizeof(DWORD)); t = GetStdHandle(STD_INPUT_HANDLE); /* save the old configuration first */ - GetConsoleMode(t, t_orig); + GetConsoleMode(t, &t_orig); /* set to the new mode */ SetConsoleMode(t, ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT); @@ -136,7 +129,7 @@ simple_prompt(const char *prompt, int maxlen, bool echo) fflush(termout); } - if (fgets(destination, maxlen + 1, termin) == NULL) + if (fgets(destination, destlen, termin) == NULL) destination[0] = '\0'; length = strlen(destination); @@ -170,10 +163,9 @@ simple_prompt(const char *prompt, int maxlen, bool echo) if (!echo) { /* reset to the original console mode */ - SetConsoleMode(t, *t_orig); + SetConsoleMode(t, t_orig); fputs("\n", termout); fflush(termout); - free(t_orig); } #endif #endif @@ -183,6 +175,4 @@ simple_prompt(const char *prompt, int maxlen, bool echo) fclose(termin); fclose(termout); } - - return destination; } -- cgit v1.2.3 From 40c3fe4980e73acb0db75a3c737a4a52e09d4cf4 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Mon, 19 Sep 2016 22:55:43 +0300 Subject: Fix latency calculation when there are \sleep commands in the script. We can't use txn_scheduled to hold the sleep-until time for \sleep, because that interferes with calculation of the latency of the transaction as whole. Backpatch to 9.4, where this bug was introduced. Fabien COELHO Discussion: --- src/bin/pgbench/pgbench.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'src/bin/pgbench') diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 56c37d537e..4676a59020 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -250,6 +250,7 @@ typedef struct int nvariables; /* number of variables */ bool vars_sorted; /* are variables sorted by name? */ int64 txn_scheduled; /* scheduled start time of transaction (usec) */ + int64 sleep_until; /* scheduled start time of next cmd (usec) */ instr_time txn_begin; /* used for measuring schedule lag times */ instr_time stmt_begin; /* used for measuring statement latencies */ int use_file; /* index in sql_scripts for this client */ @@ -1830,6 +1831,7 @@ top: } } + st->sleep_until = st->txn_scheduled; st->sleeping = true; st->throttling = true; st->is_throttled = true; @@ -1842,7 +1844,7 @@ top: { /* are we sleeping? */ if (INSTR_TIME_IS_ZERO(now)) INSTR_TIME_SET_CURRENT(now); - if (INSTR_TIME_GET_MICROSEC(now) < st->txn_scheduled) + if (INSTR_TIME_GET_MICROSEC(now) < st->sleep_until) return true; /* Still sleeping, nothing to do here */ /* Else done sleeping, go ahead with next command */ st->sleeping = false; @@ -2141,7 +2143,7 @@ top: usec *= 1000000; INSTR_TIME_SET_CURRENT(now); - st->txn_scheduled = INSTR_TIME_GET_MICROSEC(now) + usec; + st->sleep_until = INSTR_TIME_GET_MICROSEC(now) + usec; st->sleeping = true; st->listen = true; -- cgit v1.2.3 From 65c65563842cc99fb1c349211581a62dc728eee2 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 21 Sep 2016 13:14:48 +0300 Subject: Fix pgbench's calculation of average latency, when -T is not used. If the test duration was given in # of transactions (-t or no option), rather as a duration (-T), the latency average was always printed as 0. It has been broken ever since the display of latency average was added, in 9.4. Fabien Coelho Discussion: --- src/bin/pgbench/pgbench.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'src/bin/pgbench') diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 4676a59020..9033ff2caa 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -3296,9 +3296,11 @@ printResults(TState *threads, StatsData *total, instr_time total_time, if (throttle_delay || progress || latency_limit) printSimpleStats("latency", &total->latency); else - /* only an average latency computed from the duration is available */ + { + /* no measurement, show average latency computed from run time */ printf("latency average: %.3f ms\n", - 1000.0 * duration * nclients / total->cnt); + 1000.0 * time_include * nclients / total->cnt); + } if (throttle_delay) { -- cgit v1.2.3 From 2a7f4f76434d82eb0d1b5f4f7051043e1dd3ee1a Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 21 Sep 2016 13:24:13 +0300 Subject: Print test parameters like "foo: 123", and results like "foo = 123". The way "latency average" was printed was differently if it was calculated from the overall run time or was measured on a per-transaction basis. Also, the per-script weight is a test parameter, rather than a result, so use the "weight: %f" style for that. Backpatch to 9.6, since the inconsistency on "latency average" was introduced there. Fabien Coelho Discussion: --- src/bin/pgbench/pgbench.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src/bin/pgbench') diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 9033ff2caa..8b24ad50e7 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -3260,6 +3260,7 @@ printResults(TState *threads, StatsData *total, instr_time total_time, tps_exclude = total->cnt / (time_include - (INSTR_TIME_GET_DOUBLE(conn_total_time) / nclients)); + /* Report test parameters. */ printf("transaction type: %s\n", num_scripts == 1 ? sql_script[0].desc : "multiple scripts"); printf("scaling factor: %d\n", scale); @@ -3298,7 +3299,7 @@ printResults(TState *threads, StatsData *total, instr_time total_time, else { /* no measurement, show average latency computed from run time */ - printf("latency average: %.3f ms\n", + printf("latency average = %.3f ms\n", 1000.0 * time_include * nclients / total->cnt); } @@ -3326,7 +3327,7 @@ printResults(TState *threads, StatsData *total, instr_time total_time, { if (num_scripts > 1) printf("SQL script %d: %s\n" - " - weight = %d (targets %.1f%% of total)\n" + " - weight: %d (targets %.1f%% of total)\n" " - " INT64_FORMAT " transactions (%.1f%% of total, tps = %f)\n", i + 1, sql_script[i].desc, sql_script[i].weight, -- cgit v1.2.3 From 12788ae49e1933f463bc59a6efe46c4a01701b76 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Mon, 26 Sep 2016 10:56:02 +0300 Subject: Refactor script execution state machine in pgbench. The doCustom() function had grown into quite a mess. Rewrite it, in a more explicit state machine style, for readability. This also fixes one minor bug: if a script consisted entirely of meta commands, doCustom() never returned to the caller, so progress reports with the -P option were not printed. I don't want to backpatch this refactoring, and the bug is quite insignificant, so only commit this to master, and leave the bug unfixed in back-branches. Review and original bug report by Fabien Coelho. Discussion: --- src/bin/pgbench/pgbench.c | 1106 +++++++++++++++++++++++++++------------------ 1 file changed, 661 insertions(+), 445 deletions(-) (limited to 'src/bin/pgbench') diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 8b24ad50e7..1fb4ae46d5 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -235,25 +235,95 @@ typedef struct StatsData } StatsData; /* - * Connection state + * Connection state machine states. + */ +typedef enum +{ + /* + * The client must first choose a script to execute. Once chosen, it can + * either be throttled (state CSTATE_START_THROTTLE under --rate) or start + * right away (state CSTATE_START_TX). + */ + CSTATE_CHOOSE_SCRIPT, + + /* + * In CSTATE_START_THROTTLE state, we calculate when to begin the next + * transaction, and advance to CSTATE_THROTTLE. CSTATE_THROTTLE state + * sleeps until that moment. (If throttling is not enabled, doCustom() + * falls directly through from CSTATE_START_THROTTLE to CSTATE_START_TX.) + */ + CSTATE_START_THROTTLE, + CSTATE_THROTTLE, + + /* + * CSTATE_START_TX performs start-of-transaction processing. Establishes + * a new connection for the transaction, in --connect mode, and records + * the transaction start time. + */ + CSTATE_START_TX, + + /* + * We loop through these states, to process each command in the script: + * + * CSTATE_START_COMMAND starts the execution of a command. On a SQL + * command, the command is sent to the server, and we move to + * CSTATE_WAIT_RESULT state. On a \sleep meta-command, the timer is set, + * and we enter the CSTATE_SLEEP state to wait for it to expire. Other + * meta-commands are executed immediately. + * + * CSTATE_WAIT_RESULT waits until we get a result set back from the server + * for the current command. + * + * CSTATE_SLEEP waits until the end of \sleep. + * + * CSTATE_END_COMMAND records the end-of-command timestamp, increments the + * command counter, and loops back to CSTATE_START_COMMAND state. + */ + CSTATE_START_COMMAND, + CSTATE_WAIT_RESULT, + CSTATE_SLEEP, + CSTATE_END_COMMAND, + + /* + * CSTATE_END_TX performs end-of-transaction processing. Calculates + * latency, and logs the transaction. In --connect mode, closes the + * current connection. Chooses the next script to execute and starts over + * in CSTATE_START_THROTTLE state, or enters CSTATE_FINISHED if we have no + * more work to do. + */ + CSTATE_END_TX, + + /* + * Final states. CSTATE_ABORTED means that the script execution was + * aborted because a command failed, CSTATE_FINISHED means success. + */ + CSTATE_ABORTED, + CSTATE_FINISHED +} ConnectionStateEnum; + +/* + * Connection state. */ typedef struct { PGconn *con; /* connection handle to DB */ int id; /* client No. */ - int state; /* state No. */ - bool listen; /* whether an async query has been sent */ - bool sleeping; /* whether the client is napping */ - bool throttling; /* whether nap is for throttling */ - bool is_throttled; /* whether transaction throttling is done */ + ConnectionStateEnum state; /* state machine's current state. */ + + int use_file; /* index in sql_script for this client */ + int command; /* command number in script */ + + /* client variables */ Variable *variables; /* array of variable definitions */ int nvariables; /* number of variables */ bool vars_sorted; /* are variables sorted by name? */ + + /* various times about current transaction */ int64 txn_scheduled; /* scheduled start time of transaction (usec) */ int64 sleep_until; /* scheduled start time of next cmd (usec) */ instr_time txn_begin; /* used for measuring schedule lag times */ instr_time stmt_begin; /* used for measuring statement latencies */ - int use_file; /* index in sql_scripts for this client */ + bool prepared[MAX_SCRIPTS]; /* whether client prepared the script */ /* per client collected stats */ @@ -1382,7 +1452,7 @@ evalFunc(TState *thread, CState *st, Assert(nargs == 1); fprintf(stderr, "debug(script=%d,command=%d): ", - st->use_file, st->state + 1); + st->use_file, st->command + 1); if (varg->type == PGBT_INT) fprintf(stderr, "int " INT64_FORMAT "\n", varg->u.ival); @@ -1733,15 +1803,12 @@ preparedStatementName(char *buffer, int file, int state) sprintf(buffer, "P%d_%d", file, state); } -static bool -clientDone(CState *st) +static void +commandFailed(CState *st, char *message) { - if (st->con != NULL) - { - PQfinish(st->con); - st->con = NULL; - } - return false; /* always false */ + fprintf(stderr, + "client %d aborted in command %d of script %d; %s\n", + st->id, st->command, st->use_file, message); } /* return a script number with a weighted choice. */ @@ -1763,425 +1830,595 @@ chooseScript(TState *thread) return i - 1; } -/* return false iff client should be disconnected */ +/* Send a SQL command, using the chosen querymode */ static bool -doCustom(TState *thread, CState *st, StatsData *agg) +sendCommand(CState *st, Command *command) { - PGresult *res; - Command **commands; - bool trans_needs_throttle = false; - instr_time now; + int r; - /* - * gettimeofday() isn't free, so we get the current timestamp lazily the - * first time it's needed, and reuse the same value throughout this - * function after that. This also ensures that e.g. the calculated latency - * reported in the log file and in the totals are the same. Zero means - * "not set yet". Reset "now" when we step to the next command with "goto - * top", though. - */ -top: - INSTR_TIME_SET_ZERO(now); + if (querymode == QUERY_SIMPLE) + { + char *sql; - commands = sql_script[st->use_file].commands; + sql = pg_strdup(command->argv[0]); + sql = assignVariables(st, sql); - /* - * Handle throttling once per transaction by sleeping. It is simpler to - * do this here rather than at the end, because so much complicated logic - * happens below when statements finish. - */ - if (throttle_delay && !st->is_throttled) + if (debug) + fprintf(stderr, "client %d sending %s\n", st->id, sql); + r = PQsendQuery(st->con, sql); + free(sql); + } + else if (querymode == QUERY_EXTENDED) { - /* - * Generate a delay such that the series of delays will approximate a - * Poisson distribution centered on the throttle_delay time. - * - * If transactions are too slow or a given wait is shorter than a - * transaction, the next transaction will start right away. - */ - int64 wait = getPoissonRand(thread, throttle_delay); + const char *sql = command->argv[0]; + const char *params[MAX_ARGS]; - thread->throttle_trigger += wait; - st->txn_scheduled = thread->throttle_trigger; + getQueryParams(st, command, params); - /* stop client if next transaction is beyond pgbench end of execution */ - if (duration > 0 && st->txn_scheduled > end_time) - return clientDone(st); + if (debug) + fprintf(stderr, "client %d sending %s\n", st->id, sql); + r = PQsendQueryParams(st->con, sql, command->argc - 1, + NULL, params, NULL, NULL, 0); + } + else if (querymode == QUERY_PREPARED) + { + char name[MAX_PREPARE_NAME]; + const char *params[MAX_ARGS]; - /* - * If this --latency-limit is used, and this slot is already late so - * that the transaction will miss the latency limit even if it - * completed immediately, we skip this time slot and iterate till the - * next slot that isn't late yet. - */ - if (latency_limit) + if (!st->prepared[st->use_file]) { - int64 now_us; + int j; + Command **commands = sql_script[st->use_file].commands; - if (INSTR_TIME_IS_ZERO(now)) - INSTR_TIME_SET_CURRENT(now); - now_us = INSTR_TIME_GET_MICROSEC(now); - while (thread->throttle_trigger < now_us - latency_limit) + for (j = 0; commands[j] != NULL; j++) { - processXactStats(thread, st, &now, true, agg); - /* next rendez-vous */ - wait = getPoissonRand(thread, throttle_delay); - thread->throttle_trigger += wait; - st->txn_scheduled = thread->throttle_trigger; + PGresult *res; + char name[MAX_PREPARE_NAME]; + + if (commands[j]->type != SQL_COMMAND) + continue; + preparedStatementName(name, st->use_file, j); + res = PQprepare(st->con, name, + commands[j]->argv[0], commands[j]->argc - 1, NULL); + if (PQresultStatus(res) != PGRES_COMMAND_OK) + fprintf(stderr, "%s", PQerrorMessage(st->con)); + PQclear(res); } + st->prepared[st->use_file] = true; } - st->sleep_until = st->txn_scheduled; - st->sleeping = true; - st->throttling = true; - st->is_throttled = true; + getQueryParams(st, command, params); + preparedStatementName(name, st->use_file, st->command); + if (debug) - fprintf(stderr, "client %d throttling " INT64_FORMAT " us\n", - st->id, wait); + fprintf(stderr, "client %d sending %s\n", st->id, name); + r = PQsendQueryPrepared(st->con, name, command->argc - 1, + params, NULL, NULL, 0); } + else /* unknown sql mode */ + r = 0; - if (st->sleeping) - { /* are we sleeping? */ - if (INSTR_TIME_IS_ZERO(now)) - INSTR_TIME_SET_CURRENT(now); - if (INSTR_TIME_GET_MICROSEC(now) < st->sleep_until) - return true; /* Still sleeping, nothing to do here */ - /* Else done sleeping, go ahead with next command */ - st->sleeping = false; - st->throttling = false; + if (r == 0) + { + if (debug) + fprintf(stderr, "client %d could not send %s\n", + st->id, command->argv[0]); + st->ecnt++; + return false; } + else + return true; +} + +/* + * Parse the argument to a \sleep command, and return the requested amount + * of delay, in microseconds. Returns true on success, false on error. + */ +static bool +evaluateSleep(CState *st, int argc, char **argv, int *usecs) +{ + char *var; + int usec; - if (st->listen) - { /* are we receiver? */ - if (commands[st->state]->type == SQL_COMMAND) + if (*argv[1] == ':') + { + if ((var = getVariable(st, argv[1] + 1)) == NULL) { - if (debug) - fprintf(stderr, "client %d receiving\n", st->id); - if (!PQconsumeInput(st->con)) - { /* there's something wrong */ - fprintf(stderr, "client %d aborted in state %d; perhaps the backend died while processing\n", st->id, st->state); - return clientDone(st); - } - if (PQisBusy(st->con)) - return true; /* don't have the whole result yet */ + fprintf(stderr, "%s: undefined variable \"%s\"\n", + argv[0], argv[1]); + return false; } + usec = atoi(var); + } + else + usec = atoi(argv[1]); - /* - * command finished: accumulate per-command execution times in - * thread-local data structure, if per-command latencies are requested - */ - if (is_latencies) - { - if (INSTR_TIME_IS_ZERO(now)) - INSTR_TIME_SET_CURRENT(now); + if (argc > 2) + { + if (pg_strcasecmp(argv[2], "ms") == 0) + usec *= 1000; + else if (pg_strcasecmp(argv[2], "s") == 0) + usec *= 1000000; + } + else + usec *= 1000000; - /* XXX could use a mutex here, but we choose not to */ - addToSimpleStats(&commands[st->state]->stats, - INSTR_TIME_GET_DOUBLE(now) - - INSTR_TIME_GET_DOUBLE(st->stmt_begin)); - } + *usecs = usec; + return true; +} - /* transaction finished: calculate latency and log the transaction */ - if (commands[st->state + 1] == NULL) - { - if (progress || throttle_delay || latency_limit || - per_script_stats || use_log) - processXactStats(thread, st, &now, false, agg); - else - thread->stats.cnt++; - } +/* + * Advance the state machine of a connection, if possible. + */ +static void +doCustom(TState *thread, CState *st, StatsData *agg) +{ + PGresult *res; + Command *command; + instr_time now; + bool end_tx_processed = false; + int64 wait; - if (commands[st->state]->type == SQL_COMMAND) - { - /* - * Read and discard the query result; note this is not included in - * the statement latency numbers. - */ - res = PQgetResult(st->con); - switch (PQresultStatus(res)) - { - case PGRES_COMMAND_OK: - case PGRES_TUPLES_OK: - case PGRES_EMPTY_QUERY: - break; /* OK */ - default: - fprintf(stderr, "client %d aborted in state %d: %s", - st->id, st->state, PQerrorMessage(st->con)); - PQclear(res); - return clientDone(st); - } - PQclear(res); - discard_response(st); - } + /* + * gettimeofday() isn't free, so we get the current timestamp lazily the + * first time it's needed, and reuse the same value throughout this + * function after that. This also ensures that e.g. the calculated + * latency reported in the log file and in the totals are the same. Zero + * means "not set yet". Reset "now" when we execute shell commands or + * expressions, which might take a non-negligible amount of time, though. + */ + INSTR_TIME_SET_ZERO(now); - if (commands[st->state + 1] == NULL) + /* + * Loop in the state machine, until we have to wait for a result from the + * server (or have to sleep, for throttling or for \sleep). + * + * Note: In the switch-statement below, 'break' will loop back here, + * meaning "continue in the state machine". Return is used to return to + * the caller. + */ + for (;;) + { + switch (st->state) { - if (is_connect) - { - PQfinish(st->con); - st->con = NULL; - } + /* + * Select transaction to run. + */ + case CSTATE_CHOOSE_SCRIPT: - ++st->cnt; - if ((st->cnt >= nxacts && duration <= 0) || timer_exceeded) - return clientDone(st); /* exit success */ - } + st->use_file = chooseScript(thread); - /* increment state counter */ - st->state++; - if (commands[st->state] == NULL) - { - st->state = 0; - st->use_file = chooseScript(thread); - commands = sql_script[st->use_file].commands; - if (debug) - fprintf(stderr, "client %d executing script \"%s\"\n", st->id, - sql_script[st->use_file].desc); - st->is_throttled = false; - - /* - * No transaction is underway anymore, which means there is - * nothing to listen to right now. When throttling rate limits - * are active, a sleep will happen next, as the next transaction - * starts. And then in any case the next SQL command will set - * listen back to true. - */ - st->listen = false; - trans_needs_throttle = (throttle_delay > 0); - } - } + if (debug) + fprintf(stderr, "client %d executing script \"%s\"\n", st->id, + sql_script[st->use_file].desc); - if (st->con == NULL) - { - instr_time start, - end; + if (throttle_delay > 0) + st->state = CSTATE_START_THROTTLE; + else + st->state = CSTATE_START_TX; + break; - INSTR_TIME_SET_CURRENT(start); - if ((st->con = doConnect()) == NULL) - { - fprintf(stderr, "client %d aborted while establishing connection\n", - st->id); - return clientDone(st); - } - INSTR_TIME_SET_CURRENT(end); - INSTR_TIME_ACCUM_DIFF(thread->conn_time, end, start); - - /* Reset session-local state */ - st->listen = false; - st->sleeping = false; - st->throttling = false; - st->is_throttled = false; - memset(st->prepared, 0, sizeof(st->prepared)); - } + /* + * Handle throttling once per transaction by sleeping. + */ + case CSTATE_START_THROTTLE: - /* - * This ensures that a throttling delay is inserted before proceeding with - * sql commands, after the first transaction. The first transaction - * throttling is performed when first entering doCustom. - */ - if (trans_needs_throttle) - { - trans_needs_throttle = false; - goto top; - } + /* + * Generate a delay such that the series of delays will + * approximate a Poisson distribution centered on the + * throttle_delay time. + * + * If transactions are too slow or a given wait is shorter + * than a transaction, the next transaction will start right + * away. + */ + Assert(throttle_delay > 0); + wait = getPoissonRand(thread, throttle_delay); - /* Record transaction start time under logging, progress or throttling */ - if ((use_log || progress || throttle_delay || latency_limit || - per_script_stats) && st->state == 0) - { - INSTR_TIME_SET_CURRENT(st->txn_begin); + thread->throttle_trigger += wait; + st->txn_scheduled = thread->throttle_trigger; - /* - * When not throttling, this is also the transaction's scheduled start - * time. - */ - if (!throttle_delay) - st->txn_scheduled = INSTR_TIME_GET_MICROSEC(st->txn_begin); - } + /* + * stop client if next transaction is beyond pgbench end of + * execution + */ + if (duration > 0 && st->txn_scheduled > end_time) + { + st->state = CSTATE_FINISHED; + break; + } - /* Record statement start time if per-command latencies are requested */ - if (is_latencies) - INSTR_TIME_SET_CURRENT(st->stmt_begin); + /* + * If this --latency-limit is used, and this slot is already + * late so that the transaction will miss the latency limit + * even if it completed immediately, we skip this time slot + * and iterate till the next slot that isn't late yet. + */ + if (latency_limit) + { + int64 now_us; - if (commands[st->state]->type == SQL_COMMAND) - { - const Command *command = commands[st->state]; - int r; + if (INSTR_TIME_IS_ZERO(now)) + INSTR_TIME_SET_CURRENT(now); + now_us = INSTR_TIME_GET_MICROSEC(now); + while (thread->throttle_trigger < now_us - latency_limit) + { + processXactStats(thread, st, &now, true, agg); + /* next rendez-vous */ + wait = getPoissonRand(thread, throttle_delay); + thread->throttle_trigger += wait; + st->txn_scheduled = thread->throttle_trigger; + } + } - if (querymode == QUERY_SIMPLE) - { - char *sql; + st->state = CSTATE_THROTTLE; + if (debug) + fprintf(stderr, "client %d throttling " INT64_FORMAT " us\n", + st->id, wait); + break; - sql = pg_strdup(command->argv[0]); - sql = assignVariables(st, sql); + /* + * Wait until it's time to start next transaction. + */ + case CSTATE_THROTTLE: + if (INSTR_TIME_IS_ZERO(now)) + INSTR_TIME_SET_CURRENT(now); + if (INSTR_TIME_GET_MICROSEC(now) < st->txn_scheduled) + return; /* Still sleeping, nothing to do here */ + + /* Else done sleeping, start the transaction */ + st->state = CSTATE_START_TX; + break; - if (debug) - fprintf(stderr, "client %d sending %s\n", st->id, sql); - r = PQsendQuery(st->con, sql); - free(sql); - } - else if (querymode == QUERY_EXTENDED) - { - const char *sql = command->argv[0]; - const char *params[MAX_ARGS]; + /* Start new transaction */ + case CSTATE_START_TX: - getQueryParams(st, command, params); + /* + * Establish connection on first call, or if is_connect is + * true. + */ + if (st->con == NULL) + { + instr_time start; - if (debug) - fprintf(stderr, "client %d sending %s\n", st->id, sql); - r = PQsendQueryParams(st->con, sql, command->argc - 1, - NULL, params, NULL, NULL, 0); - } - else if (querymode == QUERY_PREPARED) - { - char name[MAX_PREPARE_NAME]; - const char *params[MAX_ARGS]; + if (INSTR_TIME_IS_ZERO(now)) + INSTR_TIME_SET_CURRENT(now); + start = now; + if ((st->con = doConnect()) == NULL) + { + fprintf(stderr, "client %d aborted while establishing connection\n", + st->id); + st->state = CSTATE_ABORTED; + break; + } + INSTR_TIME_SET_CURRENT(now); + INSTR_TIME_ACCUM_DIFF(thread->conn_time, now, start); - if (!st->prepared[st->use_file]) - { - int j; + /* Reset session-local state */ + memset(st->prepared, 0, sizeof(st->prepared)); + } - for (j = 0; commands[j] != NULL; j++) + /* + * Record transaction start time under logging, progress or + * throttling. + */ + if (use_log || progress || throttle_delay || latency_limit || + per_script_stats) { - PGresult *res; - char name[MAX_PREPARE_NAME]; + if (INSTR_TIME_IS_ZERO(now)) + INSTR_TIME_SET_CURRENT(now); + st->txn_begin = now; + + /* + * When not throttling, this is also the transaction's + * scheduled start time. + */ + if (!throttle_delay) + st->txn_scheduled = INSTR_TIME_GET_MICROSEC(now); + } - if (commands[j]->type != SQL_COMMAND) - continue; - preparedStatementName(name, st->use_file, j); - res = PQprepare(st->con, name, - commands[j]->argv[0], commands[j]->argc - 1, NULL); - if (PQresultStatus(res) != PGRES_COMMAND_OK) - fprintf(stderr, "%s", PQerrorMessage(st->con)); - PQclear(res); + /* Begin with the first command */ + st->command = 0; + st->state = CSTATE_START_COMMAND; + break; + + /* + * Send a command to server (or execute a meta-command) + */ + case CSTATE_START_COMMAND: + command = sql_script[st->use_file].commands[st->command]; + + /* + * If we reached the end of the script, move to end-of-xact + * processing. + */ + if (command == NULL) + { + st->state = CSTATE_END_TX; + break; } - st->prepared[st->use_file] = true; - } - getQueryParams(st, command, params); - preparedStatementName(name, st->use_file, st->state); + /* + * Record statement start time if per-command latencies are + * requested + */ + if (is_latencies) + { + if (INSTR_TIME_IS_ZERO(now)) + INSTR_TIME_SET_CURRENT(now); + st->stmt_begin = now; + } - if (debug) - fprintf(stderr, "client %d sending %s\n", st->id, name); - r = PQsendQueryPrepared(st->con, name, command->argc - 1, - params, NULL, NULL, 0); - } - else /* unknown sql mode */ - r = 0; + if (command->type == SQL_COMMAND) + { + if (!sendCommand(st, command)) + { + /* + * Failed. Stay in CSTATE_START_COMMAND state, to + * retry. ??? What the point or retrying? Should + * rather abort? + */ + return; + } + else + st->state = CSTATE_WAIT_RESULT; + } + else if (command->type == META_COMMAND) + { + int argc = command->argc, + i; + char **argv = command->argv; - if (r == 0) - { - if (debug) - fprintf(stderr, "client %d could not send %s\n", - st->id, command->argv[0]); - st->ecnt++; - } - else - st->listen = true; /* flags that should be listened */ - } - else if (commands[st->state]->type == META_COMMAND) - { - int argc = commands[st->state]->argc, - i; - char **argv = commands[st->state]->argv; + if (debug) + { + fprintf(stderr, "client %d executing \\%s", st->id, argv[0]); + for (i = 1; i < argc; i++) + fprintf(stderr, " %s", argv[i]); + fprintf(stderr, "\n"); + } - if (debug) - { - fprintf(stderr, "client %d executing \\%s", st->id, argv[0]); - for (i = 1; i < argc; i++) - fprintf(stderr, " %s", argv[i]); - fprintf(stderr, "\n"); - } + if (pg_strcasecmp(argv[0], "sleep") == 0) + { + /* + * A \sleep doesn't execute anything, we just get the + * delay from the argument, and enter the CSTATE_SLEEP + * state. (The per-command latency will be recorded + * in CSTATE_SLEEP state, not here, after the delay + * has elapsed.) + */ + int usec; + + if (!evaluateSleep(st, argc, argv, &usec)) + { + commandFailed(st, "execution of meta-command 'sleep' failed"); + st->state = CSTATE_ABORTED; + break; + } - if (pg_strcasecmp(argv[0], "set") == 0) - { - PgBenchExpr *expr = commands[st->state]->expr; - PgBenchValue result; + if (INSTR_TIME_IS_ZERO(now)) + INSTR_TIME_SET_CURRENT(now); + st->sleep_until = INSTR_TIME_GET_MICROSEC(now) + usec; + st->state = CSTATE_SLEEP; + break; + } + else + { + if (pg_strcasecmp(argv[0], "set") == 0) + { + PgBenchExpr *expr = command->expr; + PgBenchValue result; - if (!evaluateExpr(thread, st, expr, &result)) - { - st->ecnt++; - return true; - } + if (!evaluateExpr(thread, st, expr, &result)) + { + commandFailed(st, "evaluation of meta-command 'set' failed"); + st->state = CSTATE_ABORTED; + break; + } - if (!putVariableNumber(st, argv[0], argv[1], &result)) - { - st->ecnt++; - return true; - } + if (!putVariableNumber(st, argv[0], argv[1], &result)) + { + commandFailed(st, "assignment of meta-command 'set' failed"); + st->state = CSTATE_ABORTED; + break; + } + } + else if (pg_strcasecmp(argv[0], "setshell") == 0) + { + bool ret = runShellCommand(st, argv[1], argv + 2, argc - 2); - st->listen = true; - } - else if (pg_strcasecmp(argv[0], "sleep") == 0) - { - char *var; - int usec; - instr_time now; + if (timer_exceeded) /* timeout */ + { + st->state = CSTATE_FINISHED; + break; + } + else if (!ret) /* on error */ + { + commandFailed(st, "execution of meta-command 'setshell' failed"); + st->state = CSTATE_ABORTED; + break; + } + else + { + /* succeeded */ + } + } + else if (pg_strcasecmp(argv[0], "shell") == 0) + { + bool ret = runShellCommand(st, NULL, argv + 1, argc - 1); - if (*argv[1] == ':') - { - if ((var = getVariable(st, argv[1] + 1)) == NULL) + if (timer_exceeded) /* timeout */ + { + st->state = CSTATE_FINISHED; + break; + } + else if (!ret) /* on error */ + { + commandFailed(st, "execution of meta-command 'shell' failed"); + st->state = CSTATE_ABORTED; + break; + } + else + { + /* succeeded */ + } + } + + /* + * executing the expression or shell command might + * take a non-negligible amount of time, so reset + * 'now' + */ + INSTR_TIME_SET_ZERO(now); + + st->state = CSTATE_END_COMMAND; + } + } + break; + + /* + * Wait for the current SQL command to complete + */ + case CSTATE_WAIT_RESULT: + command = sql_script[st->use_file].commands[st->command]; + if (debug) + fprintf(stderr, "client %d receiving\n", st->id); + if (!PQconsumeInput(st->con)) + { /* there's something wrong */ + commandFailed(st, "perhaps the backend died while processing"); + st->state = CSTATE_ABORTED; + break; + } + if (PQisBusy(st->con)) + return; /* don't have the whole result yet */ + + /* + * Read and discard the query result; + */ + res = PQgetResult(st->con); + switch (PQresultStatus(res)) { - fprintf(stderr, "%s: undefined variable \"%s\"\n", - argv[0], argv[1]); - st->ecnt++; - return true; + case PGRES_COMMAND_OK: + case PGRES_TUPLES_OK: + case PGRES_EMPTY_QUERY: + /* OK */ + PQclear(res); + discard_response(st); + st->state = CSTATE_END_COMMAND; + break; + default: + commandFailed(st, PQerrorMessage(st->con)); + PQclear(res); + st->state = CSTATE_ABORTED; + break; } - usec = atoi(var); - } - else - usec = atoi(argv[1]); + break; - if (argc > 2) - { - if (pg_strcasecmp(argv[2], "ms") == 0) - usec *= 1000; - else if (pg_strcasecmp(argv[2], "s") == 0) - usec *= 1000000; - } - else - usec *= 1000000; + /* + * Wait until sleep is done. This state is entered after a + * \sleep metacommand. The behavior is similar to + * CSTATE_THROTTLE, but proceeds to CSTATE_START_COMMAND + * instead of CSTATE_START_TX. + */ + case CSTATE_SLEEP: + if (INSTR_TIME_IS_ZERO(now)) + INSTR_TIME_SET_CURRENT(now); + if (INSTR_TIME_GET_MICROSEC(now) < st->sleep_until) + return; /* Still sleeping, nothing to do here */ + /* Else done sleeping. */ + st->state = CSTATE_END_COMMAND; + break; - INSTR_TIME_SET_CURRENT(now); - st->sleep_until = INSTR_TIME_GET_MICROSEC(now) + usec; - st->sleeping = true; + /* + * End of command: record stats and proceed to next command. + */ + case CSTATE_END_COMMAND: - st->listen = true; - } - else if (pg_strcasecmp(argv[0], "setshell") == 0) - { - bool ret = runShellCommand(st, argv[1], argv + 2, argc - 2); + /* + * command completed: accumulate per-command execution times + * in thread-local data structure, if per-command latencies + * are requested. + */ + if (is_latencies) + { + if (INSTR_TIME_IS_ZERO(now)) + INSTR_TIME_SET_CURRENT(now); - if (timer_exceeded) /* timeout */ - return clientDone(st); - else if (!ret) /* on error */ - { - st->ecnt++; - return true; - } - else /* succeeded */ - st->listen = true; - } - else if (pg_strcasecmp(argv[0], "shell") == 0) - { - bool ret = runShellCommand(st, NULL, argv + 1, argc - 1); + /* XXX could use a mutex here, but we choose not to */ + command = sql_script[st->use_file].commands[st->command]; + addToSimpleStats(&command->stats, + INSTR_TIME_GET_DOUBLE(now) - + INSTR_TIME_GET_DOUBLE(st->stmt_begin)); + } - if (timer_exceeded) /* timeout */ - return clientDone(st); - else if (!ret) /* on error */ - { - st->ecnt++; - return true; - } - else /* succeeded */ - st->listen = true; - } + /* Go ahead with next command */ + st->command++; + st->state = CSTATE_START_COMMAND; + break; - /* after a meta command, immediately proceed with next command */ - goto top; - } + /* + * End of transaction. + */ + case CSTATE_END_TX: - return true; + /* + * transaction finished: calculate latency and log the + * transaction + */ + if (progress || throttle_delay || latency_limit || + per_script_stats || use_log) + processXactStats(thread, st, &now, false, agg); + else + thread->stats.cnt++; + + if (is_connect) + { + PQfinish(st->con); + st->con = NULL; + INSTR_TIME_SET_ZERO(now); + } + + ++st->cnt; + if ((st->cnt >= nxacts && duration <= 0) || timer_exceeded) + { + /* exit success */ + st->state = CSTATE_FINISHED; + break; + } + + /* + * No transaction is underway anymore. + */ + st->state = CSTATE_CHOOSE_SCRIPT; + + /* + * If we paced through all commands in the script in this + * loop, without returning to the caller even once, do it now. + * This gives the thread a chance to process other + * connections, and to do progress reporting. This can + * currently only happen if the script consists entirely of + * meta-commands. + */ + if (end_tx_processed) + return; + else + { + end_tx_processed = true; + break; + } + + /* + * Final states. Close the connection if it's still open. + */ + case CSTATE_ABORTED: + case CSTATE_FINISHED: + if (st->con != NULL) + { + PQfinish(st->con); + st->con = NULL; + } + return; + } + } } /* @@ -4183,29 +4420,10 @@ threadRun(void *arg) initStats(&aggs, INSTR_TIME_GET_DOUBLE(thread->start_time)); last = aggs; - /* send start up queries in async manner */ + /* initialize explicitely the state machines */ for (i = 0; i < nstate; i++) { - CState *st = &state[i]; - int prev_ecnt = st->ecnt; - Command **commands; - - st->use_file = chooseScript(thread); - commands = sql_script[st->use_file].commands; - if (debug) - fprintf(stderr, "client %d executing script \"%s\"\n", st->id, - sql_script[st->use_file].desc); - if (!doCustom(thread, st, &aggs)) - remains--; /* I've aborted */ - - if (st->ecnt > prev_ecnt && commands[st->state]->type == META_COMMAND) - { - fprintf(stderr, "client %d aborted in state %d; execution of meta-command failed\n", - i, st->state); - remains--; /* I've aborted */ - PQfinish(st->con); - st->con = NULL; - } + state[i].state = CSTATE_CHOOSE_SCRIPT; } while (remains > 0) @@ -4222,59 +4440,60 @@ threadRun(void *arg) for (i = 0; i < nstate; i++) { CState *st = &state[i]; - Command **commands = sql_script[st->use_file].commands; int sock; - if (st->con == NULL) + if (st->state == CSTATE_THROTTLE && timer_exceeded) { + /* interrupt client which has not started a transaction */ + st->state = CSTATE_FINISHED; + remains--; + PQfinish(st->con); + st->con = NULL; continue; } - else if (st->sleeping) + else if (st->state == CSTATE_SLEEP || st->state == CSTATE_THROTTLE) { - if (st->throttling && timer_exceeded) - { - /* interrupt client which has not started a transaction */ - remains--; - st->sleeping = false; - st->throttling = false; - PQfinish(st->con); - st->con = NULL; - continue; - } - else /* just a nap from the script */ - { - int this_usec; + /* a nap from the script, or under throttling */ + int this_usec; - if (min_usec == PG_INT64_MAX) - { - instr_time now; - - INSTR_TIME_SET_CURRENT(now); - now_usec = INSTR_TIME_GET_MICROSEC(now); - } + if (min_usec == PG_INT64_MAX) + { + instr_time now; - this_usec = st->txn_scheduled - now_usec; - if (min_usec > this_usec) - min_usec = this_usec; + INSTR_TIME_SET_CURRENT(now); + now_usec = INSTR_TIME_GET_MICROSEC(now); } + + this_usec = (st->state == CSTATE_SLEEP ? + st->sleep_until : st->txn_scheduled) - now_usec; + if (min_usec > this_usec) + min_usec = this_usec; } - else if (commands[st->state]->type == META_COMMAND) + else if (st->state == CSTATE_WAIT_RESULT) { - min_usec = 0; /* the connection is ready to run */ + /* + * waiting for result from server - nothing to do unless the + * socket is readable + */ + sock = PQsocket(st->con); + if (sock < 0) + { + fprintf(stderr, "invalid socket: %s", PQerrorMessage(st->con)); + goto done; + } + + FD_SET(sock, &input_mask); + + if (maxsock < sock) + maxsock = sock; break; } - - sock = PQsocket(st->con); - if (sock < 0) + else if (st->state != CSTATE_ABORTED && st->state != CSTATE_FINISHED) { - fprintf(stderr, "invalid socket: %s", PQerrorMessage(st->con)); - goto done; + /* the connection is ready to run */ + min_usec = 0; + break; } - - FD_SET(sock, &input_mask); - - if (maxsock < sock) - maxsock = sock; } /* also wake up to print the next progress report on time */ @@ -4324,14 +4543,13 @@ threadRun(void *arg) } } - /* ok, backend returns reply */ + /* ok, advance the state machine of each connection */ for (i = 0; i < nstate; i++) { CState *st = &state[i]; - Command **commands = sql_script[st->use_file].commands; - int prev_ecnt = st->ecnt; + bool ready; - if (st->con) + if (st->state == CSTATE_WAIT_RESULT && st->con) { int sock = PQsocket(st->con); @@ -4341,21 +4559,19 @@ threadRun(void *arg) PQerrorMessage(st->con)); goto done; } - if (FD_ISSET(sock, &input_mask) || - commands[st->state]->type == META_COMMAND) - { - if (!doCustom(thread, st, &aggs)) - remains--; /* I've aborted */ - } + + ready = FD_ISSET(sock, &input_mask); } + else if (st->state == CSTATE_FINISHED || st->state == CSTATE_ABORTED) + ready = false; + else + ready = true; - if (st->ecnt > prev_ecnt && commands[st->state]->type == META_COMMAND) + if (ready) { - fprintf(stderr, "client %d aborted in state %d; execution of meta-command failed\n", - i, st->state); - remains--; /* I've aborted */ - PQfinish(st->con); - st->con = NULL; + doCustom(thread, st, &aggs); + if (st->state == CSTATE_FINISHED || st->state == CSTATE_ABORTED) + remains--; } } -- cgit v1.2.3 From 9779bda86c026e645773a3308f9169c7c0791f7c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 26 Sep 2016 20:23:50 -0400 Subject: Fix newly-introduced issues in pgbench. The result of FD_ISSET() doesn't necessarily fit in a bool, though assigning it to one might accidentally work depending on platform and which socket FD number is being inquired of. Rewrite to test it with if(), rather than making any specific assumption about the result width, to match the way every other such call in PG is written. Don't break out of the input_mask-filling loop after finding the first client that we're waiting for results from. That mostly breaks parallel query management. Also, if we choose not to call select(), be sure to clear out any bits the mask-filling loop might have set, so that we don't accidentally call doCustom for clients we don't know have input. Doing so would likely be harmless, but it's a waste of cycles and doesn't seem to be intended. Make this_usec wide enough. (Yeah, the value would usually fit in an int, but then why are we using int64 everywhere else?) Minor cosmetic adjustments, mostly comment improvements. Problems introduced by commit 12788ae49. The first issue was discovered by buildfarm testing, the others by code review. --- src/bin/pgbench/pgbench.c | 82 ++++++++++++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 33 deletions(-) (limited to 'src/bin/pgbench') diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 1fb4ae46d5..d44cfdab49 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -299,7 +299,7 @@ typedef enum */ CSTATE_ABORTED, CSTATE_FINISHED -} ConnectionStateEnum; +} ConnectionStateEnum; /* * Connection state. @@ -4420,43 +4420,43 @@ threadRun(void *arg) initStats(&aggs, INSTR_TIME_GET_DOUBLE(thread->start_time)); last = aggs; - /* initialize explicitely the state machines */ + /* explicitly initialize the state machines */ for (i = 0; i < nstate; i++) { state[i].state = CSTATE_CHOOSE_SCRIPT; } + /* loop till all clients have terminated */ while (remains > 0) { fd_set input_mask; - int maxsock; /* max socket number to be waited */ - int64 now_usec = 0; + int maxsock; /* max socket number to be waited for */ int64 min_usec; + int64 now_usec = 0; /* set this only if needed */ + /* identify which client sockets should be checked for input */ FD_ZERO(&input_mask); - maxsock = -1; min_usec = PG_INT64_MAX; for (i = 0; i < nstate; i++) { CState *st = &state[i]; - int sock; if (st->state == CSTATE_THROTTLE && timer_exceeded) { - /* interrupt client which has not started a transaction */ + /* interrupt client that has not started a transaction */ st->state = CSTATE_FINISHED; - remains--; PQfinish(st->con); st->con = NULL; - continue; + remains--; } else if (st->state == CSTATE_SLEEP || st->state == CSTATE_THROTTLE) { /* a nap from the script, or under throttling */ - int this_usec; + int64 this_usec; - if (min_usec == PG_INT64_MAX) + /* get current time if needed */ + if (now_usec == 0) { instr_time now; @@ -4464,6 +4464,7 @@ threadRun(void *arg) now_usec = INSTR_TIME_GET_MICROSEC(now); } + /* min_usec should be the minimum delay across all clients */ this_usec = (st->state == CSTATE_SLEEP ? st->sleep_until : st->txn_scheduled) - now_usec; if (min_usec > this_usec) @@ -4475,22 +4476,26 @@ threadRun(void *arg) * waiting for result from server - nothing to do unless the * socket is readable */ - sock = PQsocket(st->con); + int sock = PQsocket(st->con); + if (sock < 0) { - fprintf(stderr, "invalid socket: %s", PQerrorMessage(st->con)); + fprintf(stderr, "invalid socket: %s", + PQerrorMessage(st->con)); goto done; } FD_SET(sock, &input_mask); - if (maxsock < sock) maxsock = sock; - break; } - else if (st->state != CSTATE_ABORTED && st->state != CSTATE_FINISHED) + else if (st->state != CSTATE_ABORTED && + st->state != CSTATE_FINISHED) { - /* the connection is ready to run */ + /* + * This client thread is ready to do something, so we don't + * want to wait. No need to examine additional clients. + */ min_usec = 0; break; } @@ -4515,9 +4520,10 @@ threadRun(void *arg) } /* - * Sleep until we receive data from the server, or a nap-time - * specified in the script ends, or it's time to print a progress - * report. + * If no clients are ready to execute actions, sleep until we receive + * data from the server, or a nap-time specified in the script ends, + * or it's time to print a progress report. Update input_mask to show + * which client(s) received data. */ if (min_usec > 0 && maxsock != -1) { @@ -4536,21 +4542,29 @@ threadRun(void *arg) if (nsocks < 0) { if (errno == EINTR) + { + /* On EINTR, go back to top of loop */ continue; + } /* must be something wrong */ fprintf(stderr, "select() failed: %s\n", strerror(errno)); goto done; } } + else + { + /* If we didn't call select(), don't try to read any data */ + FD_ZERO(&input_mask); + } /* ok, advance the state machine of each connection */ for (i = 0; i < nstate; i++) { CState *st = &state[i]; - bool ready; - if (st->state == CSTATE_WAIT_RESULT && st->con) + if (st->state == CSTATE_WAIT_RESULT) { + /* don't call doCustom unless data is available */ int sock = PQsocket(st->con); if (sock < 0) @@ -4560,22 +4574,24 @@ threadRun(void *arg) goto done; } - ready = FD_ISSET(sock, &input_mask); + if (!FD_ISSET(sock, &input_mask)) + continue; } - else if (st->state == CSTATE_FINISHED || st->state == CSTATE_ABORTED) - ready = false; - else - ready = true; - - if (ready) + else if (st->state == CSTATE_FINISHED || + st->state == CSTATE_ABORTED) { - doCustom(thread, st, &aggs); - if (st->state == CSTATE_FINISHED || st->state == CSTATE_ABORTED) - remains--; + /* this client is done, no need to consider it anymore */ + continue; } + + doCustom(thread, st, &aggs); + + /* If doCustom changed client to finished state, reduce remains */ + if (st->state == CSTATE_FINISHED || st->state == CSTATE_ABORTED) + remains--; } - /* progress report by thread 0 for all threads */ + /* progress report is made by thread 0 for all threads */ if (progress && thread->tid == 0) { instr_time now_time; -- cgit v1.2.3 From 41124a91e61fc6d9681c1e8b15ba30494e84d643 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Wed, 9 Nov 2016 16:26:32 -0500 Subject: pgbench: Allow the transaction log file prefix to be changed. Masahiko Sawada, reviewed by Fabien Coelho and Beena Emerson, with some a bit of wordsmithing and cosmetic adjustment by me. --- doc/src/sgml/ref/pgbench.sgml | 26 +++++++++++++++++++------- src/bin/pgbench/pgbench.c | 20 ++++++++++++++++++-- 2 files changed, 37 insertions(+), 9 deletions(-) (limited to 'src/bin/pgbench') diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index 285608d508..3a65729bf3 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -614,6 +614,16 @@ pgbench options dbname + + + + + Set the filename prefix for the transaction log file created by + + + + @@ -1121,15 +1131,17 @@ END; With the , pgbench writes the time taken by each transaction to a log file. The log file will be named - pgbench_log.nnn, where - nnn is the PID of the pgbench process. - If the