ISQLDBRows and memory

squirrel · 2017-10-19 12:13:27

@Ab, please forgive me for asking this, I know you've tackled this one a few times. I have searched the forums and documentation.

What is the proper way to clear ISQLDBRows after use? I noticed at #TITL_195 that we should just assign nil to it, however, that seem to produce a memory leak. Am I doing it incorrectly?

Sample code:

procedure TForm1.Button1Click(Sender: TObject);
var
  qry: RawUTF8;
  res: ISQLDBRows;
begin
  qry := 'SELECT count(*) cnt FROM sysdatabases WHERE name = ?';
  res := ODBCConnection.Execute(qry, ['my_db']);
  //res._Release; //this removes it from memory and prevents memory leaks, but throws AV if calling container function repeatedly
  res := nil; //this leaves it in memory and creates a memory leak
end;

Finally, are there any uninit functions that must be called before Freeing TSQLDBConnectionProperties? Just calling ODBCConnection.Free seem to leak as well.

igors233 · 2017-10-19 12:20:13

This issue seems to be related to Delphi compiler itself, simply moving rows processing to sub procedure should work.
See: https://synopse.info/forum/viewtopic.php?pid=9397#p9397

squirrel · 2017-10-19 12:47:14

Thanks igors.

I assume you mean something like this:

procedure TForm1.Button1Click(Sender: TObject);
  procedure checkDB;
  var
    qry: RawUTF8;
    res: ISQLDBRows;
  begin
    qry := 'SELECT count(*) cnt FROM sysdatabases WHERE name = ?';
    res := ODBCConnection.Execute(qry, ['my_db']);
    if Res.Step and (Res['cnt'] > 0) then
      Label1.Caption := 'DB Found: ' + FloatToStr(Res['cnt']);
  end;
begin
  CheckDB;
end;

But it looks as if it does leak if calling Button1Click multiple times. In the example above, even the query stays in memory.

Last edited by squirrel (2017-10-19 12:57:01)

ab · 2017-10-20 10:49:54

What do you call a "leak"?
What is leaked?

There are statement cache enabled by default, I guess, so the statement remains in memory for further re-use.

koraycayiroglu · 2020-01-27 09:09:50

Hello,

I have a memory leak problem with ISQLDBRows same as this, so i am reviving this old topic because i couldn't solve my problem.

I am using ISQLDBRows in an interface based service to, and i have an api call must be called every 30 seconds to get a job list. Here is my code;

function TTerminalMethods.terminal_GetList(const appName, extName: RawUTF8; const terminalId : TID) : RawJSON;
var Props: TSQLDBConnectionProperties;

  procedure GetResult;
  var I: ISQLDBRows;
  begin
    try
//      I := Props.Execute('"),[]);
//
//      Result := I.FetchAllAsJSON(True);
    finally
      I := nil;
    end;
  end;

begin
  Result := '';

  SetConnection(appName, extName, Props); // creating props for usage;
  try
    GetResult;
  finally
    FreeAndNil(Props);
  end;
end;

I use same sample from one of ab's answers but with this code my app's memory constantly rising, nearly 20kb/30 secs.

How can I solve this problem?

Last edited by koraycayiroglu (2020-01-27 09:12:17)

ab · 2020-01-27 09:38:45

Are you sure it is a leak?
What does FastMM4 in full debug mode say?

First of all, I would not recreate the connection for each call. It is a waste of resources.

Such memory increase may come from plenty of reasons, e.g. some aggressive caching in the DB client driver itself.

xalo · 2020-01-27 09:52:09

IMHO it's something related regarding some databases drivers needs to free connection in each thread that you create it, therefore you should call
.EndCurrentThread in each call. Something like this:

...
var SQLDBConnection : TSQLDBConnection;
begin
  SQLDBConnection:=fProps.NewConnection;
  try
    ...
  finally
    SQLConnection.Free;
    fProps.EndCurrentThread;
  end; 
end;

Last edited by xalo (2020-01-27 10:07:08)

koraycayiroglu · 2020-01-27 10:48:33

ab wrote:

Are you sure it is a leak?
What does FastMM4 in full debug mode say?
First of all, I would not recreate the connection for each call. It is a waste of resources.
Such memory increase may come from plenty of reasons, e.g. some aggressive caching in the DB client driver itself.

I followed 14-Interfaced based services example, and it recreates connection for each call and i thought it's suppose to be this way because it makes sense for threaded operations. How can i store a global Props in an interface-based service? I would prefer a single connection too.

FastMM don't tell me anything, it's a winsrvc/console app and i don't know why ReportMemoryLeaksOnShutdown seems not working. I tried to run it from another console windows if it's a running from ide problem but still no reports. I will look into it.

How can i disable this aggressive caching?

koraycayiroglu · 2020-01-27 11:04:05

xalo wrote:

IMHO it's something related regarding some databases drivers needs to free connection in each thread that you create it, therefore you should call
.EndCurrentThread in each call. Something like this:
...
var SQLDBConnection : TSQLDBConnection;
begin
  SQLDBConnection:=fProps.NewConnection;
  try
    ...
  finally
    SQLConnection.Free;
    fProps.EndCurrentThread;
  end; 
end;
  

Thanks Xalo,

It seems, this solve my case. I re-code my procedure to:

function TTerminalMethods.terminal_GetList(const appName, extName: RawUTF8; const terminalId : TID) : RawJSON;
var Props: TSQLDBConnectionPropertiesThreadSafe;

  procedure GetResult;
  var I: ISQLDBRows;
    SQLDBConnection : TSQLDBConnection;
  begin
    SQLDBConnection := Props.NewConnection;
    try
      I := SQLDBConnection.NewStatementPrepared('some sql', True);
      Result := I.FetchAllAsJSON(True);
    finally
      I := nil;

      SQLDBConnection.Free;
      Props.EndCurrentThread;
    end;

    Result := NSRemoveBrackets(Result);
  end;

begin
  Result := '';

  SetConnection(appName, extName, Props);
  try
    GetResult;
  finally
    Props.Disconnect;
    Props.ThreadSafeConnection.Free;

    Props := nil;
  end;
end;

This seems working as i expected with zero leaks.

Best regards

Last edited by koraycayiroglu (2020-01-27 11:04:27)

ab · 2020-01-27 11:58:34

I am not sure, but

Props.ThreadSafeConnection.Free

seems weird and unsafe to me.

IMHO there was no leak, just some aggressive caching which you interpreted as a leak.
Increasing memory of a few KB is not a leak. Most of the time, memory consumption stabilizes after some time.

There may be a real leak:
- if FastMM4 in full debug mode or FPC heaptrc report it at shutdown (we run all our tests with both - and some valgrid deep checks have been made by mpv)
- memory on long-term running servers is linearly increasing over time, to exhaust the RAM up to out-of-memory

There are SynDB servers running 24/7 on various platform with no reported leak.

koraycayiroglu · 2020-01-27 13:22:34

Probably, i am not sure about if this was classified as leak or not. But a 20kb/30sec for every client instance using this procedure made server out of memory around 6 hours. This is a problem. Not for me, for any server solution. It suppose to work for days, months etc. without restarting.

Actually, this can be manageable if this server is a standalone one. I made a winservice for our application using mORMot, thanks again by the way this framework is really good, and we install this service to our clients server. Our software is an ERP solution and some of them are very big factories. This client uses a server with rdc/vm enabled and every user working in this server, using RDC. I know it's not a good way to use a server but i can't do aything. It's their own property etc.

It seems with last changes i made, it's not growing as it used to be. I will keep an eye on it.

Best regards to you all.

macfly · 2020-01-27 13:28:55

But is it really necessary to create a connection per thread?

Which database is used?

If you do not create an error occurs?

Last edited by macfly (2020-01-27 13:31:01)

koraycayiroglu · 2020-01-27 13:53:55

macfly wrote:

But is it really necessary to create a connection per thread?

Actually, i am asking same questions to myself too after ab told the same thing. I use examples to build my code and not a single example shows a global prop for interfaced-based service, so i assume it should be this way but after this conversation i will change my code and try different solutions.

I use Props to connect SQLServer.

Is it safe to store Props as a private/protected variable? If is it so, do i need to reconnect with each api call or once it's connected then can i use same props for every instance? Or, is there a better way to achieve this? like, shared data (ILockedDocVariant) used by MVC apps.

I guess, i will try and find out.

macfly · 2020-01-27 14:01:08

I thought you were using sqlite.

I've never worked with SQLServer so I can't tell you if it's necessary or not. Better to wait for someone who has the knowledge.

I already worked with ADO which is not thread safe and I had to create a connection per thread.

ab · 2020-01-28 10:23:33

If the provider is not thread-safe, SynDB units will by themselves maintain a single connection per thread.
So you don't have to bother with maintaining the connections: the library does it for you.

xalo · 2020-01-28 11:30:04

Yes @ab, but if the call is not in the main thread you have to call:

fProps.EndCurrentThread;

IIRC in SQL server you have to proceed as stated.

ab · 2020-01-28 13:29:56

If you use our ORM/SOA server, all threading start/ending is done for you IIRC:

   ...
    // - within the mORMot server, mORMotDB unit will call this method
    // for every terminating thread created for TSQLRestServerNamedPipeResponse
    // or TSQLHttpServer multi-thread process
    procedure EndCurrentThread; virtual;

See TSQLRestStorageExternal.EndCurrentThread method.

trx · 2020-01-28 14:37:16

Note that if your TSQLRestServer is not registered with your TSQLHttpServer, TSQLRestServer.EndCurrentThread will not be called so you must call it yourself.

For example, I have two TSQLRestServers:
1. One for external DB which I don't register with TSQLHttpServer.
2. One in memory which is registered with TSQLHttpServer which provides interface based services. Those services then use the first REST server to get the required data and a DB connection is created per thread.
When the thread ends, TSQLHttpServer will call EndCurrentThread of the server 2. but not 1. so the DB connection will not be released.

To solve this I use a TSQLHttpServer descendant:

TMyHTTPServer = class(TSQLHttpServer)
  private
    FDBRest: TSQLRestServer;
  protected
    procedure HttpThreadTerminate(Sender: TThread); override;
    begin
      inherited HttpThreadTerminate(Sender);
      if Assigned(FDBRest) then
        FDBRest.EndCurrentThread(Sender); 
    end;
end;

Last edited by trx (2020-01-28 14:37:57)

macfly · 2020-01-28 16:35:36

trx wrote:

Note that if your TSQLRestServer is not registered with your TSQLHttpServer, TSQLRestServer.EndCurrentThread will not be called so you must call it yourself.

I have this scenario and there are no leaks that were reported by FastMM4 or noticed.

By doing this you are not forcing the creation of connections per thread?
Depending on the provider, is this not necessary as mentioned by ab above?

trx · 2020-01-28 22:12:17

macfly wrote:

trx wrote:
Note that if your TSQLRestServer is not registered with your TSQLHttpServer, TSQLRestServer.EndCurrentThread will not be called so you must call it yourself.
I have this scenario and there are no leaks that were reported by FastMM4 or noticed.
By doing this you are not forcing the creation of connections per thread?
Depending on the provider, is this not necessary as mentioned by ab above?

In my case some requests were not processed in the thread pool because of keep-alive, so a dedicated background thread was created.
See the implementation of TSynThreadPoolTHttpServer.Task(aCaller: TSynThread; aContext: Pointer) in SynCrtSock.pas for details.

In that case, when a DB connection (for me via ZEOS to MySQL) is created for each background thread, it is not automatically released when the thread ends.
I have only noticed this because MySQL started complaining about too many connections.

Last edited by trx (2020-01-28 22:13:23)

mpv · 2020-01-29 17:16:02

Fou our products we disable keep alive on THTTPServer level to prevent creation of DB connection for each keep alive thread, because many connection kills the database.

We setup keep alive on reverse proxy ( nginx) what stands before our server. If nginx and app server on the same host where is no performance impact in such configuration. For even better performance Unix Domain Socket can be used between nginx and app server. In such architecture our produtions works with near 1000 RPS average load without problems.

To work with keep alive without reverse proxy reenginiring of mORMot is required. Good solution is to use two diffetent thread pool - one lightweight pool for http connections ( or even single thread reactor) and another fixed size pool for task processing ( here we can use threadSafeConnection to DB and all computations.

ab · 2020-01-30 08:36:22

Good point, mpv!

I started some refactoring to have the POSIX server use a thread pool even for kept alive and WebSockets connections, instead of maintaining a thread per long-term connection.
It could help the problem of too many connections to the DB.

But on POSIX/Linux, putting nginx as front-end is highly recommended.

mORMot Open Source

#1 2017-10-19 12:13:27

ISQLDBRows and memory

#2 2017-10-19 12:20:13

Re: ISQLDBRows and memory

#3 2017-10-19 12:47:14

Re: ISQLDBRows and memory

#4 2017-10-20 10:49:54

Re: ISQLDBRows and memory

#5 2020-01-27 09:09:50

Re: ISQLDBRows and memory

#6 2020-01-27 09:38:45

Re: ISQLDBRows and memory

#7 2020-01-27 09:52:09

Re: ISQLDBRows and memory

#8 2020-01-27 10:48:33

Re: ISQLDBRows and memory

#9 2020-01-27 11:04:05

Re: ISQLDBRows and memory

#10 2020-01-27 11:58:34

Re: ISQLDBRows and memory

#11 2020-01-27 13:22:34

Re: ISQLDBRows and memory

#12 2020-01-27 13:28:55

Re: ISQLDBRows and memory

#13 2020-01-27 13:53:55

Re: ISQLDBRows and memory

#14 2020-01-27 14:01:08

Re: ISQLDBRows and memory

#15 2020-01-28 10:23:33

Re: ISQLDBRows and memory

#16 2020-01-28 11:30:04

Re: ISQLDBRows and memory

#17 2020-01-28 13:29:56

Re: ISQLDBRows and memory

#18 2020-01-28 14:37:16

Re: ISQLDBRows and memory

#19 2020-01-28 16:35:36

Re: ISQLDBRows and memory

#20 2020-01-28 22:12:17

Re: ISQLDBRows and memory

#21 2020-01-29 17:16:02

Re: ISQLDBRows and memory

#22 2020-01-30 08:36:22

Re: ISQLDBRows and memory

Board footer