Removing useless \. at the end of copy in pgbench
While populating pgbench_account table by using COPY FROM STDIN,
pgbench sends out "\." at the end of the copy data. However this is
only necessary in the version 2 of frontend/backend protocol (i.e. the
version 3 protocol does not need it). I think we can safely remove the
code to save a few CPU cycle since we only support back to PostgreSQL
9.3 and the version 3 protocol has been supported since 7.4.
If we want to support pre 7.4 backend (which only supports the version
2 protocol), we could test the current protocol version and decide
whether we should send out "\." or not, but I doubt it's necessary.
Comments?
(patch to remove the unneccessary code attached)
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
Attachments:
pgbench_remove_unnecessary_code.difftext/x-patch; charset=us-asciiDownload
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 41b756c..54b7182 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3686,11 +3686,6 @@ initGenerateData(PGconn *con)
}
}
- if (PQputline(con, "\\.\n"))
- {
- fprintf(stderr, "very last PQputline failed\n");
- exit(1);
- }
if (PQendcopy(con))
{
fprintf(stderr, "PQendcopy failed\n");
Tatsuo Ishii <ishii@sraoss.co.jp> writes:
While populating pgbench_account table by using COPY FROM STDIN,
pgbench sends out "\." at the end of the copy data. However this is
only necessary in the version 2 of frontend/backend protocol (i.e. the
version 3 protocol does not need it). I think we can safely remove the
code to save a few CPU cycle since we only support back to PostgreSQL
9.3 and the version 3 protocol has been supported since 7.4.
What exactly is the benefit of breaking compatibility with old servers?
Saving a few cycles during initial table population seems like it could
not be of interest to anyone.
regards, tom lane
Tatsuo Ishii <ishii@sraoss.co.jp> writes:
While populating pgbench_account table by using COPY FROM STDIN,
pgbench sends out "\." at the end of the copy data. However this is
only necessary in the version 2 of frontend/backend protocol (i.e. the
version 3 protocol does not need it). I think we can safely remove the
code to save a few CPU cycle since we only support back to PostgreSQL
9.3 and the version 3 protocol has been supported since 7.4.What exactly is the benefit of breaking compatibility with old servers?
Saving a few cycles during initial table population seems like it could
not be of interest to anyone.
Then we can do protocol version checking to not break backward
compatibility. I believe this is what psql does. So this will bring a
benefit to make our clients more consistent.
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
Hello Tatsuo-san,
If we want to support pre 7.4 backend (which only supports the version
2 protocol), we could test the current protocol version and decide
whether we should send out "\." or not, but I doubt it's necessary.Comments?
Code tested, ok.
I agree that compatibility with protocol v2 (pg 7.4) is not an issue.
As Tom, I would not have bothered, though, however once it is there why
not?
Should the doc state that pgbench compatibility is ok from pg 8.0?
--
Fabien.
Fabien COELHO <coelho@cri.ensmp.fr> writes:
I agree that compatibility with protocol v2 (pg 7.4) is not an issue.
As Tom, I would not have bothered, though, however once it is there why
not?
As far as the patch goes --- I think that someday in the not too distant
feature we ought to rip out libpq's support for v2 protocol, which *would*
amount to a meaningful savings in code and maintenance effort. And then
we could look around for v2-related code in our clients and get rid of
that. But I'm not in favor of doing little bits of the latter before
we've done the former.
Should the doc state that pgbench compatibility is ok from pg 8.0?
It'd definitely be a good idea to have a stated policy about what
the minimum supported server version is. psql, for instance, generally
works back to at least 7.0 (the oldest server version I've got in
captivity at the moment), but its describe.c functionality only goes
back to 7.4, as stated in the header for that file. I don't know
what a good cutoff for pgbench is, but we should figure that out
and document it.
regards, tom lane
Hello Tom,
I agree that compatibility with protocol v2 (pg 7.4) is not an issue.
As Tom, I would not have bothered, though, however once it is there why
not?As far as the patch goes --- I think that someday in the not too distant
feature we ought to rip out libpq's support for v2 protocol, which *would*
amount to a meaningful savings in code and maintenance effort. And then
we could look around for v2-related code in our clients and get rid of
that. But I'm not in favor of doing little bits of the latter before
we've done the former.
I'm fine with dropping support when it is done at libpq level, for
consistency.
Should the doc state that pgbench compatibility is ok from pg 8.0?
It'd definitely be a good idea to have a stated policy about what
the minimum supported server version is. psql, for instance, generally
works back to at least 7.0 (the oldest server version I've got in
captivity at the moment), but its describe.c functionality only goes
back to 7.4, as stated in the header for that file. I don't know
what a good cutoff for pgbench is, but we should figure that out
and document it.
Hmmm. There are several levels: protocol/libpq, and what you can put in a
pgbench initialization or script.
For the first part, it does not seem to use anything untoward, so probably
it should work with pretty old, although I have no simple way to test
much. It does not seem to use anything which did not exist in 2004
according to "libpq/exports.txt".
For the second one, UNLOGGED was introduced in 9.1, so running -i
--unlogged-tables will fail on 9.0, but very probably works without the
option. Similarly, TABLESPACE need 8.0, FIL.LFACTOR needs 8.2 and is
always used (FILLFACTOR=100). "DROP TABLE IF EXISTS" requires 8.2 as well.
So 8.2 is probably currently a minimal version for "pgbench -i"... thus
Tatsuo's patch is not an issue.
Now I could not even "initdb" a freshly compiled 8.[23] versions on my
ubuntu 18.4 laptop:-( When compiling 9.0 "*** The installed version of
Flex, /usr/bin/flex, is too old to use with PostgreSQL. *** Flex version
2.5.31 or later is required, but this is flex 2.6.4.". Ah ah, I like
autoconf soooooo much:-) I got tired and skipped to 9.3 and can confirm
that pgbench 10.4 works a postgres 9.3 server, for instance.
Basically Pgbench is really compatible with a given server version
depending on the options used, so asserting a compatibility level requires
some careful phrasing. Maybe something like that could be added to the
documentation:
"""
Pgbench is expected to work with all PostgreSQL supported versions at
the time it is released. Some options may work only with newer servers. It
may work with older version down to 8.2.
"""
See attached patch.
--
Fabien.
Attachments:
pgbench-compat-1.patchtext/plain; name=pgbench-compat-1.patchDownload
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 88cf8b3933..6fc9d53828 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -128,6 +128,13 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
and <option>-f</option> (specify a custom script file).
See below for a full list.
</para>
+
+ <para>
+ <application>Pgbench</application> is expected to work with all PostgreSQL
+ supported versions at the time it is released.
+ Some options may require newer servers.
+ It may work with older version, down to 8.2.
+ </para>
</refsect1>
<refsect1>
On 29/07/2018 01:59, Fabien COELHO wrote:
"""
Pgbench is expected to work with all PostgreSQL supported versions at
the time it is released. Some options may work only with newer servers. It
may work with older version down to 8.2.
"""
It is generally expected (nowadays) that client programs work
independent of the server version, unless restrictions are specifically
documented (e.g., pg_dump) or there is some obvious feature dependency
(e.g., unlogged tables). So the above paragraph does not add any useful
information. Also, I don't find any reason why 8.2 is the cutoff, and
saying that it may work down to 8.2 (implying that it may not) is
content-free.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hello Peter,
"""
Pgbench is expected to work with all PostgreSQL supported versions at
the time it is released. Some options may work only with newer servers. It
may work with older version down to 8.2.
"""It is generally expected (nowadays) that client programs work
independent of the server version, unless restrictions are specifically
documented (e.g., pg_dump) or there is some obvious feature dependency
(e.g., unlogged tables).
Yep, that is somehow what I said in the mail.
So the above paragraph does not add any useful information.
It tells that it will not work from 8.1 and below.
Also, I don't find any reason why 8.2 is the cutoff, and saying that it
may work down to 8.2 (implying that it may not) is content-free.
The "may" is because I could *not* test: I could not run a 8.2 on my
laptop, "initdb" fails on:
creating template1 database in <...>/src/test/regress/./tmp_check/data/base/1 ... ok
initializing pg_authid ... FATAL: wrong number of index expressions
STATEMENT: CREATE TRIGGER pg_sync_pg_database AFTER INSERT OR UPDATE OR DELETE ON pg_database FOR EACH STATEMENT EXECUTE PROCEDURE flatfile_update_trigger();
So I did not feel confident in saying that it would work for 8.2. That is
just me being precise:-)
As explained in the previous mail, pgbench "CREATE TABLE" always uses
"FILLFACTOR" which was introduced in 8.2, so it is sure to fail before
that.
What about:
"""
Pgbench requires a PostgreSQL version 8.2 or above server.
"""
Some information is provided...
I understood that Tom found that an explicit compatibility note would be
welcome, so I provided one. I'm also fine with saying nothing.
--
Fabien.
On 08/29/2018 10:35 AM, Fabien COELHO wrote:
The "may" is because I could *not* test: I could not run a 8.2 on my
laptop, "initdb" fails on:creating template1 database in
<...>/src/test/regress/./tmp_check/data/base/1 ... ok
initializing pg_authid ... FATAL: wrong number of index expressions
Ran into that myself just recently while confirming that PL/Java
still works back to 8.2 as claimed.
Rebuild postgres with -fno-aggressive-loop-optimizations added
to CFLAGS. :)
-Chap
On 2018-Aug-29, Fabien COELHO wrote:
The "may" is because I could *not* test: I could not run a 8.2 on my laptop,
"initdb" fails on:creating template1 database in <...>/src/test/regress/./tmp_check/data/base/1 ... ok
initializing pg_authid ... FATAL: wrong number of index expressions
STATEMENT: CREATE TRIGGER pg_sync_pg_database AFTER INSERT OR UPDATE OR DELETE ON pg_database FOR EACH STATEMENT EXECUTE PROCEDURE flatfile_update_trigger();
Yeah, that's a problem when compiling 8.2 and 8.3 with newer gcc. If
you grab the copy from git, it has only one commit after the 8.2.23 tag
and that one fixes this problem.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Fabien COELHO <coelho@cri.ensmp.fr> writes:
Also, I don't find any reason why 8.2 is the cutoff, and saying that it
may work down to 8.2 (implying that it may not) is content-free.
The "may" is because I could *not* test:
Works for me with 8.2. Earlier branches fail immediately:
dropping old tables...
ERROR: syntax error at or near "exists"
LINE 1: drop table if exists pgbench_accounts, pgbench_branches, pgb...
^
We could probably fix that if anyone cared, but I doubt anyone does.
regards, tom lane
The "may" is because I could *not* test:
Works for me with 8.2.
Thanks for the confirmation.
Earlier branches fail immediately: dropping old tables... ERROR:
syntax error at or near "exists" LINE 1: drop table if exists
pgbench_accounts, pgbench_branches, pgb...
^
Ok, even before the create table with a fillfactor.
We could probably fix that if anyone cared, but I doubt anyone does.
Indeed, if people needed using a new pgbench against a that old server,
they would have complained.
The point was just to document the current status of pgbench
compatibility, and 8.2 onward is the answer.
--
Fabien.
Hello,
What about:
"""
Pgbench requires a PostgreSQL version 8.2 or above server.
"""Some information is provided...
I understood that Tom found that an explicit compatibility note would be
welcome, so I provided one. I'm also fine with saying nothing.
Here is a patch with the following accurate information:
"""
In order to run, pgbench requires a PostgreSQL server version 8.2 or
above.
"""
8.2 has been tested by Tom, and is required because of DROP TABLE IF
EXISTS & CREATE TABLE ... FILLFACTOR, which I pointed out in a mail
upthread.
Now if the information is not added to the doc, this is also fine with me.
--
Fabien.
On 2018-08-29 21:42:42 +0200, Fabien COELHO wrote:
Hello,
What about:
"""
Pgbench requires a PostgreSQL version 8.2 or above server.
"""Some information is provided...
I understood that Tom found that an explicit compatibility note would be
welcome, so I provided one. I'm also fine with saying nothing.Here is a patch with the following accurate information:
"""
In order to run, pgbench requires a PostgreSQL server version 8.2 or above.
"""8.2 has been tested by Tom, and is required because of DROP TABLE IF EXISTS
& CREATE TABLE ... FILLFACTOR, which I pointed out in a mail upthread.Now if the information is not added to the doc, this is also fine with me.
I'd vote for not adding it. It seems almost guaranteed to get out of
date without anybody noticing so. Maybe that's overly pragmatic, but I
really can't see the harm of not documenting which precise ancient
version pgbench doesn't support anymore...
Greetings,
Andres Freund
On 29/08/2018 21:48, Andres Freund wrote:
I'd vote for not adding it. It seems almost guaranteed to get out of
date without anybody noticing so. Maybe that's overly pragmatic, but I
really can't see the harm of not documenting which precise ancient
version pgbench doesn't support anymore...
Yeah, chances are that someone is going to make a change that will
require for example 8.4, and nobody would update this because the
differences between 8.2 and 8.4 are long forgotten.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 30/08/2018 08:39, Peter Eisentraut wrote:
On 29/08/2018 21:48, Andres Freund wrote:
I'd vote for not adding it. It seems almost guaranteed to get out of
date without anybody noticing so. Maybe that's overly pragmatic, but I
really can't see the harm of not documenting which precise ancient
version pgbench doesn't support anymore...Yeah, chances are that someone is going to make a change that will
require for example 8.4, and nobody would update this because the
differences between 8.2 and 8.4 are long forgotten.
It seems there is more enthusiasm on the side of not documenting it, so
I'll close this commit fest entry.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Yeah, chances are that someone is going to make a change that will
require for example 8.4, and nobody would update this because the
differences between 8.2 and 8.4 are long forgotten.It seems there is more enthusiasm on the side of not documenting it, so
I'll close this commit fest entry.
Fine with me. We spent time collecting this information, though.
--
Fabien.