[Kimchi-devel] [PATCH] [Wok 1/3] Issue #158: Move AsyncTask information to memory
Paulo Ricardo Paz Vital
pvital at linux.vnet.ibm.com
Fri Aug 26 11:35:52 UTC 2016
On Aug 26 08:18AM, Aline Manera wrote:
>
>
> On 08/25/2016 11:45 AM, Paulo Ricardo Paz Vital wrote:
> > Hi Lucio.
> >
> > On Aug 25 11:23AM, Lucio Correia wrote:
> > > One suggestion is to move all task stuff to asynctask.py
> > Yeah, I think about that, but then I went to Kimchi, Gingerbase and
> > Ginger code to see how huge would be modify all call of add_task()
> > method from wok.utils to wok.asynctask.
> >
> > IMO, this is a huge change, specially now near to code freeze for 2.3
> > release. It's totally feasible, but I'm afraid that code submitted to
> > Ginger* is committed first than this one, and then things start to break
> > and the number of issues increase in this moment.
>
> We will have 1 month for bug fixes so I think we will have enough time to
> fix any issue related to that.
>
> About the patch dependency, you just need to add to the patch subject the
> dependency. So the Ginger community will be aware about it to do not merge
> the patch prior to the Wok patch.
>
Ok! I'll change that and submit a V2 for this patch, and new patches to
all other plugins with the necessary modifications.
> >
> > > One more comment below.
> > >
> > > On 24-08-2016 16:36, pvital at linux.vnet.ibm.com wrote:
> > > > From: Paulo Vital <pvital at linux.vnet.ibm.com>
> > > >
> > > > This patch moves all infrastructure of AsyncTask to store and read
> > > > the tasks' information (id, status, messages and target_uri) from
> > > > objectstore to memory, by the implementation of a dictionary of
> > > > AsyncTasks objects.
> > > >
> > > > It also removes the tasks cleanup process from objectstore.
> > > >
> > > > Signed-off-by: Paulo Vital <pvital at linux.vnet.ibm.com>
> > > > ---
> > > > src/wok/asynctask.py | 25 ++++---------------------
> > > > src/wok/i18n.py | 2 --
> > > > src/wok/objectstore.py | 6 ------
> > > > src/wok/utils.py | 31 ++++++++++++++++++++++++-------
> > > > 4 files changed, 28 insertions(+), 36 deletions(-)
> > > >
> > > > diff --git a/src/wok/asynctask.py b/src/wok/asynctask.py
> > > > index fb614a2..2008c18 100644
> > > > --- a/src/wok/asynctask.py
> > > > +++ b/src/wok/asynctask.py
> > > > @@ -21,24 +21,18 @@
> > > >
> > > > import cherrypy
> > > > import threading
> > > > +import time
> > > > import traceback
> > > >
> > > >
> > > > -from wok.exception import OperationFailed
> > > > -
> > > > -
> > > > class AsyncTask(object):
> > > > - def __init__(self, id, target_uri, fn, objstore, opaque=None):
> > > > - if objstore is None:
> > > > - raise OperationFailed("WOKASYNC0001E")
> > > > -
> > > > + def __init__(self, id, target_uri, fn, opaque=None):
> > > > self.id = str(id)
> > > > self.target_uri = target_uri
> > > > self.fn = fn
> > > > - self.objstore = objstore
> > > > self.status = 'running'
> > > > - self.message = 'OK'
> > > > - self._save_helper()
> > > > + self.message = 'Starting AsyncTask'
> > > > + self.timestamp = time.time()
> > > > self._cp_request = cherrypy.serving.request
> > > > self.thread = threading.Thread(target=self._run_helper,
> > > > args=(opaque, self._status_cb))
> > > > @@ -51,17 +45,6 @@ class AsyncTask(object):
> > > >
> > > > if message.strip():
> > > > self.message = message
> > > > - self._save_helper()
> > > > -
> > > > - def _save_helper(self):
> > > > - obj = {}
> > > > - for attr in ('id', 'target_uri', 'message', 'status'):
> > > > - obj[attr] = getattr(self, attr)
> > > > - try:
> > > > - with self.objstore as session:
> > > > - session.store('task', self.id, obj)
> > > > - except Exception as e:
> > > > - raise OperationFailed('WOKASYNC0002E', {'err': e.message})
> > > >
> > > > def _run_helper(self, opaque, cb):
> > > > cherrypy.serving.request = self._cp_request
> > > > diff --git a/src/wok/i18n.py b/src/wok/i18n.py
> > > > index 33107ee..1a0446b 100644
> > > > --- a/src/wok/i18n.py
> > > > +++ b/src/wok/i18n.py
> > > > @@ -33,8 +33,6 @@ messages = {
> > > > "WOKAPI0008E": _("Parameters does not match requirement in schema: %(err)s"),
> > > > "WOKAPI0009E": _("You don't have permission to perform this operation."),
> > > >
> > > > - "WOKASYNC0001E": _("Datastore is not initiated in the model object."),
> > > > - "WOKASYNC0002E": _("Unable to start task due error: %(err)s"),
> > > > "WOKASYNC0003E": _("Timeout of %(seconds)s seconds expired while running task '%(task)s."),
> > > >
> > > > "WOKAUTH0001E": _("Authentication failed for user '%(username)s'. [Error code: %(code)s]"),
> > > > diff --git a/src/wok/objectstore.py b/src/wok/objectstore.py
> > > > index 59354f3..817f60c 100644
> > > > --- a/src/wok/objectstore.py
> > > > +++ b/src/wok/objectstore.py
> > > > @@ -107,8 +107,6 @@ class ObjectStore(object):
> > > > c.execute('''SELECT * FROM sqlite_master WHERE type='table' AND
> > > > tbl_name='objects'; ''')
> > > > res = c.fetchall()
> > > > - # Because the tasks are regarded as temporary resource, the task states
> > > > - # are purged every time the daemon startup
> > > > if len(res) == 0:
> > > > c.execute('''CREATE TABLE objects
> > > > (id TEXT, type TEXT, json TEXT, version TEXT,
> > > > @@ -116,10 +114,6 @@ class ObjectStore(object):
> > > > conn.commit()
> > > > return
> > > >
> > > > - # Clear out expired objects from a previous session
> > > > - c.execute('''DELETE FROM objects WHERE type = 'task'; ''')
> > > > - conn.commit()
> > > > -
> > > > def _get_conn(self):
> > > > ident = threading.currentThread().ident
> > > > try:
> > > > diff --git a/src/wok/utils.py b/src/wok/utils.py
> > > > index 4c132a1..8c13c70 100644
> > > > --- a/src/wok/utils.py
> > > > +++ b/src/wok/utils.py
> > > > @@ -31,6 +31,7 @@ import re
> > > > import sqlite3
> > > > import subprocess
> > > > import sys
> > > > +import time
> > > > import traceback
> > > > import xml.etree.ElementTree as ET
> > > >
> > > > @@ -47,6 +48,7 @@ from wok.stringutils import decode_value
> > > >
> > > > wok_log = cherrypy.log.error_log
> > > > task_id = 0
> > > > +tasks_queue = {}
> > > >
> > > >
> > > > def get_next_task_id():
> > > > @@ -55,17 +57,32 @@ def get_next_task_id():
> > > > return task_id
> > > >
> > > >
> > > > -def get_task_id():
> > > > - global task_id
> > > > - return task_id
> > > > -
> > > > -
> > > > def add_task(target_uri, fn, objstore, opaque=None):
> > > objstore is not in use here
> > I know that, and the reason to keep it here in the method signature is
> > the same I explained above. Specially in this case, if I remove objstore
> > from here, I'll be forced to modify all add_task() calls from Kimchi and
> > Ginger* plugins.
> >
> > PEP8 did not pointed as an error/failure, because (I suposse) it's in the
> > signature of the method and not in the core of the method.
> >
> > > > - id = get_next_task_id()
> > > > - AsyncTask(id, target_uri, fn, objstore, opaque)
> > > > + # let's prevent memory leak
> > > > + clean_old_tasks()
> > > > + id = str(get_next_task_id())
> > > > + tasks_queue[id] = AsyncTask(id, target_uri, fn, opaque)
> > > > return id
> > > >
> > > >
> > > > +def remove_task(id):
> > > > + try:
> > > > + del tasks_queue[id]
> > > > + except KeyError:
> > > > + cherrypy.log.error_log.error("There's no task_id %s in tasks_queue."
> > > > + " Nothing changed." % id)
> > > > +
> > > > +
> > > > +def clean_old_tasks():
> > > > + """
> > > > + Check for all tasks in tasks_queue and remove those if timestamp < than
> > > > + 10 minutes ago.
> > > > + """
> > > > + for id, task in tasks_queue.items():
> > > > + if task.timestamp < (time.time()-600):
> > > > + remove_task(id)
> > > > +
> > > > +
> > > > def is_digit(value):
> > > > if isinstance(value, int):
> > > > return True
> > > >
> > >
> > > --
> > > Lucio Correia
> > > Software Engineer
> > > IBM LTC Brazil
> > >
> > > _______________________________________________
> > > Kimchi-devel mailing list
> > > Kimchi-devel at ovirt.org
> > > http://lists.ovirt.org/mailman/listinfo/kimchi-devel
> > >
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
--
Paulo Ricardo Paz Vital
Linux Technology Center, IBM Systems
http://www.ibm.com/linux/ltc/
More information about the Kimchi-devel
mailing list