Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory problem with SQL Logs #321

Open
marcus-at-localhost opened this issue Jan 21, 2018 · 9 comments
Open

Memory problem with SQL Logs #321

marcus-at-localhost opened this issue Jan 21, 2018 · 9 comments
Labels

Comments

@marcus-at-localhost
Copy link

Hi,
I have a cli script, that runs through a big (12000 items) json file and updates a mysql database. After 200 entries the script dies with the info that sql.php line 230 tried to allocate X of memory. (Sorry I don't remember the exact words)

This is line 230:

if ($log)
	$this->log=str_replace('(-0ms)',
	'('.sprintf('%.1f',1e3*(microtime(TRUE)-$now)).'ms)',
	$this->log);

This is my script (simplified)

//$this->db->log(FALSE); // turn off logging, else it runs out of memory
$this->crawlerdb = new DB\SQL\Mapper($this->db,'crawler');

$entries = json_decode($big_json_file);
foreach($entries as $entry){
	$this->crawlerdb->load(array('source_id = ?', $entry->id));

	$this->crawlerdb->copyfrom(array(
		'type' => $entry->type,
		'url' => $entry->url,
		'title' => $entry->title, //... more fields
	));
	// insert or update
	$this->crawlerdb->save();
}

After I set a var_dump($this->log) on sql.php:230 I saw that $this->log just grows (exec time and queries) and then hits the php memory limit.

I know that this is intended, to profile SQL calls, but since logging is on by default, it was confusing to see the script crash and not really knowing why.

F3 keeps track of all commands issued to the underlying SQL database driver, as well as the time it takes for each statement to complete - just the right information you need to tweak application performance. https://fatfreeframework.com/3.6/databases#Profiling

With logging turned off $this->db->log(FALSE); it seems to work.
So I wonder if I made a mistake in my application? Or should the documentation mention this? Or is this an edge case?

Thanks. Marcus

@KOTRET
Copy link
Contributor

KOTRET commented Jan 31, 2018

The log is collected in the memory.
As the log contains the complete query and the values inserted, the log can get huge when inserting tons of records.

How to solve this? No idea, as the getter for the Log imho is inconsistent: passing TRUE just returns the log, passing FALSE disables it. no way to re-enable logging or clear the log.
Maybe there should be a null-value as default (to return the log) and any boolean en-/disables logging.

@xfra35
Copy link
Member

xfra35 commented Jan 31, 2018

I also think that the behaviour of this feature could be improved. IMHO, sql logging should be disabled by default. After all this is a debugging feature and the DEBUG variable itself is disabled by default.

@bcosca
Copy link
Collaborator

bcosca commented Feb 2, 2018

I will leave it to whatever the consensus is.

@KOTRET
Copy link
Contributor

KOTRET commented Feb 2, 2018

+1 for disabling by default

well, any solution is somehow ugly:

  • passing TRUE to re-enable and return log is a bad and probably unwanted sideeffect, as you have to call log(FALSE) to get the same state if logging was disabled before.
  • using null as default to return the log and any boolean to enable / disable logging
    • is not 100% backward compatible
    • affects only users currently passing TRUE because of consistent code style
    • maybe looks a bit odd because the param can be of two types: null / boolean

still a way to reset the log is missing.

@xfra35
Copy link
Member

xfra35 commented Feb 2, 2018

Well if we want to do this properly, we need to give the possibility to perform 4 actions:

  • enable logging
  • disable logging (without clearing its contents)
  • read the log contents
  • clear the log contents

This can't be done without breaking BC so I suggest to tag this issue for v4 and fix it properly when time will come. This is not a blocking issue anyway.

@ikkez
Copy link
Member

ikkez commented Mar 7, 2018

Or maybe a buffer, like only the last 50 queries? ;)

@Rayne
Copy link
Member

Rayne commented Mar 7, 2018

After introducing a buffer someone will ask for a configuration option to bump up the buffer size to m or to reduce it to n. If the solution is only working with a buffer, then it would be also necessary to specify a big integer (e.g. INT_MAX) as buffer size to log "all queries".

  • A buffer size equal to 0 (or (int) false, (int) null) would imply disabled logging. A size of true could enable logging without a buffer.

  • We could introduce new methods and keep log() for backwards compatibility. On the other hand we are talking about adding fat to fatfree. 🤔

@ikkez
Copy link
Member

ikkez commented Mar 7, 2018

For now these are all just ideas: But when we got enough pro+cons, it could end up in something beautiful, even if it will only be in v4, but the ideas now can shape the future ;)
i.e. a new Log class, which can handle multiple different application aspects/modules and is injected into the DB object upon instantiation. No injection = no logging, otherwise the configuration could be made in the Log class itself for this instance, etc. In such a scenario you could also use a custom Log implementation if you want... just an idea ;)

@eazuka
Copy link

eazuka commented Feb 13, 2019

V4, i do hope we will get to this V4 someday.

@ikkez ikkez changed the title Memory problem with SQL Mapper Memory problem with SQL Logs Feb 14, 2019
@ikkez ikkez transferred this issue from bcosca/fatfree Dec 13, 2020
@ikkez ikkez added the v4 label Dec 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants