You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by "Randall Leeds (JIRA)" <ji...@apache.org> on 2010/08/20 09:25:16 UTC

[jira] Created: (COUCHDB-863) be quiet about dropping invalid references

be quiet about dropping invalid references
------------------------------------------

                 Key: COUCHDB-863
                 URL: https://issues.apache.org/jira/browse/COUCHDB-863
             Project: CouchDB
          Issue Type: Improvement
          Components: Database Core
    Affects Versions: 1.0.1
            Reporter: Randall Leeds
            Priority: Trivial


couch_ref_counter:drop will complain, dying with noproc, if the reference counter does not exist. Since dropping a reference to a non-existent process isn't exactly an error I think we should squelch this one. I hate log noise and I've noticed this pop up in the logs a bunch, especially running the test suite. Extra noise doesn't make debugging easier and it could confuse people trying to solve real problems.

Trivial, trivial patch unless I'm missing something really silly. I'll save everyone the extra emails from JIRA and just paste it here.

diff --git a/src/couchdb/couch_ref_counter.erl b/src/couchdb/couch_ref_counter.erl
index 5a111ab..1edc474 100644
--- a/src/couchdb/couch_ref_counter.erl
+++ b/src/couchdb/couch_ref_counter.erl
@@ -24,7 +24,9 @@ drop(RefCounterPid) ->
     drop(RefCounterPid, self()).
 
 drop(RefCounterPid, Pid) ->
-    gen_server:call(RefCounterPid, {drop, Pid}).
+    try gen_server:call(RefCounterPid, {drop, Pid})
+    catch exit:{noproc, _} -> ok
+    end.
 
 
 add(RefCounterPid) ->

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (COUCHDB-863) error log noise when a DELETE kills a replication

Posted by "Randall Leeds (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/COUCHDB-863?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Randall Leeds updated COUCHDB-863:
----------------------------------

    Attachment: 0001-couch_rep-termination-cleanup.patch

Forgot the license grant.

> error log noise when a DELETE kills a replication
> -------------------------------------------------
>
>                 Key: COUCHDB-863
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-863
>             Project: CouchDB
>          Issue Type: Improvement
>          Components: Replication
>    Affects Versions: 1.0.1
>            Reporter: Randall Leeds
>            Priority: Trivial
>         Attachments: 0001-couch_rep-termination-cleanup.patch, 0001-die-cleanly-when-local-replication-db-goes-away.patch, 0001-refactor-replication-shutdown.patch
>
>
> couch_ref_counter:drop will complain, dying with noproc, if the reference counter does not exist.
> couch_rep listens for the 'DOWN' message but then tries to terminate, closing both source and target dbs. If either one was the source of the 'DOWN' message because it was deleted, this causes loud complaints in the log and gen_server exits. We can do this more cleanly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (COUCHDB-863) error log noise when a DELETE kills a replication

Posted by "Randall Leeds (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/COUCHDB-863?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Randall Leeds updated COUCHDB-863:
----------------------------------

    Attachment:     (was: 0001-couch_rep-termination-cleanup.patch)

> error log noise when a DELETE kills a replication
> -------------------------------------------------
>
>                 Key: COUCHDB-863
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-863
>             Project: CouchDB
>          Issue Type: Improvement
>          Components: Replication
>    Affects Versions: 1.0.1
>            Reporter: Randall Leeds
>            Priority: Trivial
>         Attachments: 0001-couch_rep-termination-cleanup.patch, 0001-die-cleanly-when-local-replication-db-goes-away.patch, 0001-refactor-replication-shutdown.patch
>
>
> couch_ref_counter:drop will complain, dying with noproc, if the reference counter does not exist.
> couch_rep listens for the 'DOWN' message but then tries to terminate, closing both source and target dbs. If either one was the source of the 'DOWN' message because it was deleted, this causes loud complaints in the log and gen_server exits. We can do this more cleanly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (COUCHDB-863) be quiet about dropping invalid references

Posted by "Damien Katz (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-863?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12900706#action_12900706 ] 

Damien Katz commented on COUCHDB-863:
-------------------------------------

This sounds like a deeper bug. Under what circumstances does it attempt to drop references and it's already closed??

> be quiet about dropping invalid references
> ------------------------------------------
>
>                 Key: COUCHDB-863
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-863
>             Project: CouchDB
>          Issue Type: Improvement
>          Components: Database Core
>    Affects Versions: 1.0.1
>            Reporter: Randall Leeds
>            Priority: Trivial
>
> couch_ref_counter:drop will complain, dying with noproc, if the reference counter does not exist. Since dropping a reference to a non-existent process isn't exactly an error I think we should squelch this one. I hate log noise and I've noticed this pop up in the logs a bunch, especially running the test suite. Extra noise doesn't make debugging easier and it could confuse people trying to solve real problems.
> Trivial, trivial patch unless I'm missing something really silly. I'll save everyone the extra emails from JIRA and just paste it here.
> diff --git a/src/couchdb/couch_ref_counter.erl b/src/couchdb/couch_ref_counter.erl
> index 5a111ab..1edc474 100644
> --- a/src/couchdb/couch_ref_counter.erl
> +++ b/src/couchdb/couch_ref_counter.erl
> @@ -24,7 +24,9 @@ drop(RefCounterPid) ->
>      drop(RefCounterPid, self()).
>  
>  drop(RefCounterPid, Pid) ->
> -    gen_server:call(RefCounterPid, {drop, Pid}).
> +    try gen_server:call(RefCounterPid, {drop, Pid})
> +    catch exit:{noproc, _} -> ok
> +    end.
>  
>  
>  add(RefCounterPid) ->

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (COUCHDB-863) be quiet about dropping invalid references

Posted by "Randall Leeds (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/COUCHDB-863?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Randall Leeds updated COUCHDB-863:
----------------------------------

    Attachment: 0001-die-cleanly-when-local-replication-db-goes-away.patch

Spot on, Damien. We probably shouldn't mask strange interdependencies we create and we should instead treat them with respect and care. Here's a patch that shuts the replicator down cleanly when a local db goes away and also makes sure that if it get a spurious monitor message it complains loudly as it's murdered.

> be quiet about dropping invalid references
> ------------------------------------------
>
>                 Key: COUCHDB-863
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-863
>             Project: CouchDB
>          Issue Type: Improvement
>          Components: Database Core
>    Affects Versions: 1.0.1
>            Reporter: Randall Leeds
>            Priority: Trivial
>         Attachments: 0001-die-cleanly-when-local-replication-db-goes-away.patch
>
>
> couch_ref_counter:drop will complain, dying with noproc, if the reference counter does not exist. Since dropping a reference to a non-existent process isn't exactly an error I think we should squelch this one. I hate log noise and I've noticed this pop up in the logs a bunch, especially running the test suite. Extra noise doesn't make debugging easier and it could confuse people trying to solve real problems.
> Trivial, trivial patch unless I'm missing something really silly. I'll save everyone the extra emails from JIRA and just paste it here.
> diff --git a/src/couchdb/couch_ref_counter.erl b/src/couchdb/couch_ref_counter.erl
> index 5a111ab..1edc474 100644
> --- a/src/couchdb/couch_ref_counter.erl
> +++ b/src/couchdb/couch_ref_counter.erl
> @@ -24,7 +24,9 @@ drop(RefCounterPid) ->
>      drop(RefCounterPid, self()).
>  
>  drop(RefCounterPid, Pid) ->
> -    gen_server:call(RefCounterPid, {drop, Pid}).
> +    try gen_server:call(RefCounterPid, {drop, Pid})
> +    catch exit:{noproc, _} -> ok
> +    end.
>  
>  
>  add(RefCounterPid) ->

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (COUCHDB-863) error log noise when a DELETE kills a replication

Posted by "Randall Leeds (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/COUCHDB-863?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Randall Leeds updated COUCHDB-863:
----------------------------------

        Summary: error log noise when a DELETE kills a replication  (was: be quiet about dropping invalid references)
    Description: 
couch_ref_counter:drop will complain, dying with noproc, if the reference counter does not exist.
couch_rep listens for the 'DOWN' message but then tries to terminate, closing both source and target dbs. If either one was the source of the 'DOWN' message because it was deleted, this causes loud complaints in the log and gen_server exits. We can do this more cleanly.

  was:
couch_ref_counter:drop will complain, dying with noproc, if the reference counter does not exist. Since dropping a reference to a non-existent process isn't exactly an error I think we should squelch this one. I hate log noise and I've noticed this pop up in the logs a bunch, especially running the test suite. Extra noise doesn't make debugging easier and it could confuse people trying to solve real problems.

Trivial, trivial patch unless I'm missing something really silly. I'll save everyone the extra emails from JIRA and just paste it here.

diff --git a/src/couchdb/couch_ref_counter.erl b/src/couchdb/couch_ref_counter.erl
index 5a111ab..1edc474 100644
--- a/src/couchdb/couch_ref_counter.erl
+++ b/src/couchdb/couch_ref_counter.erl
@@ -24,7 +24,9 @@ drop(RefCounterPid) ->
     drop(RefCounterPid, self()).
 
 drop(RefCounterPid, Pid) ->
-    gen_server:call(RefCounterPid, {drop, Pid}).
+    try gen_server:call(RefCounterPid, {drop, Pid})
+    catch exit:{noproc, _} -> ok
+    end.
 
 
 add(RefCounterPid) ->

    Component/s: Replication
                     (was: Database Core)

> error log noise when a DELETE kills a replication
> -------------------------------------------------
>
>                 Key: COUCHDB-863
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-863
>             Project: CouchDB
>          Issue Type: Improvement
>          Components: Replication
>    Affects Versions: 1.0.1
>            Reporter: Randall Leeds
>            Priority: Trivial
>         Attachments: 0001-die-cleanly-when-local-replication-db-goes-away.patch
>
>
> couch_ref_counter:drop will complain, dying with noproc, if the reference counter does not exist.
> couch_rep listens for the 'DOWN' message but then tries to terminate, closing both source and target dbs. If either one was the source of the 'DOWN' message because it was deleted, this causes loud complaints in the log and gen_server exits. We can do this more cleanly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Assigned: (COUCHDB-863) error log noise when a DELETE kills a replication

Posted by "Adam Kocoloski (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/COUCHDB-863?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Adam Kocoloski reassigned COUCHDB-863:
--------------------------------------

    Assignee: Filipe Manana

Yeah, that's where I was headed.  Filipe, does it make sense to you?

> error log noise when a DELETE kills a replication
> -------------------------------------------------
>
>                 Key: COUCHDB-863
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-863
>             Project: CouchDB
>          Issue Type: Improvement
>          Components: Replication
>    Affects Versions: 1.0.1
>            Reporter: Randall Leeds
>            Assignee: Filipe Manana
>            Priority: Trivial
>         Attachments: 0001-couch_rep-termination-cleanup.patch, 0001-die-cleanly-when-local-replication-db-goes-away.patch, 0001-refactor-replication-shutdown.patch
>
>
> couch_ref_counter:drop will complain, dying with noproc, if the reference counter does not exist.
> couch_rep listens for the 'DOWN' message but then tries to terminate, closing both source and target dbs. If either one was the source of the 'DOWN' message because it was deleted, this causes loud complaints in the log and gen_server exits. We can do this more cleanly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (COUCHDB-863) error log noise when a DELETE kills a replication

Posted by "Randall Leeds (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/COUCHDB-863?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Randall Leeds updated COUCHDB-863:
----------------------------------

    Attachment: 0001-refactor-replication-shutdown.patch

Here's another loose patch I think we should wrap up and close.
I just made replication shut down a little more clear but reducing code duplication and moving some calls around. It also fixes the log noise. Info level log messages will occur if a local db shuts down during replication but there won't be any nasty backtraces about closing a db that's not open.

> error log noise when a DELETE kills a replication
> -------------------------------------------------
>
>                 Key: COUCHDB-863
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-863
>             Project: CouchDB
>          Issue Type: Improvement
>          Components: Replication
>    Affects Versions: 1.0.1
>            Reporter: Randall Leeds
>            Priority: Trivial
>         Attachments: 0001-die-cleanly-when-local-replication-db-goes-away.patch, 0001-refactor-replication-shutdown.patch
>
>
> couch_ref_counter:drop will complain, dying with noproc, if the reference counter does not exist.
> couch_rep listens for the 'DOWN' message but then tries to terminate, closing both source and target dbs. If either one was the source of the 'DOWN' message because it was deleted, this causes loud complaints in the log and gen_server exits. We can do this more cleanly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (COUCHDB-863) error log noise when a DELETE kills a replication

Posted by "Paul Joseph Davis (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/COUCHDB-863?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Paul Joseph Davis updated COUCHDB-863:
--------------------------------------

    Skill Level: Regular Contributors Level (Easy to Medium)

> error log noise when a DELETE kills a replication
> -------------------------------------------------
>
>                 Key: COUCHDB-863
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-863
>             Project: CouchDB
>          Issue Type: Improvement
>          Components: Replication
>    Affects Versions: 1.0.1
>            Reporter: Randall Leeds
>            Priority: Trivial
>         Attachments: 0001-die-cleanly-when-local-replication-db-goes-away.patch
>
>
> couch_ref_counter:drop will complain, dying with noproc, if the reference counter does not exist.
> couch_rep listens for the 'DOWN' message but then tries to terminate, closing both source and target dbs. If either one was the source of the 'DOWN' message because it was deleted, this causes loud complaints in the log and gen_server exits. We can do this more cleanly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (COUCHDB-863) error log noise when a DELETE kills a replication

Posted by "Randall Leeds (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/COUCHDB-863?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Randall Leeds updated COUCHDB-863:
----------------------------------

    Attachment: 0001-couch_rep-termination-cleanup.patch

Hey, Adam. Thanks for catching the checkpoint issue and thanks for the review.
After thinking about what you said, there really isn't any reason to explicitly close the dbs, delete the ets table, or even cancel the timer (double check me on the timer?).

So, here we go. 2 insertions. 20 deletions. Feels good, doesn't it ;).

> error log noise when a DELETE kills a replication
> -------------------------------------------------
>
>                 Key: COUCHDB-863
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-863
>             Project: CouchDB
>          Issue Type: Improvement
>          Components: Replication
>    Affects Versions: 1.0.1
>            Reporter: Randall Leeds
>            Priority: Trivial
>         Attachments: 0001-couch_rep-termination-cleanup.patch, 0001-die-cleanly-when-local-replication-db-goes-away.patch, 0001-refactor-replication-shutdown.patch
>
>
> couch_ref_counter:drop will complain, dying with noproc, if the reference counter does not exist.
> couch_rep listens for the 'DOWN' message but then tries to terminate, closing both source and target dbs. If either one was the source of the 'DOWN' message because it was deleted, this causes loud complaints in the log and gen_server exits. We can do this more cleanly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (COUCHDB-863) error log noise when a DELETE kills a replication

Posted by "Adam Kocoloski (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-863?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12922977#action_12922977 ] 

Adam Kocoloski commented on COUCHDB-863:
----------------------------------------

Hi Randall, the first part of the patch, in which the source or target is set to nil if it was the source of the 'DOWN' message, makes sense.  Although I think it could be simpler - the process is going to die, so calling close_db() for any DB is actually unnecessary, right?  The reference counters will be decremented automatically when couch_rep exits.  Same thing with deleting the ets table.

I'm having a little bit of trouble following some of the rest of the patch.  It looks like you've changed terminate(normal, State) so that it does not try to save a final checkpoint even if one was scheduled.  Was that intentional?

It looks like we're using the shutdown Reason for continuous replications.  I'm not quite sure what's going on there, but I think returning {ok, stopped} for a non-continuous replication will cause an error in the client process handling the initial _replicate request.  I realize that's not specific to your patch, it's just something else I'm confused about.

A small note - you don't need to guard a call to timer:cancel(Checkpoint).  If checkpoint is not a valid timer reference the function returns {error, badarg} but it does not crash anything.



> error log noise when a DELETE kills a replication
> -------------------------------------------------
>
>                 Key: COUCHDB-863
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-863
>             Project: CouchDB
>          Issue Type: Improvement
>          Components: Replication
>    Affects Versions: 1.0.1
>            Reporter: Randall Leeds
>            Priority: Trivial
>         Attachments: 0001-die-cleanly-when-local-replication-db-goes-away.patch, 0001-refactor-replication-shutdown.patch
>
>
> couch_ref_counter:drop will complain, dying with noproc, if the reference counter does not exist.
> couch_rep listens for the 'DOWN' message but then tries to terminate, closing both source and target dbs. If either one was the source of the 'DOWN' message because it was deleted, this causes loud complaints in the log and gen_server exits. We can do this more cleanly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.