Thread: Binary protocol patch v7
Hi, Just to remind you that the I updated the patches for sending receiving columns in v3 binary format are available at: http://mokki.dyndns.org/~mtiihone/postgresql/binarytransfer/ Changes since last update: - fixed handling of empty ResultSet. - fixed ArrayIndexOutOfBounds in CompositeParameterList It passes the current unit test suite and all other applications I have tried it with. I would be interested in any benchmarks. What you would hopefully see is reduced CPU load in both the backend and in the Java application. Also the network traffic should be somewhat reduced. Also, Kris, could you please try to find time to review the patch again and see what still needs to be fixed/clarified/tested before the patch can be accepted. --------------------------- I downloaded BenchmarkSQL 2.3.2 and run it against freshly installed postgres 8.2 backend. Results (best of 10 runs): Jan 7, 2007 CVS: 5891 tpmC - 100% + binaryPatches: 6531 tpmC - 110% The client and server ran on the same host. The Java client used around 25% of available cpu, backend the remaining time, IO was not a bottleneck. Benchmark setup: ---------------- PostgreSQL 8.2.0 with following modified config options: fsync=off, autovacuum=on, stats_row_level=on, redirect_stderr=on, checkpoint_segments=12, max_connections=10, max_prepared_transactions=50, shared_buffers=128MB, standard_conforming_strings=on The BenchmarkSQL was set up with 1 warehouse and 4 terminals. The following URL was used to connect to the backend: jdbc:postgresql://localhost:5432/test?binaryEncoding=true&prepareThreshold=1 Java HotSpot(TM) 64-Bit Server VM (build 1.6.0-b105, mixed mode) Athlon64 2x2GHz, Linux 2.6.19.1 I ran "vacuumdb --all --full" between each benchmark run to get consistent results. -Mikko
On Sun, 7 Jan 2007, Mikko Tiihonen wrote: > Just to remind you that the I updated the patches for sending receiving > columns in v3 binary format are available at: > http://mokki.dyndns.org/~mtiihone/postgresql/binarytransfer/ > > Also, Kris, could you please try to find time to review the patch again > and see what still needs to be fixed/clarified/tested before the patch > can be accepted. > I was actually looking at the v7 patch earlier this week, specifically looking at the conversion code. To try and throw some of the toughest test cases at it I modified the timestamp and time zone test code like k1-timestamp-test.patch. When running the tests with a build.local.properties including binaryTransfer=true and preparethreshold=1 this test fails. So I thought about ways to get more testing without having to rewrite every single regression test. I'm not suggesting committing any of this code, but it's just something I hacked together to try more cases. k2-describe-then-execute.patch issues a describe then an execute for every prepared statement so it won't need to be executed more than once to start using the binary transfer mode if we've set prepareThreshold to 1. This caused only one failure in an updatable resultset test because the results of ResultSet.refreshRow() were in binary while the ResultSet was in text. This failure doesn't concern me because without the hack the PreparedStatement is only used once and won't trigger a binary receive. So then I tried to k3-stmt-as-prepstmt.patch to send all Statement.executeQuery calls using a PreparedStatement to trigger binary receiver. This resulted in a deadlock in the regression tests that I did not have time to investigate. Even though this didn't work, I think it's the way to go to convince people that the new conversion code works exactly as the existing code does. I don't care how ugly it is or how slow it is, if there's a way to get the majority of our existing tests to run through the binary codepath I'd be happy to apply the patch. Kris Jurka
Attachment
On Sun, 7 Jan 2007, Kris Jurka wrote: > On Sun, 7 Jan 2007, Mikko Tiihonen wrote: > >> Just to remind you that the I updated the patches for sending receiving >> columns in v3 binary format are available at: >> http://mokki.dyndns.org/~mtiihone/postgresql/binarytransfer/ >> > > I was actually looking at the v7 patch earlier this week, specifically > looking at the conversion code. To try and throw some of the toughest test > cases at it I modified the timestamp and time zone test code like > k1-timestamp-test.patch. When running the tests with a > build.local.properties including binaryTransfer=true and preparethreshold=1 > this test fails. > > So I thought about ways to get more testing without having to rewrite every > single regression test. I'm not suggesting committing any of this code, but > it's just something I hacked together to try more cases. > With the attached patch and binaryTransfer=true and preparethreshold=1 in build.local.properties I was able to run the regression tests completely and got a number of failures. 1) DatabaseMetaData creates ResultSets by sometimes pulling raw bytes out of another ResultSet, so it needs to also pass the binary flag to the new ResultSet. 2) A number of failures in date/time tests. It seems to have some problems with time zones. Also, conversion to strings should result in something identical to the text format. 3) There's also a couple of fake failures due to the methodology used. testUpdateStreams and testHoldableResultSet fail only because of the changes I made. Kris Jurka