TSQLDBZEOSConection.create implicitly open connection to DB

mpv · 2019-03-08 11:19:18

We found a problem inside TSQLDBZEOSConection.Create what prevent calling of TSQLDBZEOSConection.Connect

Inside a constructor fDatabase.SetReadOnly(false) will implicitly set fDatabase.isOpened to true, so TSQLDBZEOSConection.Connect is never called.

This issue introduced in 2014 [6b712a4]

The problem I have: overrided TSQLDBZEOSConection.Connect method what execute some statements just after connection, for example set search_path to bla-bla for Postgres is never called (at last for Postgres)

Can it be merged? Pull request 177

Last edited by mpv (2019-03-08 19:30:21)

EgonHugeist · 2019-03-08 14:05:42

@mpv

The implizit call to TZPostgresSQLConnection.Open by setting the readonly property i've fixed a month ago, see:
https://sourceforge.net/p/zeoslib/code-0/5453/tree/ I was running into same issue in one of my projects. That patch is merged to trunk already.

What is TSQLDBZEOSConection.Open? Do you mean TSQLDBZEOSConection.Connect? Huge surprice while running tests: "connect" is never called.
-> I'd tryed to move the determination of the batch-ababilities(i'm not happy about your define solution) in TSQLDBZEOSConection.Connect but i failed. I'd send AB an update for SynDBZeos, the last was ignored maybe latest one will be accepted, however.

Michael

mpv · 2019-03-08 23:19:13

@EgonHugeist

For sure, should be TSQLDBZEOSConection.Connect - I edit my prev. post and merge request description. Thanks!
I pull latest changes from branch 7.2-patches but without my patch connection still opened on Create because of call to GetServerMajorVersion inside SetReadOnly. So @ab - please - apply Pull request 177. @EgonHugeist - I will think for my define solution workaround...

BTW I fund a good place for performance optimization inside ZEOS.
Massive calls to function GetPlainDriver: IZ*PlainDriver inside all TZ*Conenction classes like this

  GetPlainDriver.GetValue(...)

have a serious performance impact (tested on FPC but I think the same is for Delphi)

- because GetPlainDriver return interface compiler implicitly increment/decrement a ref counter before and after function call
- inside GetPlainDriver compiler implicitly create try/finally block
- and so on - see assembler instructions

My propose is to call GetPlainDriver ONCE - inside a constructor just to assign a value to FPlainDriver class property (mark abstract contsructor as virtual and override it in all classes) and in all other places use FPlainDriver (except TZ*Connection.OnPropertiesChange)

I did it for Postgres and got +3% RPS for a simple query by ID (so real ZEOS performance increased more when 3%)

With modifications:

00:36 $ ./wrk -c4 -d 5s http://localhost:8881/dbRaw
Running 5s test @ http://localhost:8881/dbRaw
  2 threads and 4 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     2.10ms    2.93ms  18.85ms   88.89%
    Req/Sec     1.44k   130.12     1.68k    60.00%
  14344 requests in 5.00s, 2.28MB read
Requests/sec:   2867.64
Transfer/sec:    467.67KB

Without modifications:

00:37 $ ./wrk -c4 -d 5s http://localhost:8881/dbRaw
Running 5s test @ http://localhost:8881/dbRaw
  2 threads and 4 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     2.13ms    2.99ms  20.79ms   89.27%
    Req/Sec     1.36k   275.05     1.66k    92.00%
  13508 requests in 5.00s, 2.15MB read
Requests/sec:   2699.04
Transfer/sec:    440.18KB

@EgonHugeist - can you apply such a changes to 7.2-patches brunch? (I don't know how to do a pull request to ZEOS SVN repo)

ab · 2019-03-09 15:30:37

https://github.com/synopse/mORMot/pull/177 has been merged.

Thanks for the feedback.

EgonHugeist · 2019-03-12 15:44:22

@mpv

i did look to your pull request. The Line

fDataBase.SetReadOnly(False)

is not required. It's Zeos default.

According the patch for 7.2-patches you have:
Open a ticket on https://sourceforge.net/p/zeoslib/tickets/
tag it as feature request and write purpose.
add a diff/patch file.

I'm willing to merge, if there is no other behavior change. That's it.

Accoding the performance: On 7.3 all the Interface calls to IZPLainDriver-Descendants do no longer exist(except dblib-protocol -> lack of time to do it)
So i would start from the premisse 7.3 is again a bit faster as 7.2 was.

Last edited by EgonHugeist (2019-03-12 16:14:49)

mpv · 2019-03-12 21:40:19

I tried to switch to 7.3. Performance is ok there.

But I found some regression - ticket #335 with fix
Another regression is for JSON fields - I'm investigate it deeper and also try to give a fix.

Last edited by mpv (2019-03-12 21:40:50)

mpv · 2019-03-14 11:51:08

My regression with BSON have the same nature as previous - in SLQ statement i select part of JSON

select fieldOfTypeBSON // {"intProp": 10}
select ... from ... where fieldOfTypeBSON ->> 'intProp' = ? 
// bind parameter as INTEGER 10

Zeos bind parameter as binary but postgres await it to be string.
I create a new patch with strict verification what binary binding will done ONLY on case type of parameter from query plane can be casts to type of parameter from binding
See Zeos ticket #336

Last edited by mpv (2019-03-14 19:37:21)

mORMot Open Source

#1 2019-03-08 11:19:18

TSQLDBZEOSConection.create implicitly open connection to DB

#2 2019-03-08 14:05:42

Re: TSQLDBZEOSConection.create implicitly open connection to DB

#3 2019-03-08 23:19:13

Re: TSQLDBZEOSConection.create implicitly open connection to DB

#4 2019-03-09 15:30:37

Re: TSQLDBZEOSConection.create implicitly open connection to DB

#5 2019-03-12 15:44:22

Re: TSQLDBZEOSConection.create implicitly open connection to DB

#6 2019-03-12 21:40:19

Re: TSQLDBZEOSConection.create implicitly open connection to DB

#7 2019-03-14 11:51:08

Re: TSQLDBZEOSConection.create implicitly open connection to DB

Board footer