Hello Nir Soffer,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/62002

to review the following change.

Change subject: jobs: Autodelete
......................................................................

jobs: Autodelete

For storage jobs we like to delete finished jobs automatically after some
delay, giving engine enough time to discover the job completion state in most
cases, and allowing easy inspection of job state.  The Job class implements now
both run() and abort(), managing the state changes and auto-deleting the job
when done. Subclasses should implement _run() and _abort().

To intergate with v2v code using old jobs framework, v2v code will have to use
_run() instead of run(). Subclass that want to enable autodelete must override
the autodelete class attribute and set it to True.

class AutodeletingJob(jobs.Job):
    autodelete = True

This feature is not relevant now for v2v jobs since engine is deleting these
jobs.

Change-Id: I4aafd2271397bd9bc4289dc1aab723398d475add
Signed-off-by: Adam Litke <ali...@redhat.com>
Signed-off-by: Nir Soffer <nsof...@redhat.com>
---
M lib/vdsm/config.py.in
M lib/vdsm/jobs.py
M tests/jobsTests.py
3 files changed, 67 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/02/62002/1

diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index 9813fe5..50282e3 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -394,6 +394,14 @@
             '/sys/block/<device>/queue/discard_max_bytes is not zero.'),
     ]),
 
+    # Section: [jobs]
+    ('jobs', [
+        ('autodelete_delay', '3600',
+            'Automatically delete completed jobs from memory after the '
+            'specified delay (in seconds).  When this value is negative '
+            'autodelete will be disabled.'),
+    ]),
+
     # Section: [addresses]
     ('addresses', [
 
diff --git a/lib/vdsm/jobs.py b/lib/vdsm/jobs.py
index 21b6cb0..d6b3416 100644
--- a/lib/vdsm/jobs.py
+++ b/lib/vdsm/jobs.py
@@ -22,6 +22,7 @@
 import logging
 import threading
 
+from vdsm import config
 from vdsm import exception
 from vdsm import response
 
@@ -68,6 +69,10 @@
 
 class Job(object):
     _JOB_TYPE = None
+
+    # If set to True, jobs of this class will be automatically deleted when
+    # aborted or finished after a configurable delay.
+    autodelete = False
 
     def __init__(self, job_id, description=''):
         self._id = job_id
@@ -118,11 +123,15 @@
         return self.status in (STATUS.PENDING, STATUS.RUNNING)
 
     def abort(self):
+        # TODO: Don't abort if not pending and not running
         self._status = STATUS.ABORTED
         logging.info('Job %r aborting...', self._id)
         self._abort()
+        if self.autodelete:
+            self._autodelete()
 
     def run(self):
+        # TODO: Don't run if aborted or not pending
         self._status = STATUS.RUNNING
         vars.job_id = self.id
         try:
@@ -138,6 +147,8 @@
             self._status = STATUS.DONE
         finally:
             vars.job_id = None
+            if self.autodelete:
+                self._autodelete()
 
     def _abort(self):
         """
@@ -151,6 +162,18 @@
         """
         raise NotImplementedError()
 
+    def _autodelete(self):
+        timeout = config.getint("jobs", "autodelete_delay")
+        if timeout >= 0:
+            _scheduler.schedule(timeout, self._delete)
+
+    def _delete(self):
+        logging.info("Autodeleting job %r", self.info())
+        try:
+            _delete(self._id)
+        except Exception:
+            logging.exception("Cannot delete job %s", self._id)
+
     def __repr__(self):
         s = "<{self.__class__.__name__} id={self.id} status={self.status} "
         if self.progress is not None:
diff --git a/tests/jobsTests.py b/tests/jobsTests.py
index dce6535..1781dba 100644
--- a/tests/jobsTests.py
+++ b/tests/jobsTests.py
@@ -17,12 +17,15 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
+import time
 import uuid
 
 from vdsm import exception, jobs, response, schedule
 from vdsm.storage.threadlocal import vars
 
+from monkeypatch import MonkeyPatchScope
 from testlib import VdsmTestCase, expandPermutations, permutations
+from testlib import make_config
 from testlib import wait_for_job
 
 
@@ -53,6 +56,10 @@
     _JOB_TYPE = 'bar'
 
 
+class AutodeleteJob(TestingJob):
+    autodelete = True
+
+
 class ProgressingJob(jobs.Job):
 
     def __init__(self):
@@ -74,6 +81,7 @@
 
     def setUp(self):
         scheduler = schedule.Scheduler()
+        scheduler.start()
         jobs.start(scheduler)
 
     def tearDown(self):
@@ -266,3 +274,31 @@
         self.run_job(job)
         self.assertEqual(jobs.STATUS.FAILED, job.status)
         self.assertIsInstance(job.error, exception.VdsmException)
+
+    @permutations(((None, ), (Exception(),)))
+    def test_autodelete_when_finished(self, error):
+        cfg = make_config([('jobs', 'autodelete_delay', '0')])
+        job = AutodeleteJob(exception=error)
+        jobs.add(job)
+        with MonkeyPatchScope([(jobs, 'config', cfg)]):
+            self.run_job(job)
+            time.sleep(0.1)  # Wait for the scheduler
+            self.assertRaises(jobs.NoSuchJob, jobs.get, job.id)
+
+    def test_autodelete_when_aborted(self):
+        cfg = make_config([('jobs', 'autodelete_delay', '0')])
+        job = AutodeleteJob()
+        jobs.add(job)
+        with MonkeyPatchScope([(jobs, 'config', cfg)]):
+            job.abort()
+            time.sleep(0.1)  # Wait for the scheduler
+            self.assertRaises(jobs.NoSuchJob, jobs.get, job.id)
+
+    def test_autodelete_disabled(self):
+        cfg = make_config([('jobs', 'autodelete_delay', '-1')])
+        job = AutodeleteJob()
+        jobs.add(job)
+        with MonkeyPatchScope([(jobs, 'config', cfg)]):
+            self.run_job(job)
+            time.sleep(0.1)  # Wait for the scheduler
+            self.assertEqual(job.id, jobs.get(job.id).id)


-- 
To view, visit https://gerrit.ovirt.org/62002
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4aafd2271397bd9bc4289dc1aab723398d475add
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Reply via email to