synopse / mORMot2

OpenSource RESTful ORM/SOA/MVC Framework for Delphi and FreePascal
https://synopse.info
Other
485 stars 122 forks source link

"AutoFlushTimeOut" property does not work. #232

Closed nakisen closed 5 months ago

nakisen commented 5 months ago

I made a small correction and this propery works. I'm not sure if this will be enough.


Screenshot_2

synopse commented 5 months ago

The Flush(true) is not mandatory - false is enough in practice, and true would wait for pending data to be actually written on disk, which slows down everything for no reason. Flush(true) seems only needed when an exception is catched, not in normal process.

About the Tix10 flag, perhaps https://github.com/synopse/mORMot2/commit/d71c071e makes more sense.

nakisen commented 5 months ago

I think BufferSize is controlled by Writer. Therefore it should be Flush(true). At the same time, it would be appropriate to update the fNextFlushTix10 value after the flush.

procedure TSynLog.Flush(ForceDiskWrite: boolean);
var
  diskflush: THandle;
begin
  if fWriter = nil then
    exit;
  diskflush := 0;
  mormot.core.os.EnterCriticalSection(GlobalThreadLock);
  try
    fWriter.FlushToStream;
    if ForceDiskWrite and
       fWriterStream.InheritsFrom(THandleStream) then
      diskflush := THandleStream(fWriterStream).Handle;

    if fNextFlushTix10 <> 0 then
      inc(fNextFlushTix10, (mormot.core.os.GetTickCount64 shr 10) + fFamily.AutoFlushTimeOut);

  finally
    mormot.core.os.LeaveCriticalSection(GlobalThreadLock);
  end;
  if diskflush <> 0 then
    FlushFileBuffers(diskflush); // slow OS operation outside of the main lock
end;
synopse commented 5 months ago

Flush(true) has nothing to do with the buffers: it is just present to trigger FlushFileBuffer() OS API call.

You are right about fNextFlushTix10 but I guess the correct place is within each fWriter.FlushToStream() call: https://github.com/synopse/mORMot2/commit/a5e40ed7

nakisen commented 5 months ago

Thanks, this worked.