Re: About binaryTransfer.

From: Dave Cramer <pg(at)fastcrypt(dot)com>
To: Tomonari Katsumata <t(dot)katsumata1122(at)gmail(dot)com>
Cc: Tomonari Katsumata <katsumata(dot)tomonari(at)po(dot)ntts(dot)co(dot)jp>, Mikko Tiihonen <Mikko(dot)Tiihonen(at)nitorcreations(dot)com>, "pgsql-jdbc(at)postgresql(dot)org" <pgsql-jdbc(at)postgresql(dot)org>
Subject: Re: About binaryTransfer.
Date: 2014-04-23 14:56:00
Message-ID: CADK3HHLVEB00xK7Efaj0r3Z3zYwgAuO-=neG89hRkqZEoS6Kww@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-jdbc

Tomonari,

I committed the code to REL9_3_STABLE. As the "new" functionality is
invoked by setting prepareThreshold to -1 it should not be intrusive.

Can you please test the current REL9_3_STABLE code ?

Dave Cramer

dave.cramer(at)credativ(dot)ca
http://www.credativ.ca

On 21 April 2014 06:31, Dave Cramer <pg(at)fastcrypt(dot)com> wrote:

> Tomonari,
>
> I'm hesitant to apply the first patch, as the static variable means every
> statement will have the same behaviour. Think about this. What if another
> statement sets it to false after you have set it to true ?
>
> You have to keep in mind there is only copy of the driver inside a JVM.
>
>
>
>
>
> Dave Cramer
>
> dave.cramer(at)credativ(dot)ca
> http://www.credativ.ca
>
>
> On 17 April 2014 11:29, Tomonari Katsumata <t(dot)katsumata1122(at)gmail(dot)com>wrote:
>
>> Hi Dave,
>>
>> Thank you for check.
>>
>> > We still have to deal with the static variable ForceBinaryTransfer. It
>> has
>> > to be per statement. The static variable will change the behaviour of
>> all
>> > statements.
>> >
>> > It should also be named forceBinaryTransfer.
>> >
>> I've misunderstood what you say.
>> Now I could understand it.
>>
>> I changed public variable "forceBinaryTransfers" to
>> static variable "ForceBinaryTransfers" .
>> Please check fix_ForceBinaryTranfers-part2.patch.
>> This patch is for current master(*1).
>> (*1)0eadcb873c68715cc3d0e9478c13ce3e38b798ae
>>
>> Because this patch is for new feature, I think
>> it is not necessary to be applyed to REL9_3_STABLE.
>> But I want to apply a patch which I sent in last mail(*2)
>> to REL9_3_STABLE for resolving performance problem.
>> (*2) sending again. see attached
>> fix_ForceBinaryTranfers-for-93stable.patch
>>
>> Still am I missing something?
>>
>> regards,
>> -----------------
>> Tomonari Katsumata
>>
>> >
>> > Dave Cramer
>> >
>> > dave.cramer(at)credativ(dot)ca
>> > http://www.credativ.ca
>> >
>> >
>> > On 17 April 2014 05:15, Tomonari Katsumata
>> > <katsumata(dot)tomonari(at)po(dot)ntts(dot)co(dot)jp
>> >> wrote:
>> >
>> >> Hi Dave, Mikko,
>> >>
>> >> Sorry for slow response.
>> >>
>> >> Mikko, thanks for cleaning things up.
>> >> My thought is all that Mikko says.
>> >>
>> >>
>> >> >> So the desired behaviour you want is what? You want the first query
>> >> >> to
>> >> >> be able to use binary, or not ?
>> >> >>
>> >> I don't need using binary transfer for the first query.
>> >> But someone need and added this feature, I've thought to
>> >> remain this feature.
>> >>
>> >>
>> >> > I think the only problem with Tomonari's patch is some conventions
>> such
>> >> as
>> >> > Capitalization of the first letter, and the use of a static field in
>> >> > the
>> >> > statement as opposed to a per statement setting
>> >> >
>> >> I've changed the behavior drastically for including
>> >> both "first query binaryTransfer" and "No extra round-trips".
>> >> As Dave says, this may cause some trouble...
>> >>
>> >> Then, I create a new patch for REL9_3_STABLE.
>> >> Please see attached patch.
>> >> This patch will just reduce extra round-trips.
>> >> A extra round-trip would occur only once, but this can not be helped.
>> >>
>> >> regards,
>> >> ----------------
>> >> Tomonari Katsumata
>> >>
>> >>
>> >> (2014/04/15 22:12), Dave Cramer wrote:
>> >> > Mikko,
>> >> >
>> >> > Thanks for clearing that up.
>> >> >
>> >> > I think the only problem with Tomonari's patch is some conventions
>> such
>> >> as
>> >> > Capitalization of the first letter, and the use of a static field in
>> >> > the
>> >> > statement as opposed to a per statement setting
>> >> >
>> >> >
>> >> >
>> >> > Dave Cramer
>> >> >
>> >> > dave.cramer(at)credativ(dot)ca
>> >> > http://www.credativ.ca
>> >> >
>> >> >
>> >> > On 15 April 2014 08:53, Mikko Tiihonen <Mikko.Tiihonen@
>> >> nitorcreations.com>wrote:
>> >> >
>> >> >> I think the changes in the driver go like this:
>> >> >>
>> >> >>
>> >> >> 1) initial binary transfer implementation
>> >> >>
>> >> >> - first n queries use text and after that binary (no extra
>> >> >> round-trips)
>> >> >>
>> >> >> - there was a debug flag to enable binary transfers for first query
>> >> (with
>> >> >> the cost of extra round-trip)
>> >> >>
>> >> >>
>> >> >> 2) someone wanted the binary transfers for one-shot operations too
>> >> >>
>> >> >> - but modified the code so that the extra driver-server round trip
>> >> >> cost
>> >> is
>> >> >> now on _every_ execution of prepared statement
>> >> >>
>> >> >>
>> >> >> 3) tomonari created a patch the fixes the extra round-trip but
>> still
>> >> >> allows binary transfers for first query - all configurable
>> >> >>
>> >> >>
>> >> >> Now the request is to either revert 2) change or apply 3) change.
>> The
>> >> >> extra round-trip between each execution is really a killer for many
>> >> >> installations.
>> >> >>
>> >> >>
>> >> >> -Mikko
>> >> >> ------------------------------
>> >> >> *From:* davecramer(at)gmail(dot)com <davecramer(at)gmail(dot)com> on behalf of
>> Dave
>> >> >> Cramer <pg(at)fastcrypt(dot)com>
>> >> >> *Sent:* 15 April 2014 15:34
>> >> >> *To:* Tomonari Katsumata
>> >> >> *Cc:* Tomonari Katsumata; Mikko Tiihonen; pgsql-jdbc(at)postgresql(dot)org
>> >> >> *Subject:* Re: [JDBC] About binaryTransfer.
>> >>
>> >> >>
>> >> >> Tomonari,
>> >> >>
>> >> >> So the desired behaviour you want is what? You want the first query
>> >> >> to
>> >> >> be able to use binary, or not ?
>> >> >>
>> >> >> Dave
>> >> >>
>> >> >> Dave Cramer
>> >> >>
>> >> >> dave.cramer(at)credativ(dot)ca
>> >> >> http://www.credativ.ca
>> >> >>
>> >> >>
>> >> >> On 15 April 2014 04:20, Tomonari Katsumata <
>> >> >> katsumata(dot)tomonari(at)po(dot)ntts(dot)co(dot)jp> wrote:
>> >> >>
>> >> >>> Hi Dave,
>> >> >>>
>> >> >>>
>> >> >>>> I pulled this into master.
>> >> >>>>
>> >> >>> Thanks for merging my fix against master!
>> >> >>>
>> >> >>>
>> >> >>>> I'm not quite sure I want this back patched into
>> >> >>>> 9.3 though. This is new behaviour.
>> >> >>>>
>> >> >>> I think the original change(*) was done for rare case.
>> >> >>> But this change would cause a performance issue
>> >> >>> in many cases like me.
>> >> >>>
>> >> >>> So I want this fix into 9.3.
>> >> >>> If we cannot do so, we should revert
>> >> >>> the original change(*) at least.
>> >> >>> (*)https://github.com/pgjdbc/pgjdbc/commit/
>> >> dbf76c2d662896c5703cf20d7362e1
>> >> >>> d061e1e43f
>> >> >>>
>> >> >>> regards,
>> >> >>> ---------------
>> >> >>> Tomonari Katsumata
>> >> >>>
>> >> >>>
>> >> >>>>
>> >> >>>>
>> >> >>>> Dave Cramer
>> >> >>>>
>> >> >>>> dave.cramer(at)credativ(dot)ca
>> >> >>>> http://www.credativ.ca
>> >> >>>>
>> >> >>>>
>> >> >>>> On 3 April 2014 04:37, Tomonari Katsumata
>> >> >>>> <katsumata(dot)tomonari(at)po(dot)ntts(dot)co(dot)jp>wrote:
>> >> >>>>
>> >> >>>>> Hi Dave,
>> >> >>>>>
>> >> >>>>> Did you check my pull-request?
>> >> >>>>> https://github.com/pgjdbc/pgjdbc/pull/128
>> >> >>>>>
>> >> >>>>> I don't want to use 9.2-1004 Driver, because it has some bugs
>> >> >>>>> about setQueryTimeout fixed by Heikki(*).
>> >> >>>>> (*)
>> >> >>>>> http://www.postgresql.org/message-id/52A0D28A.7040902@vmware.com
>> >> >>>>>
>> >> >>>>> So I need the PostgreSQL Driver 9.3-1101 have good performance
>> >> >>>>> like 9.2-1004 as soon as possible.
>> >> >>>>>
>> >> >>>>> Could you check it please?
>> >> >>>>> If I'm missing something, please tell me it!
>> >> >>>>>
>> >> >>>>> regards,
>> >> >>>>> ------------------------
>> >> >>>>> Tomonari Katsumata
>> >> >>>>>
>> >> >>>>>
>> >> >>>>> (2014/03/06 18:13), Tomonari Katsumata wrote:
>> >> >>>>>> Hi Dave,
>> >> >>>>>>
>> >> >>>>>> I've misunderstood the behavior of this problem.
>> >> >>>>>> The main problem is that the Describe message is send/receive
>> >> >>>>>> repeatedly, if user don't want to do so.
>> >> >>>>>>
>> >> >>>>>> The pull-request I've sent before(#124) didn't solve the
>> problem.
>> >> >>>>>>
>> >> >>>>>> Now, I fixed it and sent a new pull-request.
>> >> >>>>>> https://github.com/pgjdbc/pgjdbc/pull/128
>> >> >>>>>>
>> >> >>>>>> Please check it!
>> >> >>>>>>
>> >> >>>>>> regards,
>> >> >>>>>> ----------------------
>> >> >>>>>> Tomonari Katsumata
>> >> >>>>>
>> >> >>>>>>
>> >> >>>>>> (2014/02/23 9:34), Tomonari Katsumata wrote:
>> >> >>>>>>> Hi Dave,
>> >> >>>>>>> I sent a Pull Request about this.
>> >> >>>>>>> https://github.com/pgjdbc/pgjdbc/pull/124
>> >> >>>>>>>
>> >> >>>>>>> regards,
>> >> >>>>>>> -------------------
>> >> >>>>>>> Tomonari Katsumata
>> >> >>>>>>>
>> >> >>>>>>>
>> >> >>>>>>> 2014-02-21 21:09 GMT+09:00 Dave Cramer <pg(at)fastcrypt(dot)com>:
>> >> >>>>>>>
>> >> >>>>>>>> Please submit a Pull Request
>> >> >>>>>>>>
>> >> >>>>>>>> Dave Cramer
>> >> >>>>>>>>
>> >> >>>>>>>> dave.cramer(at)credativ(dot)ca
>> >> >>>>>>>> http://www.credativ.ca
>> >> >>>>>>>>
>> >> >>>>>>>>
>> >> >>>>>>>> On Fri, Feb 21, 2014 at 3:57 AM, Tomonari Katsumata <
>> >> >>>>>>>> katsumata(dot)tomonari(at)po(dot)ntts(dot)co(dot)jp> wrote:
>> >> >>>>>>>>
>> >> >>>>>>>>> Hi Mikko,
>> >> >>>>>>>>>
>> >> >>>>>>>>> Thank you for the explanation.
>> >> >>>>>>>>>
>> >> >>>>>>>>> I agree with your proposal adding prepareThreshold=-1.
>> >> >>>>>>>>>
>> >> >>>>>>>>> If there are no objection, I'll do for it!
>> >> >>>>>>>>>
>> >> >>>>>>>>> regards,
>> >> >>>>>>>>> ----------------
>> >> >>>>>>>>> Tomonari Katsumata
>> >> >>>>>>>>>
>> >> >>>>>>>>>
>> >> >>>>>>>>> (2014/02/21 16:50), Mikko Tiihonen wrote:
>> >> >>>>>>>>>> Before the patch the functionality was (if
>> >> >>>>>>>>>> binaryTransfer=true):
>> >> >>>>>>>>>> - prepared statements after prepareThreshold were done in
>> >> >>>>>>>>>> binary
>> >> >>>>> mode
>> >> >>>>>>>>>> - forceBinary=true could be enabled to force all statements
>> >> >>>>> (prepared +
>> >> >>>>>>>>> one-shot) to be executed in binary mode (at cost of extra
>> >> >>> round-trip)
>> >> >>>>>>>>>>
>> >> >>>>>>>>>> After the patch in question (if binaryTransfer=true):
>> >> >>>>>>>>>> - All prepared statements have extra round-trip before on
>> >> >>>>>>>>>> first
>> >> >>> use
>> >> >>>>> and
>> >> >>>>>>>>> are immediately in binary mode
>> >> >>>>>>>>>> - forceBinary=true can be enabled to force also one-shot
>> >> >>> statements
>> >> >>>>> to
>> >> >>>>>>>>> be executed in binary mode (at cost of extra round-trip)
>> >> >>>>>>>>>>
>> >> >>>>>>>>>> Since there are users that use prepared statements in
>> one-shot
>> >> way
>> >> >>>>>>>>> (prepare+execute+discard) the patch adds a mandatory extra
>> >> >>>>> round-trip for
>> >> >>>>>>>>> them.
>> >> >>>>>>>>>>
>> >> >>>>>>>>>> As a side note: the forceBinary is meant only as a debug
>> flag
>> >> >>> (used
>> >> >>>>> for
>> >> >>>>>>>>> example in pgjdbc tests), not for production use.
>> >> >>>>>>>>>>
>> >> >>>>>>>>>> So the only thing the before-state could not do was to use
>> >> binary
>> >> >>>>>>>>> transfers for the first prepared statement execution. This is
>> >> >>> because
>> >> >>>>>>>>> setting prepareThreshold=0 disables the prepare instead of
>> >> >>> preparing
>> >> >>>>> before
>> >> >>>>>>>>> first use.
>> >> >>>>>>>>>>
>> >> >>>>>>>>>> I propose we revert that patch and instead add support for
>> >> >>>>>>>>> prepareThreshold=-1 which would force prepare+describe to be
>> >> >>>>>>>>> done
>> >> >>>>> even for
>> >> >>>>>>>>> the first execution. That would allow users to keep
>> controlling
>> >> the
>> >> >>>>>>>>> behavior instead of forcing binary transfers immediately?
>> >> >>>>>>>>>> Alternatively we can separate the binary transfer logic from
>> >> >>>>> statement
>> >> >>>>>>>>> prepare threshold and add a separate binaryThreshold.
>> >> >>>>>>>>>>
>> >> >>>>>>>>>> -Mikko
>> >> >>>>>>>>>> ________________________________________
>> >> >>>>>>>>>> From: pgsql-jdbc-owner(at)postgresql(dot)org<pgsql-jdbc-owner@
>> >> postgresql.
>> >> >>>>> org>
>> >> >>>>>>>>> on behalf of Tomonari Katsumata <katsumata(dot)tomonari(at)po(dot)ntts(dot)
>> >> co.jp>
>> >> >>>>>>>>>> Sent: 21 February 2014 08:40
>> >> >>>>>>>>>> To: pgsql-jdbc(at)postgresql(dot)org
>> >> >>>>>>>>>> Subject: [JDBC] About binaryTransfer.
>> >> >>>>>>>>>>
>> >> >>>>>>>>>> Hi,
>> >> >>>>>>>>>>
>> >> >>>>>>>>>> I have a peformance trouble with 9.3-1100 driver.
>> >> >>>>>>>>>> Running same application(*) with 9.2-1004 and 9.3-1100,
>> >> >>>>>>>>>> It does another behavior.
>> >> >>>>>>>>>> (*) Retrieving 9990 rows with preparedStatement.
>> >> >>>>>>>>>>
>> >> >>>>>>>>>> 9.2-1004:
>> >> >>>>>>>>>> Always flags = 16.
>> >> >>>>>>>>>> ----
>> >> >>>>>>>>>> 14:09:55.730 (1) simple execute,
>> >> >>>>>>>>>> handler=org.postgresql.jdbc2.AbstractJdbc2Statement$
>> >> >>>>>>>>> StatementResultHandler(at)8232a5d,
>> >> >>>>>>>>>> maxRows=0, fetchSize=0, flags=16
>> >> >>>>>>>>>> 14:09:55.878 (1) simple execute,
>> >> >>>>>>>>>> handler=org.postgresql.jdbc2.AbstractJdbc2Statement$
>> >> >>>>>>>>> StatementResultHandler(at)34e671de,
>> >> >>>>>>>>>> maxRows=0, fetchSize=0, flags=16
>> >> >>>>>>>>>> ----
>> >> >>>>>>>>>>
>> >> >>>>>>>>>> 9.3-1100
>> >> >>>>>>>>>> Repeatedly flags = 48 and 16.
>> >> >>>>>>>>>> The count of "flags=16" is same with 9.2-1004, so
>> >> >>>>>>>>>> "flags=48" is extra executing.
>> >> >>>>>>>>>> ----
>> >> >>>>>>>>>> 14:20:34.991 (1) simple execute,
>> >> >>>>>>>>>> handler=org.postgresql.jdbc2.AbstractJdbc2Statement$
>> >> >>>>>>>>> StatementResultHandler(at)19cdbc83,
>> >> >>>>>>>>>> maxRows=0, fetchSize=0, flags=48
>> >> >>>>>>>>>> 14:20:34.992 (1) simple execute,
>> >> >>>>>>>>>> handler=org.postgresql.jdbc2.AbstractJdbc2Statement$
>> >> >>>>>>>>> StatementResultHandler(at)304b0cbc,
>> >> >>>>>>>>>> maxRows=0, fetchSize=0, flags=16
>> >> >>>>>>>>>> ----
>> >> >>>>>>>>>>
>> >> >>>>>>>>>> This change has caused by below commit.
>> >> >>>>>>>>>> https://github.com/pgjdbc/pgjdbc/commit/
>> >> >>>>> dbf76c2d662896c5703cf20d7362e1
>> >> >>>>>>>>> d061e1e43f
>> >> >>>>>>>>>>
>> >> >>>>>>>>>> It seems that binarytransfer mode is good at dealing with
>> >> >>>>>>>>>> big-data(many columns?many rows?), but some packets are
>> >> >>>>>>>>>> sent/received for this function, right?
>> >> >>>>>>>>>>
>> >> >>>>>>>>>> I want to make 9.3-1100 driver do old behavior like
>> 9.2-1004.
>> >> >>>>>>>>>> What can I do ?
>> >> >>>>>>>>>>
>> >> >>>>>>>>>> regards,
>> >> >>>>>>>>>> ----------------
>> >> >>>>>>>>>> Tomonari Katsumata
>> >> >>>>>>>>>>
>> >> >>>>>>>>>>
>> >> >>>>>>>>>>
>> >> >>>>>>>>>>
>> >> >>>>>>>>>> --
>> >> >>>>>>>>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc(at)postgresql(dot)org
>> )
>> >> >>>>>>>>>> To make changes to your subscription:
>> >> >>>>>>>>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>> >> >>>>>>>>>>
>> >> >>>>>>>>>>
>> >> >>>>>>>>>
>> >> >>>>>>>>>
>> >> >>>>>>>>>
>> >> >>>>>>>>>
>> >> >>>>>>>>> --
>> >> >>>>>>>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc(at)postgresql(dot)org)
>> >> >>>>>>>>> To make changes to your subscription:
>> >> >>>>>>>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>> >> >>>>>>>>>
>> >> >>>>>>>>
>> >> >>>>>>>>
>> >> >>>>>>>
>> >> >>>>>>
>> >> >>>>>>
>> >> >>>>>>
>> >> >>>>>
>> >> >>>>>
>> >> >>>>>
>> >> >>>>
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>
>> >> >
>> >>
>> >>
>> >>
>> >
>>
>
>

In response to

Responses

Browse pgsql-jdbc by date

  From Date Subject
Next Message Tomonari Katsumata 2014-04-24 08:23:11 Re: About binaryTransfer.
Previous Message Tom Dunstan 2014-04-23 02:36:22 Re: Postgres jdbc build broken with plain ant since e7c2c93