Skip to content
This repository has been archived by the owner on Jul 22, 2019. It is now read-only.

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

Open
djc opened this issue Jul 12, 2014 · 16 comments
Open

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

djc opened this issue Jul 12, 2014 · 16 comments
Labels

Comments

@djc
Copy link
Owner

djc commented Jul 12, 2014

From [email protected] on November 18, 2011 17:49:17

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: http://code.google.com/p/couchdb-python/issues/detail?id=205

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From djc.ochtman on November 18, 2011 10:26:23

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.

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From [email protected] on November 18, 2011 11:02:48

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.

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From [email protected] on November 22, 2011 02:27:42

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.

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From [email protected] on November 22, 2011 10:37:51

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.

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From [email protected] on September 21, 2012 14:46:04

Owner: [email protected]
Labels: Milestone-0.9

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From djc.ochtman on October 22, 2012 04:26:28

Any progress on this?

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From kxepal on October 22, 2012 04:43:25

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?

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From djc.ochtman on October 22, 2012 04:51:08

Please do the refactoring in a separate patch first, then the fix after that. :)

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From kxepal on October 22, 2012 04:54:06

Got it, thanks!

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From [email protected] on October 22, 2012 08:56:27

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.

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From [email protected] on March 07, 2013 08:56:55

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

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From djc.ochtman on March 07, 2013 23:58:29

Consider it done. ;)

Labels: -Milestone-0.9

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From [email protected] on January 05, 2014 18:35:39

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)

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From [email protected] on January 05, 2014 18:58:31

Forget to mention that we are using version 0.8

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From djc.ochtman on January 05, 2014 23:51:50

You should start by upgrading to 0.9.

@djc
Copy link
Owner Author

djc commented Jul 12, 2014

From [email protected] on January 06, 2014 10:00:38

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

1 participant