gcarranza / couchdb-python

Automatically exported from code.google.com/p/couchdb-python
Other
0 stars 0 forks source link

Having a well-defined way to reset the connection pool would be useful #205

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. connect to couchdb
2. fork() the python process
3. try to use the db in the child process

What is the expected output? What do you see instead?

The child gets exceptions like:

  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 1003, in rows
    self._fetch()
  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 990, in _fetch
    data = self.view._exec(self.options)
  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 880, in _exec
    _, _, data = self.resource.get_json(**self._encode_options(options))
  File "/usr/local/lib/python2.7/dist-packages/couchdb/http.py", line 394, in get_json
    if 'application/json' in headers.get('content-type'):
TypeError: argument of type 'NoneType' is not iterable

and

  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 1003, in rows
    self._fetch()
  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 990, in _fetch
    data = self.view._exec(self.options)
  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 878, in _exec
    **self._encode_options(options))
  File "/usr/local/lib/python2.7/dist-packages/couchdb/http.py", line 401, in post_json
    data = json.decode(data.read())
  File "/usr/local/lib/python2.7/dist-packages/couchdb/http.py", line 94, in read
    bytes = self.resp.read(size)
  File "/usr/lib/python2.7/httplib.py", line 541, in read
    return self._read_chunked(amt)
  File "/usr/lib/python2.7/httplib.py", line 586, in _read_chunked
    raise IncompleteRead(''.join(value))
httplib.IncompleteRead: IncompleteRead(1258 bytes read)

What version of the product are you using? On what operating system?

v0.8 on Ubuntu, python 2.7

Please provide any additional information below.

I'm able to work around the problem by including code like the following under 
0.8:

with my_db.resource.session.lock:
    my_db.resource.session.conns = {}

Trunk has a connection pool object, so it looks like that would now be:

with my_db.resource.session.connection_pool.lock:
    my_db.resource.session.connection_pool.conns = {}

Having this encapsulated in a public API function would be useful for us.  :)

Thanks!

Original issue reported on code.google.com by wickedg...@gmail.com on 18 Nov 2011 at 4:49

GoogleCodeExporter commented 9 years ago
Could we make the connection pool thread-local, or something like that? I.e. 
something that does the right thing without needing the user's cooperation.

Original comment by djc.ochtman on 18 Nov 2011 at 6:26

GoogleCodeExporter commented 9 years ago
That would be great, if something like that would work.  Threading.local won't 
do the trick by itself, though:

Python 2.7.2 (v2.7.2:8527427914a2, Jun 11 2011, 15:22:34) 
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import threading
>>> import os
>>> def f():
...   l = threading.local()
...   l.foo = 'bar'
...   pid = os.fork()
...   if pid == 0:
...     print dir(l)
... 
>>> f()
>>> ['__class__', '__delattr__', '__doc__', '__format__', '__getattribute__', 
'__hash__', '__init__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', 
'__setattr__', '__sizeof__', '__str__', '__subclasshook__', 'foo']
>>> 

Namespacing the connections by os.getpid() would probably work for the fork 
case, though.  Something along the lines of:

# Session
def __init__(self, ...):
    ...
    self._connectionPool_dict = {}
    ...

@property
def connection_pool(self):
    if os.getpid() not in self._connectionPool_dict:
        self._connectionPool_dict.clear()
        self._connectionPool_dict[os.getpid()] = ConnectionPool(...)
    return self._connectionPool_dict[os.getpid()]

Not sure what the intended behavior is WRT threading, but that could be added 
to the mix too if desired.

Original comment by wickedg...@gmail.com on 18 Nov 2011 at 7:02

GoogleCodeExporter commented 9 years ago
I have no objection to extending the connection pool - that's actually one of 
the reasons I extracted the code to the ConnectionPool class. However, isn't 
this just the common problem of forking a process with open file handles? The 
solution is generally to fork first and open files later.

So, in this particular case I think it's best to fork the process and create a 
new couchdb.Server or couchdb.Database instance in the child process for it to 
use.

Original comment by matt.goo...@gmail.com on 22 Nov 2011 at 10:27

GoogleCodeExporter commented 9 years ago
That approach presumes a lot about the application design.  For my case, the 
parts doing the forking and the parts talking to couchdb are pretty loosely 
coupled, and broadcasting "hey, we just forked; reset all of your connections 
please" wouldn't be very clean (as attested to by the hack that I put in place 
to work around this issue for the time being).  I can't not connect to couchdb 
in the parent, because the information about what/when to fork is contained 
there.

I was surprised by the current couchdb-python behavior because I don't 
typically lump open files and http requests into the same stateful bucket - 
keep-alives seem like an implementation detail, in this case.  Not having 
customers have to be aware of and account for that would be nice.

Original comment by wickedg...@gmail.com on 22 Nov 2011 at 6:37

GoogleCodeExporter commented 9 years ago

Original comment by wickedg...@gmail.com on 21 Sep 2012 at 9:46

GoogleCodeExporter commented 9 years ago
Any progress on this?

Original comment by djc.ochtman on 22 Oct 2012 at 11:26

GoogleCodeExporter commented 9 years ago
It's easy to fix, but hard to test due to there is one big god method 
Session.request. Is it allowed to apply additional refactoring during fix?

Original comment by kxepal on 22 Oct 2012 at 11:43

GoogleCodeExporter commented 9 years ago
Please do the refactoring in a separate patch first, then the fix after that. :)

Original comment by djc.ochtman on 22 Oct 2012 at 11:51

GoogleCodeExporter commented 9 years ago
Got it, thanks!

Original comment by kxepal on 22 Oct 2012 at 11:54

GoogleCodeExporter commented 9 years ago
I've been trying to reproduce a minimal test case for my original problem 
(forking), but haven't had any success in doing so against trunk.  I need to 
spin up a virtual env and try against 0.8; it's possible that the changes 
between 0.8 and now have fixed the problem already.  That would be nice, but I 
suspect that I'm just doing a poor job of simulating the conditions of my 
original problem.

Original comment by wickedg...@gmail.com on 22 Oct 2012 at 3:56

GoogleCodeExporter commented 9 years ago
We've worked around this issue in our application code.  I would like to remove 
the Milestone-0.9 label; any objections?

Original comment by wickedg...@gmail.com on 7 Mar 2013 at 4:56

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Consider it done. ;)

Original comment by djc.ochtman on 8 Mar 2013 at 7:58

GoogleCodeExporter commented 9 years ago
Would you please shed some light on how to work around this issue as we are 
encountering similar issue when accessing the designed view.

Two similar exceptions are as follows:

Traceback (most recent call last):
  File "/opt/prophetstor/federator/ussfed/common/database/couchdb/couchdb_store.py", line 216, in find_by_name
    if res and len(res) > 0:
  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 1160, in __len__
    return len(self.rows)
  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 1176, in rows
    self._fetch()
  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 1163, in _fetch
    data = self.view._exec(self.options)
  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 1029, in _exec
    _, _, data = _call_viewlike(self.resource, options)
  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 1088, in _call_viewlike
    return resource.get_json(**_encode_view_options(options))
  File "/usr/local/lib/python2.7/dist-packages/couchdb/http.py", line 523, in get_json
    return self._request_json('GET', path, headers=headers, **params)
  File "/usr/local/lib/python2.7/dist-packages/couchdb/http.py", line 547, in _request_json
    if 'application/json' in headers.get('content-type'):
TypeError: argument of type 'NoneType' is not iterable

and 

Traceback (most recent call last):
  File "/opt/prophetstor/federator/ussfed/common/database/couchdb/couchdb_store.py", line 216, in find_by_name
    if res and len(res) > 0:
  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 1160, in __len__
    return len(self.rows)
  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 1176, in rows
    self._fetch()
  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 1163, in _fetch
    data = self.view._exec(self.options)
  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 1029, in _exec
    _, _, data = _call_viewlike(self.resource, options)
  File "/usr/local/lib/python2.7/dist-packages/couchdb/client.py", line 1088, in _call_viewlike
    return resource.get_json(**_encode_view_options(options))
  File "/usr/local/lib/python2.7/dist-packages/couchdb/http.py", line 523, in get_json
    return self._request_json('GET', path, headers=headers, **params)
  File "/usr/local/lib/python2.7/dist-packages/couchdb/http.py", line 548, in _request_json
    data = json.decode(data.read())
  File "/usr/local/lib/python2.7/dist-packages/couchdb/http.py", line 175, in read
    bytes = self.resp.read(size)
  File "/usr/lib/python2.7/httplib.py", line 543, in read
    return self._read_chunked(amt)
  File "/usr/lib/python2.7/httplib.py", line 597, in _read_chunked
    raise IncompleteRead(''.join(value))
IncompleteRead: IncompleteRead(0 bytes read)

Original comment by panny.w...@gmail.com on 6 Jan 2014 at 2:35

GoogleCodeExporter commented 9 years ago
Forget to mention that we are using version 0.8

Original comment by panny.w...@gmail.com on 6 Jan 2014 at 2:58

GoogleCodeExporter commented 9 years ago
You should start by upgrading to 0.9.

Original comment by djc.ochtman on 6 Jan 2014 at 7:51

GoogleCodeExporter commented 9 years ago
We use the following properties:

    @property
    def db(self):
        if self._db_pid != os.getpid():
            self.db = couchdb.Database(self.url)

        return self._db

    @db.setter
    def db(self, value):
        self._db_pid = os.getpid()
        self._db = value

I don't recall if we put this code in place before or after we switched to 0.9. 
 I suspect it was before, but it was long enough ago that I can't say for 
certain.

Original comment by wickedg...@gmail.com on 6 Jan 2014 at 6:00

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub. Please continue discussion here:

https://github.com/djc/couchdb-python/issues/205

Original comment by djc.ochtman on 15 Jul 2014 at 7:22