Hi,
attached is a new version of the patch that
(a) converts the 'log_step_seconds' variable to a constant (and does
not allow changing it using a command-line option etc.)
(b) keeps the current logging as a default
(c) adds a "-q" switch that enables the new logging with a 5-second
interval
I'm still not convinced there should be yet another know for tuning the
log interval - opinions?
Tomas
On 11.12.2012 10:23, Jeevan Chalke wrote:
>
>
>
> On Sun, Dec 9, 2012 at 8:11 AM, Tomas Vondra <t...@fuzzy.cz
> <mailto:t...@fuzzy.cz>> wrote:
>
> On 20.11.2012 08:22, Jeevan Chalke wrote:
> > Hi,
> >
> >
> > On Tue, Nov 20, 2012 at 12:08 AM, Tomas Vondra <t...@fuzzy.cz
> <mailto:t...@fuzzy.cz>
> > <mailto:t...@fuzzy.cz <mailto:t...@fuzzy.cz>>> wrote:
> >
> > On 19.11.2012 11:59, Jeevan Chalke wrote:
> > > Hi,
> > >
> > > I gone through the discussion for this patch and here is my
> review:
> > >
> > > The main aim of this patch is to reduce the number of log
> lines. It is
> > > also suggested to use an options to provide the interval but
> few of us
> > > are not much agree on it. So final discussion ended at
> keeping 5 sec
> > > interval between each log line.
> > >
> > > However, I see, there are two types of users here:
> > > 1. Who likes these log lines, so that they can troubleshoot some
> > > slowness and all
> > > 2. Who do not like these log lines.
> >
> > Who likes these lines / needs them for something useful?
> >
> >
> > No idea. I fall in second category.
> >
> > But from the discussion, I believe some people may need detailed
> (or lot
> > more) output.
>
> I've read the thread again and my impression is that no one really needs
> or likes those lines, but
>
> (1) it's rather pointless to print a message every 100k rows, as it
> usually fills the console with garbabe
>
> (2) it's handy to have regular updates of the progress
>
> I don't think there're people (in the thread) that require to keep the
> current amount of log messages.
>
> But there might be users who actually use the current logs to do
> something (although I can't imagine what). If we want to do this in a
> backwards compatible way, we should probably use a new option (e.g.
> "-q") to enable the new (less verbose) logging.
>
> Do we want to allow both types of logging, or shall we keep only the new
> one? If both, which one should be the default one?
>
>
> Both the options are fine with me, but the default should be the current
> behaviour.
>
>
> > > So keeping these in mind, I rather go for an option which
> will control
> > > this. People falling in category one can set this option to
> very low
> > > where as users falling under second category can keep it high.
> >
> > So what option(s) would you expect? Something that tunes the
> interval
> > length or something else?
> >
> >
> > Interval length.
>
> Well, I can surely imagine something like "--interval N".
>
>
> +1
>
>
>
> > A switch that'd choose between the old and new behavior might
> be a good
> > idea, but I'd strongly vote against "automagic" heuristics. It
> makes the
> > behavior very difficult to predict and I really don't want to
> force the
> > users to wonder whether the long delay is due to general
> slowness of the
> > machine or some "clever" logic that causes long delays between log
> > messages.
> >
> > That's why I choose a very simple approach with constant time
> interval.
> > It does what I was aiming for (less logs) and it's easy to
> predict.
> > Sure, we could choose different interval (or make it an option).
> >
> >
> > I am preferring an option for choosing an interval, say from 1
> second to
> > 10 seconds.
>
> Ummmm, why not to allow arbitrary integer? Why saying 1 to 10 seconds?
>
>
> Hmm.. actually, I have no issues with any number there. Just put 1..10
> as we hard-coded it 5. No particular reason as such.
>
>
>
> > BTW, what if, we put one log message every 10% (or 5%) with time taken
> > (time taken for last 10% (or 5%) and cumulative) over 5 seconds ?
> > This will have only 10 (or 20) lines per pgbench initialisation.
> > And since we are showing time taken for each block, if any slowness
> > happens, one can easily find a block by looking at the timings and
> > troubleshoot it.
> > Though 10% or 5% is again a debatable number, but keeping it constant
> > will eliminate the requirement of an option.
>
> That's what I originally proposed in September (see the messages from
> 17/9), and Alvaro was not relly excited about this.
>
> Attached is a patch with fixed whitespace / indentation errors etc.
> Otherwise it's the same as the previous version.
>
>
> OK. Looks good now.
>
> Any other views / suggestions are welcome.
>
> Thanks
>
>
> Tomas
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org
> <mailto:pgsql-hackers@postgresql.org>)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>
>
>
> --
> Jeevan B Chalke
> Senior Software Engineer, R&D
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company
>
> Phone: +91 20 30589500
>
> Website: www.enterprisedb.com <http://www.enterprisedb.com>
> EnterpriseDB Blog: http://blogs.enterprisedb.com/
> Follow us on Twitter: http://www.twitter.com/enterprisedb
>
> This e-mail message (and any attachment) is intended for the use of the
> individual or entity to whom it is addressed. This message contains
> information from EnterpriseDB Corporation that may be privileged,
> confidential, or exempt from disclosure under applicable law. If you are
> not the intended recipient or authorized to receive this for the
> intended recipient, any use, dissemination, distribution, retention,
> archiving, or copying of this communication is strictly prohibited. If
> you have received this e-mail in error, please notify the sender
> immediately by reply e-mail and delete this message.
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 334ce4c..14d8ad5 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -39,6 +39,7 @@
#include "portability/instr_time.h"
#include <ctype.h>
+#include <math.h>
#ifndef WIN32
#include <sys/time.h>
@@ -102,6 +103,7 @@ extern int optind;
#define MAXCLIENTS 1024
#endif
+#define LOG_STEP_SECONDS 5 /* seconds between log messages */
#define DEFAULT_NXACTS 10 /* default nxacts */
int nxacts = 0; /* number of
transactions per client */
@@ -135,11 +137,6 @@ int unlogged_tables = 0;
double sample_rate = 0.0;
/*
- * logging steps (seconds between log messages)
- */
-int log_step_seconds = 5;
-
-/*
* tablespace selection
*/
char *tablespace = NULL;
@@ -155,6 +152,7 @@ char *index_tablespace = NULL;
#define naccounts 100000
bool use_log; /* log transaction latencies to
a file */
+bool use_quiet; /* quiet logging onto stderr */
bool is_connect; /* establish connection for
each transaction */
bool is_latencies; /* report per-command latencies */
int main_pid; /* main process id used
in log filename */
@@ -394,6 +392,7 @@ usage(void)
" -v vacuum all four standard tables before
tests\n"
"\nCommon options:\n"
" -d print debugging output\n"
+ " -q quiet logging (a message each 5 seconds)\n"
" -h HOSTNAME database server host or socket directory\n"
" -p PORT database server port number\n"
" -U USERNAME connect as specified database user\n"
@@ -1453,8 +1452,14 @@ init(bool is_no_vacuum)
exit(1);
}
+ /* If we want to stick with the original logging, print a
message each
+ * 100k inserted rows. */
+ if ((! use_quiet) && (j % 100000 == 0))
+ fprintf(stderr, "%d of %d tuples (%d%%) done.\n",
+ j, naccounts * scale,
+ (int) (((int64) j *
100) / (naccounts * scale)));
/* let's not call the timing for each row, but only each 100
rows */
- if (j % 100 == 0 || j == scale * naccounts)
+ else if (use_quiet && (j % 100 == 0))
{
INSTR_TIME_SET_CURRENT(diff);
INSTR_TIME_SUBTRACT(diff, start);
@@ -1462,15 +1467,15 @@ init(bool is_no_vacuum)
elapsed_sec = INSTR_TIME_GET_DOUBLE(diff);
remaining_sec = (scale * naccounts - j) * elapsed_sec /
j;
- /* have we reached the next interval? */
- if (elapsed_sec >= log_interval * log_step_seconds) {
+ /* have we reached the next interval (or end)? */
+ if ((j == scale * naccounts) || (elapsed_sec >=
log_interval * LOG_STEP_SECONDS)) {
fprintf(stderr, "%d of %d tuples (%d%%) done
(elapsed %.2f s, remaining %.2f s).\n",
j, naccounts * scale,
(int) (((int64) j * 100) /
(naccounts * scale)), elapsed_sec, remaining_sec);
/* skip to the next interval */
- log_interval =
(int)ceil(elapsed_sec/log_step_seconds);
+ log_interval =
(int)ceil(elapsed_sec/LOG_STEP_SECONDS);
}
}
@@ -2016,7 +2021,7 @@ main(int argc, char **argv)
state = (CState *) pg_malloc(sizeof(CState));
memset(state, 0, sizeof(CState));
- while ((c = getopt_long(argc, argv,
"ih:nvp:dSNc:j:Crs:t:T:U:lf:D:F:M:", long_options, &optindex)) != -1)
+ while ((c = getopt_long(argc, argv,
"ih:nvp:dqSNc:j:Crs:t:T:U:lf:D:F:M:", long_options, &optindex)) != -1)
{
switch (c)
{
@@ -2124,6 +2129,9 @@ main(int argc, char **argv)
case 'l':
use_log = true;
break;
+ case 'q':
+ use_quiet = true;
+ break;
case 'f':
ttype = 3;
filename = pg_strdup(optarg);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
