Thread: 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 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.
Attachment
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 EXECUTEPROCEDURE 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 STATEMENTEXECUTE 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.