[server] remove cnxset tracking, it is now unneeded

Indeed, all cnxsets will now be acquired using the following API:

with session.new_cnx() as cnx:
     cnx.execute(...)
     # do stuff
     cnx.commit()

These well-scoped blocks are the only place a cnxset can be acquired.

The old use-case for un-delimited cnxsets (pyro/zmqpickle protocols) have been removed.

The "mode" of connection objects becomes "write" forever (it will be removed in a couple of changesets).

Related to #2919309.

authorAurelien Campeas <aurelien.campeas@logilab.fr>
changeset8b35a898b334
branchdefault
phasepublic
hiddenno
parent revision#e1ebf3d12098 [devtools] remove the remaining bw compat for old-style tests
child revision#21461f80f348 [connection] remove ensure_cnx_set context manager uses
files modified by this revision
server/session.py
server/sources/native.py
# HG changeset patch
# User Aurelien Campeas <aurelien.campeas@logilab.fr>
# Date 1402493527 -7200
# Wed Jun 11 15:32:07 2014 +0200
# Node ID 8b35a898b334930ac7fe5c55ce117cd25c5ef239
# Parent e1ebf3d120986b047e1fa84417806e1ec6c0c5c7
[server] remove cnxset tracking, it is now unneeded

Indeed, all cnxsets will now be acquired using the following API::

with session.new_cnx() as cnx:
cnx.execute(...)
# do stuff
cnx.commit()

These well-scoped blocks are the only place a cnxset can be acquired.

The old use-case for un-delimited cnxsets (pyro/zmqpickle protocols)
have been removed.

The "mode" of connection objects becomes "write" forever (it will be
removed in a couple of changesets).

Related to #2919309.

diff --git a/server/session.py b/server/session.py
@@ -17,11 +17,10 @@
1  # with CubicWeb.  If not, see <http://www.gnu.org/licenses/>.
2  """Repository users' and internal' sessions."""
3  __docformat__ = "restructuredtext en"
4 
5  import sys
6 -import threading
7  from time import time
8  from uuid import uuid4
9  from warnings import warn
10  import functools
11  from contextlib import contextmanager
@@ -169,150 +168,10 @@
12  DEFAULT_SECURITY = object() # evaluated to true by design
13 
14  class SessionClosedError(RuntimeError):
15      pass
16 
17 -class CnxSetTracker(object):
18 -    """Keep track of which connection use which cnxset.
19 -
20 -    There should be one of these objects per session (including internal sessions).
21 -
22 -    Session objects are responsible for creating their CnxSetTracker object.
23 -
24 -    Connections should use the :meth:`record` and :meth:`forget` to inform the
25 -    tracker of cnxsets they have acquired.
26 -
27 -    .. automethod:: cubicweb.server.session.CnxSetTracker.record
28 -    .. automethod:: cubicweb.server.session.CnxSetTracker.forget
29 -
30 -    Sessions use the :meth:`close` and :meth:`wait` methods when closing.
31 -
32 -    .. automethod:: cubicweb.server.session.CnxSetTracker.close
33 -    .. automethod:: cubicweb.server.session.CnxSetTracker.wait
34 -
35 -    This object itself is threadsafe. It also requires caller to acquired its
36 -    lock in some situation.
37 -    """
38 -
39 -    def __init__(self):
40 -        self._active = True
41 -        self._condition = threading.Condition()
42 -        self._record = {}
43 -
44 -    def __enter__(self):
45 -        return self._condition.__enter__()
46 -
47 -    def __exit__(self, *args):
48 -        return self._condition.__exit__(*args)
49 -
50 -    def record(self, cnxid, cnxset):
51 -        """Inform the tracker that a cnxid has acquired a cnxset
52 -
53 -        This method is to be used by Connection objects.
54 -
55 -        This method fails when:
56 -        - The cnxid already has a recorded cnxset.
57 -        - The tracker is not active anymore.
58 -
59 -        Notes about the caller:
60 -        (1) It is responsible for retrieving a cnxset.
61 -        (2) It must be prepared to release the cnxset if the
62 -            `cnxsettracker.forget` call fails.
63 -        (3) It should acquire the tracker lock until the very end of the operation.
64 -        (4) However it must only lock the CnxSetTracker object after having
65 -            retrieved the cnxset to prevent deadlock.
66 -
67 -        A typical usage look like::
68 -
69 -        cnxset = repo._get_cnxset() # (1)
70 -        try:
71 -            with cnxset_tracker: # (3) and (4)
72 -                cnxset_tracker.record(caller.id, cnxset)
73 -                # (3') operation ends when caller is in expected state only
74 -                caller.cnxset = cnxset
75 -        except Exception:
76 -            repo._free_cnxset(cnxset) # (2)
77 -            raise
78 -        """
79 -        # dubious since the caller is supposed to have acquired it anyway.
80 -        with self._condition:
81 -            if not self._active:
82 -                raise SessionClosedError('Closed')
83 -            old = self._record.get(cnxid)
84 -            if old is not None:
85 -                raise ValueError('connection "%s" already has a cnx_set (%r)'
86 -                                 % (cnxid, old))
87 -            self._record[cnxid] = cnxset
88 -
89 -    def forget(self, cnxid, cnxset):
90 -        """Inform the tracker that a cnxid have release a cnxset
91 -
92 -        This methode is to be used by Connection object.
93 -
94 -        This method fails when:
95 -        - The cnxset for the cnxid does not match the recorded one.
96 -
97 -        Notes about the caller:
98 -        (1) It is responsible for releasing the cnxset.
99 -        (2) It should acquire the tracker lock during the operation to ensure
100 -            the internal tracker state is always accurate regarding its own state.
101 -
102 -        A typical usage look like::
103 -
104 -        cnxset = caller.cnxset
105 -        try:
106 -            with cnxset_tracker:
107 -                # (2) you can not have caller.cnxset out of sync with
108 -                #     cnxset_tracker state while unlocked
109 -                caller.cnxset = None
110 -                cnxset_tracker.forget(caller.id, cnxset)
111 -        finally:
112 -            cnxset = repo._free_cnxset(cnxset) # (1)
113 -        """
114 -        with self._condition:
115 -            old = self._record.get(cnxid, None)
116 -            if old is not cnxset:
117 -                raise ValueError('recorded cnxset for "%s" mismatch: %r != %r'
118 -                                 % (cnxid, old, cnxset))
119 -            self._record.pop(cnxid)
120 -            self._condition.notify_all()
121 -
122 -    def close(self):
123 -        """Marks the tracker as inactive.
124 -
125 -        This method is to be used by Session objects.
126 -
127 -        An inactive tracker does not accept new records anymore.
128 -        """
129 -        with self._condition:
130 -            self._active = False
131 -
132 -    def wait(self, timeout=10):
133 -        """Wait for all recorded cnxsets to be released
134 -
135 -        This method is to be used by Session objects.
136 -
137 -        Returns a tuple of connection ids that remain open.
138 -        """
139 -        with self._condition:
140 -            if  self._active:
141 -                raise RuntimeError('Cannot wait on active tracker.'
142 -                                   ' Call tracker.close() first')
143 -            while self._record and timeout > 0:
144 -                start = time()
145 -                self._condition.wait(timeout)
146 -                timeout -= time() - start
147 -            return tuple(self._record)
148 -
149 -
150 -def _with_cnx_set(func):
151 -    """decorator for Connection method that ensure they run with a cnxset """
152 -    @functools.wraps(func)
153 -    def wrapper(cnx, *args, **kwargs):
154 -        with cnx.ensure_cnx_set:
155 -            return func(cnx, *args, **kwargs)
156 -    return wrapper
157 
158  def _open_only(func):
159      """decorator for Connection method that check it is open"""
160      @functools.wraps(func)
161      def check_open(cnx, *args, **kwargs):
@@ -384,15 +243,14 @@
162 
163        :attr:`read_security` and :attr:`write_security`, boolean flags telling if
164        read/write security is currently activated.
165 
166      """
167 -
168 +    mode = 'write'
169      is_request = False
170      hooks_in_progress = False
171      is_repo_in_memory = True # bw compat
172 -    mode = 'read'
173 
174      def __init__(self, session):
175          # using super(Connection, self) confuse some test hack
176          RequestSessionBase.__init__(self, session.vreg)
177          #: connection unique id
@@ -400,29 +258,19 @@
178          self.connectionid = '%s-%s' % (session.sessionid, uuid4().hex)
179          self.session = session
180          self.sessionid = session.sessionid
181          #: reentrance handling
182          self.ctx_count = 0
183 -        #: count the number of entry in a context needing a cnxset
184 -        self._cnxset_count = 0
185 -        #: Boolean for compat with the older explicite set_cnxset/free_cnx API
186 -        #: When a call set_cnxset is done, no automatic freeing will be done
187 -        #: until free_cnx is called.
188 -        self._auto_free_cnx_set = True
189 
190          #: server.Repository object
191          self.repo = session.repo
192          self.vreg = self.repo.vreg
193          self._execute = self.repo.querier.execute
194 
195          # other session utility
196          self._session_timestamp = session._timestamp
197 
198 -        #: connection set used to execute queries on sources
199 -        self._cnxset = None
200 -        #: CnxSetTracker used to report cnxset usage
201 -        self._cnxset_tracker = CnxSetTracker()
202          # internal (root) session
203          self.is_internal_session = isinstance(session.user, InternalManager)
204 
205          #: dict containing arbitrary data cleared at the end of the transaction
206          self.transaction_data = {}
@@ -538,17 +386,20 @@
207      # life cycle handling ####################################################
208 
209      def __enter__(self):
210          assert self._open is None # first opening
211          self._open = True
212 +        self.cnxset = self.repo._get_cnxset()
213          return self
214 
215      def __exit__(self, exctype=None, excvalue=None, tb=None):
216          assert self._open # actually already open
217 -        assert self._cnxset_count == 0
218 -        self.rollback()
219 +        self.clear()
220          self._open = False
221 +        self.cnxset.cnxset_freed()
222 +        self.repo._free_cnxset(self.cnxset)
223 +        self.cnxset = None
224 
225      @contextmanager
226      def running_hooks_ops(self):
227          """this context manager should be called whenever hooks or operations
228          are about to be run (but after hook selection)
@@ -602,87 +453,27 @@
229          self.commit_state = None
230          self.pruned_hooks_cache = {}
231          self.local_perm_cache.clear()
232          self.rewriter = RQLRewriter(self)
233 
234 -    # Connection Set Management ###############################################
235 -    @property
236 -    @_open_only
237 -    def cnxset(self):
238 -        return self._cnxset
239 -
240 -    @cnxset.setter
241 -    @_open_only
242 -    def cnxset(self, new_cnxset):
243 -        with self._cnxset_tracker:
244 -            old_cnxset = self._cnxset
245 -            if new_cnxset is old_cnxset:
246 -                return #nothing to do
247 -            if old_cnxset is not None:
248 -                old_cnxset.rollback()
249 -                self._cnxset = None
250 -                self.ctx_count -= 1
251 -                self._cnxset_tracker.forget(self.connectionid, old_cnxset)
252 -            if new_cnxset is not None:
253 -                self._cnxset_tracker.record(self.connectionid, new_cnxset)
254 -                self._cnxset = new_cnxset
255 -                self.ctx_count += 1
256 -
257 -    @_open_only
258 -    def _set_cnxset(self):
259 -        """the connection need a connections set to execute some queries"""
260 -        if self.cnxset is None:
261 -            cnxset = self.repo._get_cnxset()
262 -            try:
263 -                self.cnxset = cnxset
264 -            except:
265 -                self.repo._free_cnxset(cnxset)
266 -                raise
267 -        return self.cnxset
268 -
269 -    @_open_only
270 -    def _free_cnxset(self, ignoremode=False):
271 -        """the connection is no longer using its connections set, at least for some time"""
272 -        # cnxset may be none if no operation has been done since last commit
273 -        # or rollback
274 -        cnxset = self.cnxset
275 -        if cnxset is not None and (ignoremode or self.mode == 'read'):
276 -            assert self._cnxset_count == 0
277 -            try:
278 -                self.cnxset = None
279 -            finally:
280 -                cnxset.cnxset_freed()
281 -                self.repo._free_cnxset(cnxset)
282 -
283      @deprecated('[3.19] cnxset are automatically managed now.'
284                  ' stop using explicit set and free.')
285      def set_cnxset(self):
286 -        self._auto_free_cnx_set = False
287 -        return self._set_cnxset()
288 +        pass
289 
290      @deprecated('[3.19] cnxset are automatically managed now.'
291                  ' stop using explicit set and free.')
292      def free_cnxset(self, ignoremode=False):
293 -        self._auto_free_cnx_set = True
294 -        return self._free_cnxset(ignoremode=ignoremode)
295 -
296 +        pass
297 
298      @property
299      @contextmanager
300      @_open_only
301 +    @deprecated('[3.21] a cnxset is automatically set on __enter__ call now.'
302 +                ' stop using .ensure_cnx_set')
303      def ensure_cnx_set(self):
304 -        assert self._cnxset_count >= 0
305 -        if self._cnxset_count == 0:
306 -            self._set_cnxset()
307 -        try:
308 -            self._cnxset_count += 1
309 -            yield
310 -        finally:
311 -            self._cnxset_count = max(self._cnxset_count - 1, 0)
312 -            if self._cnxset_count == 0 and self._auto_free_cnx_set:
313 -                self._free_cnxset()
314 -
315 +        yield
316 
317      # Entity cache management #################################################
318      #
319      # The connection entity cache as held in cnx.transaction_data is removed at the
320      # end of the connection (commit and rollback)
@@ -990,31 +781,28 @@
321      @_open_only
322      def source_defs(self):
323          return self.repo.source_defs()
324 
325      @deprecated('[3.19] use .entity_metas(eid) instead')
326 -    @_with_cnx_set
327      @_open_only
328      def describe(self, eid, asdict=False):
329          """return a tuple (type, sourceuri, extid) for the entity with id <eid>"""
330          etype, extid, source = self.repo.type_and_source_from_eid(eid, self)
331          metas = {'type': etype, 'source': source, 'extid': extid}
332          if asdict:
333              metas['asource'] = metas['source'] # XXX pre 3.19 client compat
334              return metas
335          return etype, source, extid
336 
337 -    @_with_cnx_set
338      @_open_only
339      def entity_metas(self, eid):
340          """return a tuple (type, sourceuri, extid) for the entity with id <eid>"""
341          etype, extid, source = self.repo.type_and_source_from_eid(eid, self)
342          return {'type': etype, 'source': source, 'extid': extid}
343 
344      # core method #############################################################
345 
346 -    @_with_cnx_set
347      @_open_only
348      def execute(self, rql, kwargs=None, build_descr=True):
349          """db-api like method directly linked to the querier execute method.
350 
351          See :meth:`cubicweb.dbapi.Cursor.execute` documentation.
@@ -1024,25 +812,20 @@
352          rset.req = self
353          self._session_timestamp.touch()
354          return rset
355 
356      @_open_only
357 -    def rollback(self, free_cnxset=True, reset_pool=None):
358 +    def rollback(self, free_cnxset=None, reset_pool=None):
359          """rollback the current transaction"""
360 -        if reset_pool is not None:
361 -            warn('[3.13] use free_cnxset argument instead for reset_pool',
362 +        if free_cnxset is not None:
363 +            warn('[3.21] free_cnxset is now unneeded',
364                   DeprecationWarning, stacklevel=2)
365 -            free_cnxset = reset_pool
366 -        if self._cnxset_count != 0:
367 -            # we are inside ensure_cnx_set, don't lose it
368 -            free_cnxset = False
369 +        if reset_pool is not None:
370 +            warn('[3.13] reset_pool is now unneeded',
371 +                 DeprecationWarning, stacklevel=2)
372          cnxset = self.cnxset
373 -        if cnxset is None:
374 -            self.clear()
375 -            self._session_timestamp.touch()
376 -            self.debug('rollback transaction %s done (no db activity)', self.connectionid)
377 -            return
378 +        assert cnxset is not None
379          try:
380              # by default, operations are executed with security turned off
381              with self.security_enabled(False, False):
382                  while self.pending_operations:
383                      try:
@@ -1053,30 +836,22 @@
384                          continue
385                  cnxset.rollback()
386                  self.debug('rollback for transaction %s done', self.connectionid)
387          finally:
388              self._session_timestamp.touch()
389 -            if free_cnxset:
390 -                self._free_cnxset(ignoremode=True)
391              self.clear()
392 
393      @_open_only
394 -    def commit(self, free_cnxset=True, reset_pool=None):
395 +    def commit(self, free_cnxset=None, reset_pool=None):
396          """commit the current session's transaction"""
397 -        if reset_pool is not None:
398 -            warn('[3.13] use free_cnxset argument instead for reset_pool',
399 +        if free_cnxset is not None:
400 +            warn('[3.21] free_cnxset is now unneeded',
401                   DeprecationWarning, stacklevel=2)
402 -            free_cnxset = reset_pool
403 -        if self.cnxset is None:
404 -            assert not self.pending_operations
405 -            self.clear()
406 -            self._session_timestamp.touch()
407 -            self.debug('commit transaction %s done (no db activity)', self.connectionid)
408 -            return
409 -        if self._cnxset_count != 0:
410 -            # we are inside ensure_cnx_set, don't lose it
411 -            free_cnxset = False
412 +        if reset_pool is not None:
413 +            warn('[3.13] reset_pool is now unneeded',
414 +                 DeprecationWarning, stacklevel=2)
415 +        assert self.cnxset is not None
416          cstate = self.commit_state
417          if cstate == 'uncommitable':
418              raise QueryError('transaction must be rolled back')
419          if cstate == 'precommit':
420              self.warn('calling commit in precommit makes no sense; ignoring commit')
@@ -1132,11 +907,11 @@
421                                  self.critical('error while reverting precommit',
422                                                exc_info=True)
423                      # XXX use slice notation since self.pending_operations is a
424                      # read-only property.
425                      self.pending_operations[:] = processed + self.pending_operations
426 -                    self.rollback(free_cnxset)
427 +                    self.rollback()
428                      raise
429                  self.cnxset.commit()
430                  self.commit_state = 'postcommit'
431                  if debug:
432                      print self.commit_state, '*' * 20
@@ -1153,29 +928,23 @@
433                                            exc_info=sys.exc_info())
434                  self.debug('postcommit transaction %s done', self.connectionid)
435                  return self.transaction_uuid(set=False)
436          finally:
437              self._session_timestamp.touch()
438 -            if free_cnxset:
439 -                self._free_cnxset(ignoremode=True)
440              self.clear()
441 
442      # resource accessors ######################################################
443 
444 -    @_with_cnx_set
445      @_open_only
446      def call_service(self, regid, **kwargs):
447          self.debug('calling service %s', regid)
448          service = self.vreg['services'].select(regid, self, **kwargs)
449          return service.call(**kwargs)
450 
451 -    @_with_cnx_set
452      @_open_only
453      def system_sql(self, sql, args=None, rollback_on_failure=True):
454          """return a sql cursor on the system database"""
455 -        if sql.split(None, 1)[0].upper() != 'SELECT':
456 -            self.mode = 'write'
457          source = self.repo.system_source
458          try:
459              return source.doexec(self, sql, args, rollback=rollback_on_failure)
460          except (source.OperationalError, source.InterfaceError):
461              if not rollback_on_failure:
diff --git a/server/sources/native.py b/server/sources/native.py
@@ -1539,11 +1539,11 @@
462                                          SQL_PREFIX + 'CWUser',
463                                          SQL_PREFIX + 'upassword',
464                                          SQL_PREFIX + 'login'),
465                                         {'newhash': self.source._binary(newhash),
466                                          'login': login})
467 -                    cnx.commit(free_cnxset=False)
468 +                    cnx.commit()
469              return user
470          except IndexError:
471              raise AuthenticationError('bad password')
472 
473