Re: svn commit: r763858 - in /couchdb/trunk: share/www/script/test/ src/couchdb/

Fri, 10 Apr 2009 08:15:34 -0700

If you don't trap exits, then the default OTP behavior is to propagate linked exit messages to other child linked process, killing the entire process tree. The exception is for linked normal exits, which have no effect on non-trapping gen_server processes. They are just ignored. I think it's to allow optional manual propagation in the terminate handler, but I want it to be automatic.
Previously, when the couch_file processes receive a exit 'normal', it ignores it exit. Which is why I had it trapping exits before, so that it acted on the normal exit messages and shut down quietly. But if it got a 'kill' message, it would do the infamous process error dump instead of shutdown quietly. 


Yesterday I introduced a regression earlier because I thought the  
files were getting cleaned up after I turned off trap exits in  
couch_file, but the tests for how many os_files are open was removed,  
so I didn't notice.



Now what I've done is make it so the gen_server processes exiting  
normally will now propagate a shutdown exit to any linked processes.  
This gives the desired behavior of automatic cleanup regardless how  
the process tree terminates..



The couch_ref_counter is still required. It makes sure the file stays  
open for any DB readers even when there is a compaction. Otherwise,  
any time there was a file compaction, any readers or writers in middle  
of a transaction would get a error as they tried to use a closed file  
descriptor.



I added back in the os_files test and I think know that I understand  
how the OTP stuff is supposed to work (you must explicitly cleanup  
linked process in the normal case) it's now solid.


-Damien


On Apr 10, 2009, at 10:40 AM, Adam Kocoloski wrote:


Hi Damien, I see that the basic idea here is to terminate processes  
linked to a couch_db, couch_db_updater, couch_ref_counter,  
couch_server, or couch_view process even if the processes is doing a  
normal termination.  I have a couple of questions:



* where do the 'EXIT' messages handled by couch_file come from?  The  
process doesn't trap exits anymore.


* Is the couch_ref_counter dance still required?


* should we look into one_for_all supervision as an alternative way  
to achieve this effect?


Best, Adam

On Apr 9, 2009, at 10:21 PM, [email protected] wrote:


Author: damien
Date: Fri Apr 10 02:21:37 2009
New Revision: 763858

URL: http://svn.apache.org/viewvc?rev=763858&view=rev
Log:
Fixes for leaked file handles, with test.

Modified:
  couchdb/trunk/share/www/script/test/basics.js
  couchdb/trunk/share/www/script/test/stats.js
  couchdb/trunk/src/couchdb/couch_db.erl
  couchdb/trunk/src/couchdb/couch_db_updater.erl
  couchdb/trunk/src/couchdb/couch_file.erl
  couchdb/trunk/src/couchdb/couch_ref_counter.erl
  couchdb/trunk/src/couchdb/couch_server.erl
  couchdb/trunk/src/couchdb/couch_util.erl
  couchdb/trunk/src/couchdb/couch_view.erl
  couchdb/trunk/src/couchdb/couch_view_group.erl

Modified: couchdb/trunk/share/www/script/test/basics.js
URL: 
http://svn.apache.org/viewvc/couchdb/trunk/share/www/script/test/basics.js?rev=763858&r1=763857&r2=763858&view=diff

= 
= 
= 
= 
= 
= 
= 
= 
= 
=====================================================================

--- couchdb/trunk/share/www/script/test/basics.js (original)

+++ couchdb/trunk/share/www/script/test/basics.js Fri Apr 10  
02:21:37 2009

@@ -29,10 +29,10 @@
   if (debug) debugger;

   // creating a new DB should return Location header
-    xhr = CouchDB.request("DELETE", "/new-db");
-    xhr = CouchDB.request("PUT", "/new-db");
-    TEquals("/new-db",
-      xhr.getResponseHeader("Location").substr(-7),
+    xhr = CouchDB.request("DELETE", "/test_suite_db");
+    xhr = CouchDB.request("PUT", "/test_suite_db");
+    TEquals("/test_suite_db",
+      xhr.getResponseHeader("Location").substr(-14),
     "should return Location header to newly created document");

   TEquals("http://";,

Modified: couchdb/trunk/share/www/script/test/stats.js
URL: 
http://svn.apache.org/viewvc/couchdb/trunk/share/www/script/test/stats.js?rev=763858&r1=763857&r2=763858&view=diff

= 
= 
= 
= 
= 
= 
= 
= 
= 
=====================================================================

--- couchdb/trunk/share/www/script/test/stats.js (original)

+++ couchdb/trunk/share/www/script/test/stats.js Fri Apr 10  
02:21:37 2009

@@ -56,7 +56,8 @@
         value: max.toString()}],

       function () {

-          var files_open = requestStatsTest("couchdb",  
"open_databases").current;
+          var dbs_open = requestStatsTest("couchdb",  
"open_databases").current;
+          var files_open = requestStatsTest("couchdb",  
"open_os_files").current;

         for(var i=0; i<max+1; i++) {
           var db = new CouchDB("test_suite_db" + i);
           db.deleteDb();
@@ -70,7 +71,8 @@
           var db = new CouchDB("test_suite_db" + i);
           db.deleteDb();
         }

-          T(files_open == requestStatsTest("couchdb",  
"open_databases").current);
+          T(dbs_open == requestStatsTest("couchdb",  
"open_databases").current);
+          T(files_open == requestStatsTest("couchdb",  
"open_os_files").current);

       })
   },
};

Modified: couchdb/trunk/src/couchdb/couch_db.erl
URL: 
http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_db.erl?rev=763858&r1=763857&r2=763858&view=diff

= 
= 
= 
= 
= 
= 
= 
= 
= 
=====================================================================

--- couchdb/trunk/src/couchdb/couch_db.erl (original)
+++ couchdb/trunk/src/couchdb/couch_db.erl Fri Apr 10 02:21:37 2009
@@ -677,7 +677,8 @@
   couch_ref_counter:add(RefCntr),
   {ok, Db}.

-terminate(_Reason, _Db) ->
+terminate(Reason, _Db) ->
+    couch_util:terminate_linked(Reason),
   ok.


handle_call({open_ref_count, OpenerPid}, _,  
#db{fd_ref_counter=RefCntr}=Db) ->


Modified: couchdb/trunk/src/couchdb/couch_db_updater.erl
URL: 
http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_db_updater.erl?rev=763858&r1=763857&r2=763858&view=diff

= 
= 
= 
= 
= 
= 
= 
= 
= 
=====================================================================

--- couchdb/trunk/src/couchdb/couch_db_updater.erl (original)

+++ couchdb/trunk/src/couchdb/couch_db_updater.erl Fri Apr 10  
02:21:37 2009

@@ -37,7 +37,9 @@
   Db2 = refresh_validate_doc_funs(Db),
   {ok, Db2#db{main_pid=MainPid}}.

-terminate(_Reason, _Db) ->
+
+terminate(Reason, _Srv) ->
+    couch_util:terminate_linked(Reason),
   ok.

handle_call(get_db, _From, Db) ->

Modified: couchdb/trunk/src/couchdb/couch_file.erl
URL: 
http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_file.erl?rev=763858&r1=763857&r2=763858&view=diff

= 
= 
= 
= 
= 
= 
= 
= 
= 
=====================================================================

--- couchdb/trunk/src/couchdb/couch_file.erl (original)
+++ couchdb/trunk/src/couchdb/couch_file.erl Fri Apr 10 02:21:37 2009
@@ -311,7 +311,7 @@
   end.


-terminate(_Reason, _Fd) ->
+terminate(_Reason, _Fd) ->
   ok.

track_stats() ->
@@ -359,5 +359,5 @@
code_change(_OldVsn, State, _Extra) ->
   {ok, State}.

-handle_info(foo, _Fd) ->
-    ok.
+handle_info({'EXIT', _, Reason}, Fd) ->
+    {stop, Reason, Fd}.

Modified: couchdb/trunk/src/couchdb/couch_ref_counter.erl
URL: 
http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_ref_counter.erl?rev=763858&r1=763857&r2=763858&view=diff

= 
= 
= 
= 
= 
= 
= 
= 
= 
=====================================================================

--- couchdb/trunk/src/couchdb/couch_ref_counter.erl (original)

+++ couchdb/trunk/src/couchdb/couch_ref_counter.erl Fri Apr 10  
02:21:37 2009

@@ -49,7 +49,8 @@
   {ok, #srv{referrers=Referrers}}.


-terminate(_Reason, _Srv) ->
+terminate(Reason, _Srv) ->
+    couch_util:terminate_linked(Reason),
   ok.



Modified: couchdb/trunk/src/couchdb/couch_server.erl
URL: 
http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_server.erl?rev=763858&r1=763857&r2=763858&view=diff

= 
= 
= 
= 
= 
= 
= 
= 
= 
=====================================================================

--- couchdb/trunk/src/couchdb/couch_server.erl (original)

+++ couchdb/trunk/src/couchdb/couch_server.erl Fri Apr 10 02:21:37  
2009

@@ -159,7 +159,8 @@
               max_dbs_open=MaxDbsOpen,
               start_time=httpd_util:rfc1123_date()}}.

-terminate(_Reason, _Server) ->
+terminate(Reason, _Srv) ->
+    couch_util:terminate_linked(Reason),
   ok.

all_databases() ->

Modified: couchdb/trunk/src/couchdb/couch_util.erl
URL: 
http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_util.erl?rev=763858&r1=763857&r2=763858&view=diff

= 
= 
= 
= 
= 
= 
= 
= 
= 
=====================================================================

--- couchdb/trunk/src/couchdb/couch_util.erl (original)
+++ couchdb/trunk/src/couchdb/couch_util.erl Fri Apr 10 02:21:37 2009
@@ -12,7 +12,7 @@

-module(couch_util).

--export([start_driver/1]).
+-export([start_driver/1,terminate_linked/1]).

-export([should_flush/0, should_flush/1, to_existing_atom/1,  
to_binary/1]).

-export([new_uuid/0, rand32/0, implode/2, collate/2, collate/3]).
-export([abs_pathname/1,abs_pathname/2, trim/1, ascii_lower/1]).
@@ -43,6 +43,14 @@
   V.


+terminate_linked(normal) ->
+    terminate_linked(shutdown);
+terminate_linked(Reason) ->
+    {links, Links} = process_info(self(), links),
+    [catch exit(Pid, Reason) || Pid <- Links],
+    ok.
+
+
new_uuid() ->
   list_to_binary(to_hex(crypto:rand_bytes(16))).


Modified: couchdb/trunk/src/couchdb/couch_view.erl
URL: 
http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_view.erl?rev=763858&r1=763857&r2=763858&view=diff

= 
= 
= 
= 
= 
= 
= 
= 
= 
=====================================================================

--- couchdb/trunk/src/couchdb/couch_view.erl (original)
+++ couchdb/trunk/src/couchdb/couch_view.erl Fri Apr 10 02:21:37 2009
@@ -226,7 +226,9 @@
   process_flag(trap_exit, true),
   {ok, #server{root_dir=RootDir}}.

-terminate(_Reason,_State) ->
+
+terminate(Reason, _Srv) ->
+    couch_util:terminate_linked(Reason),
   ok.



Modified: couchdb/trunk/src/couchdb/couch_view_group.erl
URL: 
http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_view_group.erl?rev=763858&r1=763857&r2=763858&view=diff

= 
= 
= 
= 
= 
= 
= 
= 
= 
=====================================================================

--- couchdb/trunk/src/couchdb/couch_view_group.erl (original)

+++ couchdb/trunk/src/couchdb/couch_view_group.erl Fri Apr 10  
02:21:37 2009

@@ -272,6 +272,7 @@

terminate(Reason, State) ->
   reply_all(State, Reason),
+    couch_util:terminate_linked(Reason),
   ok.

code_change(_OldVsn, State, _Extra) ->




























					Reply via email to