Re: pgbench -f and vacuum

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Tatsuo Ishii <ishii(at)postgresql(dot)org>, Tomáš Vondra <tv(at)fuzzy(dot)cz>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pgbench -f and vacuum
Date: 2015-04-30 20:27:08
Message-ID: CA+TgmoY5zWia8p3LE64bT_BCQH=gGF07Wi2q-zfq8wg6OQgG6g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Apr 30, 2015 at 4:17 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Wed, Feb 11, 2015 at 2:00 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
>>> But as far as what has been discussed on the central topic of this thread, I
>>> think that doing the vacuum and making the failure for non-existent tables
>>> be non-fatal when -f is provided would be an improvement. Or maybe just
>>> making it non-fatal at all times--if the table is needed and not present,
>>> the session will fail quite soon anyway. I don't see the other changes as
>>> being improvements. I would rather just learn to add the -n when I use -f
>>> and don't have the default tables in place, than have to learn new methods
>>> for saying "no really, I left -n off on purpose" when I have a custom file
>>> which does use the default tables and I want them vacuumed.
>
>> So, discussion seems to have died off here. I think what Jeff is
>> proposing here is a reasonable compromise. Patch for that attached.
>
> +1 as to the basic behavior, but I'm not convinced that this is
> user-friendly reporting:
>
> + if (PQresultStatus(res) != PGRES_COMMAND_OK)
> + fprintf(stderr, "%s", PQerrorMessage(con));
>
> I would be a bit surprised to see pgbench report an ERROR and then
> continue on anyway; I might think that was a bug, even. I am not
> sure exactly what it should print instead though. Some perhaps viable
> proposals:
>
> * don't print anything at all, just chug along.
>
> * do something like
> fprintf(stderr, "Ignoring: %s", PQerrorMessage(con));
>
> * add something like "(Ignoring this error and continuing anyway)"
> on a line after the error message.
>
> (I realize this takes us right back into the bikeshedding game, but
> I do think that what's displayed is important.)

I tried it out before sending the patch and it's really not that bad.
It's says:

starting vacuum.... ERROR: blah
ERROR: blah
ERROR: blah
done

And then continues on. Sure, that's not the greatest error reporting
output ever, but what do you expect from pgbench? I think it's clear
enough what's going on there. The messages appear in quick
succession, because it doesn't take very long to notice that the table
isn't there, so it's not like you are sitting there going "wait,
what?".

If we're going to add something, I like your second suggestion
"(ignoring this error and continuing anyway)" more than the first one.
Putting "ignoring:" before the thing you plan to ignore will be
confusing, I think.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2015-04-30 20:43:13 Re: [PATCH] HINT: pg_hba.conf changed since last config reload
Previous Message Tom Lane 2015-04-30 20:17:33 Re: pgbench -f and vacuum