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

[MRG+1] Added MEMUSAGE_CHECK_INTERVAL_SECONDS to Memory usage extension options. #1282

Merged
merged 3 commits into from Jul 2, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/topics/extensions.rst
Expand Up @@ -222,6 +222,7 @@ can be configured with the following settings:
* :setting:`MEMUSAGE_WARNING_MB`
* :setting:`MEMUSAGE_NOTIFY_MAIL`
* :setting:`MEMUSAGE_REPORT`
* :setting:`MEMUSAGE_CHECK_INTERVAL_SECONDS`

Memory debugger extension
~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down
14 changes: 14 additions & 0 deletions docs/topics/settings.rst
Expand Up @@ -711,6 +711,20 @@ Scrapy (if MEMUSAGE_ENABLED is True). If zero, no check will be performed.

See :ref:`topics-extensions-ref-memusage`.

.. setting:: MEMUSAGE_CHECK_INTERVAL_SECONDS

MEMUSAGE_CHECK_INTERVAL_SECONDS
-------------------------------

Default: ``60.0``

Scope: ``scrapy.extensions.memusage``

The frequence which the current memory usage will be checked against the
Copy link
Member

Choose a reason for hiding this comment

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

  • Frequency is an inverse of interval.
  • Even though option name has 'SECONDS' in it, it'd be good to mention the number is in seconds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, now I understand. And there was a typo as well, which you gracefully ignored. Thanks. :) Will push a change very soon.

limits set by :setting:`MEMUSAGE_LIMIT_MB` and :setting:`MEMUSAGE_WARNING_MB`.

See :ref:`topics-extensions-ref-memusage`.

.. setting:: MEMUSAGE_NOTIFY_MAIL

MEMUSAGE_NOTIFY_MAIL
Expand Down
7 changes: 4 additions & 3 deletions scrapy/extensions/memusage.py
Expand Up @@ -36,6 +36,7 @@ def __init__(self, crawler):
self.limit = crawler.settings.getint('MEMUSAGE_LIMIT_MB')*1024*1024
self.warning = crawler.settings.getint('MEMUSAGE_WARNING_MB')*1024*1024
self.report = crawler.settings.getbool('MEMUSAGE_REPORT')
self.check_interval = crawler.settings.getfloat('MEMUSAGE_CHECK_INTERVAL_SECONDS', 60.0)
Copy link
Member

Choose a reason for hiding this comment

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

I think it is better to put a default value (60.0) to https://github.com/scrapy/scrapy/blob/master/scrapy/settings/default_settings.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

self.mail = MailSender.from_settings(crawler.settings)
crawler.signals.connect(self.engine_started, signal=signals.engine_started)
crawler.signals.connect(self.engine_stopped, signal=signals.engine_stopped)
Expand All @@ -56,15 +57,15 @@ def engine_started(self):
self.tasks = []
tsk = task.LoopingCall(self.update)
self.tasks.append(tsk)
tsk.start(60.0, now=True)
tsk.start(self.check_interval, now=True)
if self.limit:
tsk = task.LoopingCall(self._check_limit)
self.tasks.append(tsk)
tsk.start(60.0, now=True)
tsk.start(self.check_interval, now=True)
if self.warning:
tsk = task.LoopingCall(self._check_warning)
self.tasks.append(tsk)
tsk.start(60.0, now=True)
tsk.start(self.check_interval, now=True)

def engine_stopped(self):
for tsk in self.tasks:
Expand Down