Skip to content
This repository has been archived by the owner on Jan 5, 2022. It is now read-only.

Logs don't rotate for a running process #2

Closed
vitch opened this issue Oct 6, 2011 · 5 comments
Closed

Logs don't rotate for a running process #2

vitch opened this issue Oct 6, 2011 · 5 comments

Comments

@vitch
Copy link
Contributor

vitch commented Oct 6, 2011

Hi,

Since you store the handle to the log file in a static variable the file is only opened once per process. In my case I have a process which has run across a date boundary and I'm using an implementation of log_file() based on date('Ymd').

Since the handle is already there the code never re-calls log_file() and doesn't find out that there should be a new filename so it carries on writing to the old log file.

Possible solutions would be:

  • Closing the handle every call to log
  • Adding another variable which stores the filename for the current handle and if this is different to that returned by log_file() then close the handle and open a new one
  • ...or something else?

I think I prefer the second option and can implement it with a pull request if you like but maybe I'm missing something and there is a better third option?

Thanks,

Kelvin :)

@shaneharter
Copy link
Owner

Again, a good call!

This is something I've had on my private to-do list for a while. In the places I've implemented it, I've kept the auto-restart interval at 1 day (or sometimes 12 hours), so log rotation does happen every 24 hours, but not at midnight directly. That's been an acceptable solution given time constraints.

I definitely like the 2nd option, and if you submit a pull request I'd absolutely merge it in.

Not sure if you saw the Named Workers branch, but that's been what I've been focusing most of my dev time on this project on. Basically, it's an attempt at implementing the Web Workers API. It's pretty far along, but not there yet.

The pretty great thing about is it that you can create a named worker like "pushDataToAPI", pass it a closure, and then, in your execute() method, you can just call $this->pushDataToAPI() as if it was a local method -- behind the scenes it forks and runs the closure and returns an object you can use to check the status of it. You can do all of this now with the simple fork() method, but you'd have to write it yourself.

My use case for this is that I have a daemon that is running every 30 seconds. I want to update a DB with some aggregate data, and that takes 1-2 mins. In this case, I'd start the Worker, and my daemon keeps on iterating, and each iteration it checks the Worker and if it's done, it starts it again. So I'm continuously updating the DB in the worker without holding-up the daemon itself.

It's great to get a 2nd opinion on this project, if you have any input on the implementation I described, I'd love to hear it now before I finish the feature and merge it into master.

Shane

@vitch
Copy link
Contributor Author

vitch commented Oct 6, 2011

Hi,

Cool - I'll try to implement the second option if I get a chance. It's not a super high priority for me right now because as you say the files do rotate, just not at midnight (which confused me the first morning I looked at my logs!).

I hadn't seen the named workers branch and it actually looks like it might be really useful for something that I've just been trying to set up (a listener to a RabbitMQ queue which blocks waiting for messages so has to be forked off from the main daemon. I'll see about integrating it and let you know if I have any further feedback,

Cheers,

Kelvin :)

@shaneharter
Copy link
Owner

Named workers are definitely not ready for use -- there are some key pieces
of the implementation missing. I do want to wrap it up soon, though.

On Thu, Oct 6, 2011 at 11:22 AM, Kelvin Luck <
[email protected]>wrote:

Hi,

Cool - I'll try to implement the second option if I get a chance. It's not
a super high priority for me right now because as you say the files do
rotate, just not at midnight (which confused me the first morning I looked
at my logs!).

I hadn't seen the named workers branch and it actually looks like it might
be really useful for something that I've just been trying to set up (a
listener to a RabbitMQ queue which blocks waiting for messages so has to be
forked off from the main daemon. I'll see about integrating it and let you
know if I have any further feedback,

Cheers,

Kelvin :)

Reply to this email directly or view it on GitHub:
#2 (comment)

vitch added a commit to vitch/PHP-Daemon that referenced this issue Oct 7, 2011
vitch added a commit to vitch/PHP-Daemon that referenced this issue Oct 19, 2011
…is still open.

Fixes issue shaneharter#2...
Conflicts:

	Core/Daemon.php
@shaneharter
Copy link
Owner

For the v2.0 release I've seriously simplified the log method, getting rid of the code that writes to stdout when it can't write to a log file. Now it just raises an E_USER_WARNING instead.

I did, though, agree that it's silly that log files couldn't be rotated after the application was started, so I took your code for that and I tweaked it a bit to avoid calling the log_file() method every time a message is logged. Instead, it will check for a changed log file result at even 5 minute increments (00:05, 00:10, etc).

@vitch
Copy link
Contributor Author

vitch commented Jun 11, 2012

Thanks for the update. The changes for 2.0 look great - I'll definitely check it out next time I need to daemonise a PHP script :)

ekzobrain added a commit to ekzobrain/PHP-Daemon that referenced this issue Jun 15, 2013
2. Improved memory allocation in SysV
3. Fixed notice on exit, reason was cyclic reference when destroying objects:
PHP Notice: Undefined property: Core_Worker_Via_SysV::$mediator in /opt/PHP-Daemon/Core/Worker/Via/SysV.php on line 95 pid 16905
#0  Core_Worker_Via_SysV->setup_ipc() called at [/opt/PHP-Daemon/Core/Worker/Via/SysV.php:521]
shaneharter#1  Core_Worker_Via_SysV->release() called at [/opt/PHP-Daemon/Core/Worker/Mediator.php:258]
shaneharter#2  Core_Worker_Mediator->__destruct() called at [/opt/PHP-Daemon/Core/Worker/Via/SysV.php:53]
shaneharter#3  Core_Worker_Via_SysV->__destruct()
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants