[JAVA-221] Try to remove synchronized in DBApiLayer finalize Created: 28/Nov/10 Updated: 22/Dec/10 Resolved: 13/Dec/10 |
|
| Status: | Closed |
| Project: | Java Driver |
| Component/s: | Performance |
| Affects Version/s: | None |
| Fix Version/s: | 2.4 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Eliot Horowitz (Inactive) | Assignee: | Antoine Girbal |
| Resolution: | Done | Votes: | 2 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Description |
|
See |
| Comments |
| Comment by auto [ 10/Dec/10 ] |
|
Author: {u'login': u'agirbal', u'name': u'agirbal', u'email': u'antoine@10gen.com'}Message: |
| Comment by auto [ 02/Dec/10 ] |
|
Author: {'login': 'agirbal', 'name': 'agirbal', 'email': 'antoine@10gen.com'}Message: |
| Comment by auto [ 02/Dec/10 ] |
|
Author: {'login': 'agirbal', 'name': 'agirbal', 'email': 'antoine@10gen.com'}Message: |
| Comment by auto [ 02/Dec/10 ] |
|
Author: {'login': 'agirbal', 'name': 'agirbal', 'email': 'antoine@10gen.com'}Message: |
| Comment by Eliot Horowitz (Inactive) [ 01/Dec/10 ] |
|
Once a cursor is exhausted, the server closes automatically and we should not call kill cursors on it. The reason .close() is so important is for people who do this c = coll.find().sort( ... ) while ( c.next() ) i..e they read some non-pre determined amount of data from the cursor. some people can do thousands of these per second, which can cause problems on the sever. |
| Comment by Antoine Girbal [ 01/Dec/10 ] |
|
ok just note that it will generate a lot more individual msg to servers. On another note: isnt the cursor supposed to close itself on server side once exhausted? |
| Comment by Eliot Horowitz (Inactive) [ 01/Dec/10 ] |
|
The reason why close() exists is that its easy for a client to eat a ton of resources on the server in some cases, and its much better to get them closed asap. Also its problematic if the JVM dies before the background thread cleans them up. Most people don't call close explicitly, so I think its better for it to kill the cursor immediately. Can always adjust later. |
| Comment by Antoine Girbal [ 01/Dec/10 ] |
|
basically it's a tradeoff.
Solution B:
Right now I chose solution B because it should be faster under load. |
| Comment by Eliot Horowitz (Inactive) [ 01/Dec/10 ] |
|
Yes, cursor.close() should close immediately. |
| Comment by Scott Hernandez (Inactive) [ 01/Dec/10 ] |
|
If I call close on the cursor I expect it to be closed immediately. What is the reason for not doing that? |
| Comment by Antoine Girbal [ 01/Dec/10 ] |
|
committed patch earlier, commit ebf3c729ff070ba9bc9dd862fccc5a6d42ca0982 |
| Comment by Eliot Horowitz (Inactive) [ 30/Nov/10 ] |
|
Looks good. |
| Comment by Antoine Girbal [ 29/Nov/10 ] |
|
implementation plan:
The thread will check every 1s if there are cursors to be closed, should be good enough. |
| Comment by Eliot Horowitz (Inactive) [ 29/Nov/10 ] |
|
Moving cleanCursor logic to a background thread would make sense. |
| Comment by Antoine Girbal [ 29/Nov/10 ] |
|
ok trying to understand the principle of cursor cleanup. This means that:
we can try to get rid of locks, the cleanest would be to use a background thread in charge of cleaning dead cursors. It also has the advantage of not slowing user threads for cursor cleanup. |