Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[MonologBridge] Reset loggers on workers#40761

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:5.xfroml-vo:messenger_reset_loggers
Apr 13, 2021

Conversation

@l-vo
Copy link
Contributor

@l-vol-vo commentedApr 10, 2021
edited
Loading

QA
Branch?5.x
Bug fix?no
New feature?yes
Deprecations?no
Tickets
LicenseMIT
Doc PR

This PR tries to solve some problems with buffered handlers (FingerCrossed) in workers.

Let's consider the default configuration (stop_buffering: true):

  • When the threshold is crossed, all logs are flushed. Logs for the current message but also logs of previous messages in the buffer. Although buffer is limitedbuffer_size, it's a shame to keep logs of previous messages.
  • When the threshold is crossed, buffering is disabled. So finger crossed configuration is not used anymore, all the logs are flushed as soon as they are written.

Then with (stop_buffering: false) (why isn't this the default configuration ?)

  • It's a bit better since buffering isn't disabled when the threshold is crossed
  • Like withstop_buffering: true, logs of previous messages are kept in memory

In a similar way ofDoctrineClearEntityManagerWorkerSubscriber, this PR adds aResetLoggersWorkerSubscribber to reset resettable loggers.

Integration in Monolog bundle:symfony/monolog-bundle#403

zmitic reacted with thumbs up emoji
@l-vol-voforce-pushed themessenger_reset_loggers branch 2 times, most recently from34ad0d8 to3845558CompareApril 10, 2021 14:59
@l-vol-vo changed the titleWIP: Reset loggers on workersReset loggers on workersApr 10, 2021
@l-vol-vo changed the titleReset loggers on workers[Messenger] Reset loggers on workersApr 10, 2021
@l-vol-voforce-pushed themessenger_reset_loggers branch from3845558 to895e5afCompareApril 10, 2021 15:26
@l-vol-vo closed thisApr 10, 2021
@l-vol-vo reopened thisApr 10, 2021
Copy link
Member

@jderussejderusse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Would be great to provide the "glue" in monolog-bundle

@carsonbotcarsonbot changed the title[Messenger] Reset loggers on workers[MonologBridge] Reset loggers on workersApr 10, 2021
@jderussejderusse added this to the5.x milestoneApr 10, 2021
@l-vol-voforce-pushed themessenger_reset_loggers branch from895e5af toadfa302CompareApril 10, 2021 20:57
@l-vo
Copy link
ContributorAuthor

Would be great to provide the "glue" in monolog-bundle

just pushed it:symfony/monolog-bundle#403

@l-vol-voforce-pushed themessenger_reset_loggers branch fromadfa302 to005686cCompareApril 11, 2021 13:43
CHANGELOG
=========

5.3.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.


5.3.0
-----
* Added`ResetLoggersWorkerSubscriber` to reset buffered logs in messenger workers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

missing empty line above

Added -> Add

@fabpot
Copy link
Member

Thank you@l-vo.

@gggeek
Copy link

Sorry for chiming in so late, just giving my 2c of advice: this is a nice feature to have, that is in fact useful forany long-running sf console script, be it a daemon or a huge batch script doing import/export of data and taking many hours to run.

As such, it could probably use a better name than 'SomethingWorker', to make it more evident to developers that it is not tied exclusively to the Messenger component.
It could also benefit of being given more visibility in the docs (if not in the code itself, at least in the symfony docs)

@l-vo
Copy link
ContributorAuthor

l-vo commentedMay 11, 2021
edited
Loading

@gggeek It would be useful for any long-running process that uses a loop BUT this feature relies on a worker event. It's why it is named somethingWorker 🙂

@gggeek
Copy link

@l-vo doh. I missed the namespace of the event, my bad. Wouldn't it be better then to have the worker emit instead a resetLogs event?

@l-vo
Copy link
ContributorAuthor

@gggeek No it wouldn't. The reason is decoupling. The emitted event should not be aware of what the listeners are going to do.

@gggeek
Copy link

But then the monolog bridge is forced to know about this event - as well as any other event required by daemon-like scripts. It does not seem very different...

nicolas-grekas added a commit that referenced this pull requestMay 23, 2021
This PR was merged into the 6.0 branch.Discussion----------[MonologBridge] Remove deprecated code| Q             | A| ------------- | ---| Branch?       | 6.0| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       |#40761| License       | MIT| Doc PR        | N/ACommits-------53ce26e [MonologBridge] Remove deprecated code
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@jderussejderussejderusse left review comments

@fabpotfabpotfabpot approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

5 participants

@l-vo@fabpot@gggeek@jderusse@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp