Re: About binaryTransfer.

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.katsumata1122@gmail.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.tomonari@po.ntts.co.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@gmail.com <davecramer@gmail.com> on behalf of Dave
>> >> Cramer <pg@fastcrypt.com>
>> >> *Sent:* 15 April 2014 15:34
>> >> *To:* Tomonari Katsumata
>> >> *Cc:* Tomonari Katsumata; Mikko Tiihonen; pgsql-jdbc@postgresql.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.tomonari@po.ntts.co.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.tomonari@po.ntts.co.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@fastcrypt.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.tomonari@po.ntts.co.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@postgresql.org<pgsql-jdbc-owner@
>> postgresql.
>> >>>>> org>
>> >>>>>>>>> on behalf of Tomonari Katsumata <katsumata.tomonari@po.ntts.
>> co.jp>
>> >>>>>>>>>> Sent: 21 February 2014 08:40
>> >>>>>>>>>> To: pgsql-jdbc@postgresql.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@8232a5d,
>> >>>>>>>>>> maxRows=0, fetchSize=0, flags=16
>> >>>>>>>>>> 14:09:55.878 (1) simple execute,
>> >>>>>>>>>> handler=org.postgresql.jdbc2.AbstractJdbc2Statement$
>> >>>>>>>>> StatementResultHandler@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@19cdbc83,
>> >>>>>>>>>> maxRows=0, fetchSize=0, flags=48
>> >>>>>>>>>> 14:20:34.992 (1) simple execute,
>> >>>>>>>>>> handler=org.postgresql.jdbc2.AbstractJdbc2Statement$
>> >>>>>>>>> StatementResultHandler@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@postgresql.org)
>> >>>>>>>>>> To make changes to your subscription:
>> >>>>>>>>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>> >>>>>>>>>>
>> >>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>> --
>> >>>>>>>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>> >>>>>>>>> To make changes to your subscription:
>> >>>>>>>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>> >>>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>
>> >>>
>> >>>
>> >>>
>> >>>
>> >>
>> >
>>
>>
>>
>