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

FileTarget::$rotateByCopy: if not specified, automatically set to false when OS is not windows #13057

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@felix021

felix021 commented Nov 22, 2016

Q A
Is bugfix? no
New feature? yes
Breaks BC? no
Tests pass? yes
Fixed issues

FileTarget::$rotateByCopy: if not specified, automatically set to false when OS is not windows

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Nov 22, 2016

Member

Why?

Member

samdark commented Nov 22, 2016

Why?

@yii-bot

This comment has been minimized.

Show comment
Hide comment
@yii-bot

yii-bot Nov 22, 2016

Thanks for posting in our issue tracker.
In order to properly assist you, we need additional information:

  • When does the issue occur?
  • What do you see?
  • What was the expected result?
  • Can you supply us with a stacktrace? (optional)
  • Do you have exact code to reproduce it? Maybe a PHPUnit tests that fails? (optional)

Thanks!

This is an automated comment, triggered by adding the label status:need more info.

yii-bot commented Nov 22, 2016

Thanks for posting in our issue tracker.
In order to properly assist you, we need additional information:

  • When does the issue occur?
  • What do you see?
  • What was the expected result?
  • Can you supply us with a stacktrace? (optional)
  • Do you have exact code to reproduce it? Maybe a PHPUnit tests that fails? (optional)

Thanks!

This is an automated comment, triggered by adding the label status:need more info.

@felix021

This comment has been minimized.

Show comment
Hide comment
@felix021

felix021 Nov 22, 2016

In our application on Yii 2, maxFileSize is set to be 1 GB (with 10 log files, the default value), and by default $rotateByCopy = true, which means, when log files need to be rotated, there may be as much as 9 GB of logs to be read into memory, and write to disk, which is meaningless on Linux servers and causes large pressure on disk IO, and thus make the server not responding to requests. Automatic OS detection in this pull request helps improve situation on non-windows servers.

felix021 commented Nov 22, 2016

In our application on Yii 2, maxFileSize is set to be 1 GB (with 10 log files, the default value), and by default $rotateByCopy = true, which means, when log files need to be rotated, there may be as much as 9 GB of logs to be read into memory, and write to disk, which is meaningless on Linux servers and causes large pressure on disk IO, and thus make the server not responding to requests. Automatic OS detection in this pull request helps improve situation on non-windows servers.

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Nov 22, 2016

Member

you should choose a smaller file size then.

Member

cebe commented Nov 22, 2016

you should choose a smaller file size then.

@felix021

This comment has been minimized.

Show comment
Hide comment
@felix021

felix021 Nov 22, 2016

I can change it to 100 MB, but 900 MB of disk io will still take my server down for over 1 minute, and as is described in the comments, this flag is set to true because rotation fails ONLY on Windows, so why don't we just set it to false on non-windows servers to avoid such condition? And I have to say, that you guys didn't warn developers to use smaller maxFileSize in advance either, which I spent days to find out the reason of downtime. This is not friendly to yii users.

felix021 commented Nov 22, 2016

I can change it to 100 MB, but 900 MB of disk io will still take my server down for over 1 minute, and as is described in the comments, this flag is set to true because rotation fails ONLY on Windows, so why don't we just set it to false on non-windows servers to avoid such condition? And I have to say, that you guys didn't warn developers to use smaller maxFileSize in advance either, which I spent days to find out the reason of downtime. This is not friendly to yii users.

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Nov 22, 2016

Member

Okay now I understand the problem. Thanks for explaining. It makes sense now.

Member

cebe commented Nov 22, 2016

Okay now I understand the problem. Thanks for explaining. It makes sense now.

@cebe cebe added this to the 2.0.12 milestone Nov 22, 2016

@cebe cebe modified the milestones: 2.1.0, 2.0.12 Apr 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment