Re: pgbench to the MAXINT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Em 10-01-2011 05:25, Greg Smith escreveu:
Euler Taveira de Oliveira wrote:
Em 07-01-2011 22:59, Greg Smith escreveu:
setrandom: invalid maximum number -2147467296

It is failing at atoi() circa pgbench.c:1036. But it just the first
one. There are some variables and constants that need to be converted
to int64 and some functions that must speak 64-bit such as getrand().
Are you working on a patch?

http://archives.postgresql.org/pgsql-hackers/2010-01/msg02868.php
http://archives.postgresql.org/message-id/4C326F46.4050801@xxxxxxxxxxxxxxx

Greg, I just improved your patch. I tried to work around the problems pointed out in the above threads. Also, I want to raise some points:

(i) If we want to support and scale factor greater than 21474 we have to convert some columns to bigint; it will change the test. From the portability point it is a pity but as we have never supported it I'm not too worried about it. Why? Because it will use bigint columns only if the scale factor is greater than 21474. Is it a problem? I don't think so because generally people compare tests with the same scale factor.

(ii) From the performance perspective, we need to test if the modifications don't impact performance. I don't create another code path for 64-bit modifications (it is too ugly) and I'm afraid some modifications affect the 32-bit performance. I'm in a position to test it though because I don't have a big machine ATM. Greg, could you lead these tests?

(iii) I decided to copy scanint8() (called strtoint64 there) from backend (Robert suggestion [1]) because Tom pointed out that strtoll() has portability issues. I replaced atoi() with strtoint64() but didn't do any performance tests.

Comments?


[1] http://archives.postgresql.org/pgsql-hackers/2010-07/msg00173.php


--
  Euler Taveira de Oliveira
  http://www.timbira.com/
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 7c2ca6e..e9eb720 100644
*** a/contrib/pgbench/pgbench.c
--- b/contrib/pgbench/pgbench.c
***************
*** 60,65 ****
--- 60,67 ----
  #define INT64_MAX	INT64CONST(0x7FFFFFFFFFFFFFFF)
  #endif
  
+ #define MAX_RANDOM_VALUE64	INT64_MAX
+ 
  /*
   * Multi-platform pthread implementations
   */
*************** usage(const char *progname)
*** 364,378 ****
  		   progname, progname);
  }
  
  /* random number generator: uniform distribution from min to max inclusive */
! static int
! getrand(int min, int max)
  {
  	/*
  	 * Odd coding is so that min and max have approximately the same chance of
  	 * being selected as do numbers between them.
  	 */
! 	return min + (int) (((max - min + 1) * (double) random()) / (MAX_RANDOM_VALUE + 1.0));
  }
  
  /* call PQexec() and exit() on failure */
--- 366,451 ----
  		   progname, progname);
  }
  
+ /*
+  * strtoint64 -- convert a string to 64-bit integer
+  *
+  * this function is a modified version of scanint8() from
+  * src/backend/utils/adt/int8.c.
+  *
+  * XXX should it have a return value?
+  *
+  */
+ static int64
+ strtoint64(const char *str)
+ {
+ 	const char *ptr = str;
+ 	int64		result = 0;
+ 	int			sign = 1;
+ 
+ 	/*
+ 	 * Do our own scan, rather than relying on sscanf which might be broken
+ 	 * for long long.
+ 	 */
+ 
+ 	/* skip leading spaces */
+ 	while (*ptr && isspace((unsigned char) *ptr))
+ 		ptr++;
+ 
+ 	/* handle sign */
+ 	if (*ptr == '-')
+ 	{
+ 		ptr++;
+ 
+ 		/*
+ 		 * Do an explicit check for INT64_MIN.	Ugly though this is, it's
+ 		 * cleaner than trying to get the loop below to handle it portably.
+ 		 */
+ 		if (strncmp(ptr, "9223372036854775808", 19) == 0)
+ 		{
+ 			result = -INT64CONST(0x7fffffffffffffff) - 1;
+ 			ptr += 19;
+ 			goto gotdigits;
+ 		}
+ 		sign = -1;
+ 	}
+ 	else if (*ptr == '+')
+ 		ptr++;
+ 
+ 	/* require at least one digit */
+ 	if (!isdigit((unsigned char) *ptr))
+ 		fprintf(stderr, "invalid input syntax for integer: \"%s\"\n", str);
+ 
+ 	/* process digits */
+ 	while (*ptr && isdigit((unsigned char) *ptr))
+ 	{
+ 		int64		tmp = result * 10 + (*ptr++ - '0');
+ 
+ 		if ((tmp / 10) != result)		/* overflow? */
+ 			fprintf(stderr, "value \"%s\" is out of range for type bigint\n", str);
+ 		result = tmp;
+ 	}
+ 
+ gotdigits:
+ 
+ 	/* allow trailing whitespace, but not other trailing chars */
+ 	while (*ptr != '\0' && isspace((unsigned char) *ptr))
+ 		ptr++;
+ 
+ 	if (*ptr != '\0')
+ 		fprintf(stderr, "invalid input syntax for integer: \"%s\"\n", str);
+ 
+ 	return ((sign < 0) ? -result : result);
+ }
+ 
  /* random number generator: uniform distribution from min to max inclusive */
! static int64
! getrand(int64 min, int64 max)
  {
  	/*
  	 * Odd coding is so that min and max have approximately the same chance of
  	 * being selected as do numbers between them.
  	 */
! 	return min + (int64) (((max - min + 1) * (double) random()) / (MAX_RANDOM_VALUE64 + 1.0));
  }
  
  /* call PQexec() and exit() on failure */
*************** top:
*** 887,893 ****
  		if (commands[st->state] == NULL)
  		{
  			st->state = 0;
! 			st->use_file = getrand(0, num_files - 1);
  			commands = sql_files[st->use_file];
  		}
  	}
--- 960,966 ----
  		if (commands[st->state] == NULL)
  		{
  			st->state = 0;
! 			st->use_file = (int) getrand(0, num_files - 1);
  			commands = sql_files[st->use_file];
  		}
  	}
*************** top:
*** 1007,1013 ****
  		if (pg_strcasecmp(argv[0], "setrandom") == 0)
  		{
  			char	   *var;
! 			int			min,
  						max;
  			char		res[64];
  
--- 1080,1086 ----
  		if (pg_strcasecmp(argv[0], "setrandom") == 0)
  		{
  			char	   *var;
! 			int64		min,
  						max;
  			char		res[64];
  
*************** top:
*** 1019,1033 ****
  					st->ecnt++;
  					return true;
  				}
! 				min = atoi(var);
  			}
  			else
! 				min = atoi(argv[2]);
  
  #ifdef NOT_USED
  			if (min < 0)
  			{
! 				fprintf(stderr, "%s: invalid minimum number %d\n", argv[0], min);
  				st->ecnt++;
  				return;
  			}
--- 1092,1106 ----
  					st->ecnt++;
  					return true;
  				}
! 				min = strtoint64(var);
  			}
  			else
! 				min = strtoint64(argv[2]);
  
  #ifdef NOT_USED
  			if (min < 0)
  			{
! 				fprintf(stderr, "%s: invalid minimum number " INT64_FORMAT "\n", argv[0], min);
  				st->ecnt++;
  				return;
  			}
*************** top:
*** 1041,1062 ****
  					st->ecnt++;
  					return true;
  				}
! 				max = atoi(var);
  			}
  			else
! 				max = atoi(argv[3]);
  
! 			if (max < min || max > MAX_RANDOM_VALUE)
  			{
! 				fprintf(stderr, "%s: invalid maximum number %d\n", argv[0], max);
  				st->ecnt++;
  				return true;
  			}
  
  #ifdef DEBUG
! 			printf("min: %d max: %d random: %d\n", min, max, getrand(min, max));
  #endif
! 			snprintf(res, sizeof(res), "%d", getrand(min, max));
  
  			if (!putVariable(st, argv[0], argv[1], res))
  			{
--- 1114,1135 ----
  					st->ecnt++;
  					return true;
  				}
! 				max = strtoint64(var);
  			}
  			else
! 				max = strtoint64(argv[3]);
  
! 			if (max < min || max > MAX_RANDOM_VALUE64)
  			{
! 				fprintf(stderr, "%s: invalid maximum number " INT64_FORMAT "\n", argv[0], max);
  				st->ecnt++;
  				return true;
  			}
  
  #ifdef DEBUG
! 			printf("min: " INT64_FORMAT " max: " INT64_FORMAT " random: " INT64_FORMAT "\n", min, max, getrand(min, max));
  #endif
! 			snprintf(res, sizeof(res), INT64_FORMAT, getrand(min, max));
  
  			if (!putVariable(st, argv[0], argv[1], res))
  			{
*************** top:
*** 1069,1075 ****
  		else if (pg_strcasecmp(argv[0], "set") == 0)
  		{
  			char	   *var;
! 			int			ope1,
  						ope2;
  			char		res[64];
  
--- 1142,1148 ----
  		else if (pg_strcasecmp(argv[0], "set") == 0)
  		{
  			char	   *var;
! 			int64		ope1,
  						ope2;
  			char		res[64];
  
*************** top:
*** 1081,1093 ****
  					st->ecnt++;
  					return true;
  				}
! 				ope1 = atoi(var);
  			}
  			else
! 				ope1 = atoi(argv[2]);
  
  			if (argc < 5)
! 				snprintf(res, sizeof(res), "%d", ope1);
  			else
  			{
  				if (*argv[4] == ':')
--- 1154,1166 ----
  					st->ecnt++;
  					return true;
  				}
! 				ope1 = strtoint64(var);
  			}
  			else
! 				ope1 = strtoint64(argv[2]);
  
  			if (argc < 5)
! 				snprintf(res, sizeof(res), INT64_FORMAT, ope1);
  			else
  			{
  				if (*argv[4] == ':')
*************** top:
*** 1098,1114 ****
  						st->ecnt++;
  						return true;
  					}
! 					ope2 = atoi(var);
  				}
  				else
! 					ope2 = atoi(argv[4]);
  
  				if (strcmp(argv[3], "+") == 0)
! 					snprintf(res, sizeof(res), "%d", ope1 + ope2);
  				else if (strcmp(argv[3], "-") == 0)
! 					snprintf(res, sizeof(res), "%d", ope1 - ope2);
  				else if (strcmp(argv[3], "*") == 0)
! 					snprintf(res, sizeof(res), "%d", ope1 * ope2);
  				else if (strcmp(argv[3], "/") == 0)
  				{
  					if (ope2 == 0)
--- 1171,1187 ----
  						st->ecnt++;
  						return true;
  					}
! 					ope2 = strtoint64(var);
  				}
  				else
! 					ope2 = strtoint64(argv[4]);
  
  				if (strcmp(argv[3], "+") == 0)
! 					snprintf(res, sizeof(res), INT64_FORMAT, ope1 + ope2);
  				else if (strcmp(argv[3], "-") == 0)
! 					snprintf(res, sizeof(res), INT64_FORMAT, ope1 - ope2);
  				else if (strcmp(argv[3], "*") == 0)
! 					snprintf(res, sizeof(res), INT64_FORMAT, ope1 * ope2);
  				else if (strcmp(argv[3], "/") == 0)
  				{
  					if (ope2 == 0)
*************** top:
*** 1117,1123 ****
  						st->ecnt++;
  						return true;
  					}
! 					snprintf(res, sizeof(res), "%d", ope1 / ope2);
  				}
  				else
  				{
--- 1190,1196 ----
  						st->ecnt++;
  						return true;
  					}
! 					snprintf(res, sizeof(res), INT64_FORMAT, ope1 / ope2);
  				}
  				else
  				{
*************** init(void)
*** 1239,1247 ****
  		"drop table if exists pgbench_tellers",
  		"create table pgbench_tellers(tid int not null,bid int,tbalance int,filler char(84)) with (fillfactor=%d)",
  		"drop table if exists pgbench_accounts",
! 		"create table pgbench_accounts(aid int not null,bid int,abalance int,filler char(84)) with (fillfactor=%d)",
  		"drop table if exists pgbench_history",
! 		"create table pgbench_history(tid int,bid int,aid int,delta int,mtime timestamp,filler char(22))"
  	};
  	static char *DDLAFTERs[] = {
  		"alter table pgbench_branches add primary key (bid)",
--- 1312,1320 ----
  		"drop table if exists pgbench_tellers",
  		"create table pgbench_tellers(tid int not null,bid int,tbalance int,filler char(84)) with (fillfactor=%d)",
  		"drop table if exists pgbench_accounts",
! 		"create table pgbench_accounts(aid %s not null,bid int,abalance int,filler char(84)) with (fillfactor=%d)",
  		"drop table if exists pgbench_history",
! 		"create table pgbench_history(tid int,bid int,aid %s,delta int,mtime timestamp,filler char(22))"
  	};
  	static char *DDLAFTERs[] = {
  		"alter table pgbench_branches add primary key (bid)",
*************** init(void)
*** 1253,1258 ****
--- 1326,1332 ----
  	PGresult   *res;
  	char		sql[256];
  	int			i;
+ 	int64		k;
  
  	if ((con = doConnect()) == NULL)
  		exit(1);
*************** init(void)
*** 1263,1270 ****
  		 * set fillfactor for branches, tellers and accounts tables
  		 */
  		if ((strstr(DDLs[i], "create table pgbench_branches") == DDLs[i]) ||
! 			(strstr(DDLs[i], "create table pgbench_tellers") == DDLs[i]) ||
! 			(strstr(DDLs[i], "create table pgbench_accounts") == DDLs[i]))
  		{
  			char		ddl_stmt[128];
  
--- 1337,1343 ----
  		 * set fillfactor for branches, tellers and accounts tables
  		 */
  		if ((strstr(DDLs[i], "create table pgbench_branches") == DDLs[i]) ||
! 			(strstr(DDLs[i], "create table pgbench_tellers") == DDLs[i]))
  		{
  			char		ddl_stmt[128];
  
*************** init(void)
*** 1272,1277 ****
--- 1345,1382 ----
  			executeStatement(con, ddl_stmt);
  			continue;
  		}
+ 		else if (strstr(DDLs[i], "create table pgbench_accounts") == DDLs[i])
+ 		{
+ 			char		ddl_stmt[128];
+ 
+ 			/*
+ 			 * consider using bigint columns in cases where scale factor is
+ 			 * bigger than INT_MAX / naccounts. For small scale factors, we
+ 			 * still use int columns for compatibility reasons.
+ 			 */
+ 			if ((naccounts * scale) > INT64CONST(0x7fffffff))
+ 				snprintf(ddl_stmt, 128, DDLs[i], "bigint", fillfactor);
+ 			else
+ 				snprintf(ddl_stmt, 128, DDLs[i], "int", fillfactor);
+ 			executeStatement(con, ddl_stmt);
+ 			continue;
+ 		}
+ 		else if (strstr(DDLs[i], "create table pgbench_history") == DDLs[i])
+ 		{
+ 			char		ddl_stmt[128];
+ 
+ 			/*
+ 			 * consider using bigint columns in cases where scale factor is
+ 			 * bigger than INT_MAX / naccounts. For small scale factors, we
+ 			 * still use int columns for compatibility reasons.
+ 			 */
+ 			if ((naccounts * scale) > INT64CONST(0x7fffffff))
+ 				snprintf(ddl_stmt, 128, DDLs[i], "bigint");
+ 			else
+ 				snprintf(ddl_stmt, 128, DDLs[i], "int");
+ 			executeStatement(con, ddl_stmt);
+ 			continue;
+ 		}
  		else
  			executeStatement(con, DDLs[i]);
  	}
*************** init(void)
*** 1309,1319 ****
  	}
  	PQclear(res);
  
! 	for (i = 0; i < naccounts * scale; i++)
  	{
! 		int			j = i + 1;
  
! 		snprintf(sql, 256, "%d\t%d\t%d\t\n", j, i / naccounts + 1, 0);
  		if (PQputline(con, sql))
  		{
  			fprintf(stderr, "PQputline failed\n");
--- 1414,1424 ----
  	}
  	PQclear(res);
  
! 	for (k = 0; k < naccounts * scale; k++)
  	{
! 		int64			j = k + 1;
  
! 		snprintf(sql, 256, INT64_FORMAT "\t" INT64_FORMAT "\t%d\t\n", j, k / naccounts + 1, 0);
  		if (PQputline(con, sql))
  		{
  			fprintf(stderr, "PQputline failed\n");
*************** init(void)
*** 1321,1327 ****
  		}
  
  		if (j % 10000 == 0)
! 			fprintf(stderr, "%d tuples done.\n", j);
  	}
  	if (PQputline(con, "\\.\n"))
  	{
--- 1426,1432 ----
  		}
  
  		if (j % 10000 == 0)
! 			fprintf(stderr, INT64_FORMAT " tuples done.\n", j);
  	}
  	if (PQputline(con, "\\.\n"))
  	{
-- 
Sent via pgsql-performance mailing list (pgsql-performance@xxxxxxxxxxxxxx)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-performance

[Postgresql General]     [Postgresql PHP]     [PHP Users]     [PHP Home]     [PHP on Windows]     [Kernel Newbies]     [PHP Classes]     [PHP Books]     [PHP Databases]     [Yosemite]

  Powered by Linux