Improve logging of Redis errors

diff --git a/src/ejabberd_redis.erl b/src/ejabberd_redis.erl
index 740b552f45a6eea32883d0af1af485e22ba068bc..905fe62c09b13321fd4897923b4ae6703cd96b83 100644 (file)
--- a/src/ejabberd_redis.erl
+++ b/src/ejabberd_redis.erl
@@ -31,7 +31,7 @@
 -compile({no_auto_import, [get/1, put/2]}).
 
 %% API
--export([start_link/1, get_proc/1, q/1, qp/1]).
+-export([start_link/1, get_proc/1, q/1, qp/1, format_error/1]).
 %% Commands
 -export([multi/1, get/1, set/2, del/1,
         sadd/2, srem/2, smembers/1, sismember/2, scard/1,
@@ -55,7 +55,7 @@
                num :: pos_integer(),
                pending_q :: p1_queue:queue()}).
 
--type redis_error() :: {error, binary() | timeout | disconnected}.
+-type redis_error() :: {error, binary() | timeout | disconnected | overloaded}.
 -type redis_reply() :: binary() | [binary()].
 -type redis_command() :: [binary()].
 -type redis_pipeline() :: [redis_command()].
@@ -107,6 +107,15 @@ multi(F) ->
            erlang:error(nested_transaction)
     end.
 
+-spec format_error(atom() | binary()) -> binary().
+format_error(Reason) when is_atom(Reason) ->
+    format_error(aux:atom_to_binary(Reason));
+format_error(Reason) ->
+    Reason.
+
+%%%===================================================================
+%%% Redis commands API
+%%%===================================================================
 -spec get(iodata()) -> {ok, undefined | binary()} | redis_error().
 get(Key) ->
     case erlang:get(?TR_STACK) of
@@ -408,13 +417,28 @@ call({Conn, Parent}, {F, Cmd}, Retries) ->
            try ?GEN_SERVER:call(Parent, connect, ?CALL_TIMEOUT) of
                ok -> call({Conn, Parent}, {F, Cmd}, Retries-1);
                {error, _} = Err -> Err
-           catch exit:{timeout, _} -> {error, timeout};
-                 exit:{_, {?GEN_SERVER, call, _}} -> {error, disconnected}
+           catch exit:{Why, {?GEN_SERVER, call, _}} ->
+                   Reason1 = case Why of
+                                timeout -> timeout;
+                                _ -> disconnected
+                            end,
+                   log_error(Cmd, Reason1),
+                   {error, Reason1}
            end;
+       {error, Reason1} ->
+           log_error(Cmd, Reason1),
+           Res;
        _ ->
            Res
     end.
 
+-spec log_error(redis_command() | redis_pipeline(), atom() | binary()) -> ok.
+log_error(Cmd, Reason) ->
+    ?ERROR_MSG("Redis request has failed:~n"
+              "** request = ~p~n"
+              "** response = ~s",
+              [Cmd, format_error(Reason)]).
+
 -spec get_worker() -> {atom(), atom()}.
 get_worker() ->
     Time = p1_time_compat:system_time(),
@@ -501,7 +525,7 @@ clean_queue(Q, CurrTime) ->
            ?ERROR_MSG("Redis request queue is overloaded", []),
            p1_queue:dropwhile(
              fun({From, _Time}) ->
-                     ?GEN_SERVER:reply(From, {error, disconnected}),
+                     ?GEN_SERVER:reply(From, {error, overloaded}),
                      true
              end, Q1);
        true ->

diff --git a/src/ejabberd_router_redis.erl b/src/ejabberd_router_redis.erl
index cf240a2a086de6738006ab073fa7a348c4d818db..58c1fca4ad905790d9023b55ef3a5997743812e9 100644 (file)
--- a/src/ejabberd_router_redis.erl
+++ b/src/ejabberd_router_redis.erl
@@ -51,9 +51,8 @@ register_route(Domain, ServerHost, LocalHint, _, Pid) ->
           end) of
        {ok, _} ->
            ok;
-       Err ->
-           ?ERROR_MSG("failed to register route in redis: ~p", [Err]),
-           Err
+       {error, _} ->
+           {error, db_failure}
     end.
 
 unregister_route(Domain, _, Pid) ->
@@ -76,9 +75,8 @@ unregister_route(Domain, _, Pid) ->
           true ->
                ok
        end
-    catch _:{badmatch, Err} ->
-           ?ERROR_MSG("failed to unregister route in redis: ~p", [Err]),
-           Err
+    catch _:{badmatch, {error, _}} ->
+           {error, db_failure}
     end.
 
 find_routes(Domain) ->
@@ -86,8 +84,7 @@ find_routes(Domain) ->
     case ejabberd_redis:hgetall(DomKey) of
        {ok, Vals} ->
            decode_routes(Domain, Vals);
-       Err ->
-           ?ERROR_MSG("failed to find routes in redis: ~p", [Err]),
+       {error, _} ->
            []
     end.
 
@@ -97,10 +94,7 @@ host_of_route(Domain) ->
        {ok, [{_Pid, Data}|_]} ->
            {ServerHost, _} = binary_to_term(Data),
            {ok, ServerHost};
-       {ok, []} ->
-           error;
-       Err ->
-           ?ERROR_MSG("failed to get host of route in redis: ~p", [Err]),
+       _ ->
            error
     end.
 
@@ -108,8 +102,7 @@ is_my_route(Domain) ->
     case ejabberd_redis:sismember(?ROUTES_KEY, Domain) of
        {ok, Bool} ->
            Bool;
-       Err ->
-           ?ERROR_MSG("failed to check route in redis: ~p", [Err]),
+       {error, _} ->
            false
     end.
 
@@ -120,8 +113,7 @@ get_all_routes() ->
     case ejabberd_redis:smembers(?ROUTES_KEY) of
        {ok, Routes} ->
            Routes;
-       Err ->
-           ?ERROR_MSG("failed to fetch routes from redis: ~p", [Err]),
+       {error, _} ->
            []
     end.
 

diff --git a/src/ejabberd_sm_redis.erl b/src/ejabberd_sm_redis.erl
index 8c9dc56d3924c51fda83e9ef846770d22462525e..4854bf8a6ec0fdb8975451c12da230782c77ac04 100644 (file)
--- a/src/ejabberd_sm_redis.erl
+++ b/src/ejabberd_sm_redis.erl
@@ -50,16 +50,12 @@ set_session(Session) ->
     SIDKey = sid_to_key(Session#session.sid),
     ServKey = server_to_key(element(2, Session#session.us)),
     USSIDKey = us_sid_to_key(Session#session.us, Session#session.sid),
-    case ejabberd_redis:multi(
-          fun() ->
-                  ejabberd_redis:hset(USKey, SIDKey, T),
-                  ejabberd_redis:hset(ServKey, USSIDKey, T)
-          end) of
-       {ok, _} ->
-           ok;
-       Err ->
-           ?ERROR_MSG("failed to set session for redis: ~p", [Err])
-    end.
+    ejabberd_redis:multi(
+      fun() ->
+             ejabberd_redis:hset(USKey, SIDKey, T),
+             ejabberd_redis:hset(ServKey, USSIDKey, T)
+      end),
+    ok.
 
 -spec delete_session(binary(), binary(), binary(), sid()) ->
                            {ok, #session{}} | {error, notfound}.
@@ -75,20 +71,14 @@ delete_session(LUser, LServer, _LResource, SID) ->
                    SIDKey = sid_to_key(SID),
                    ServKey = server_to_key(element(2, Session#session.us)),
                    USSIDKey = us_sid_to_key(Session#session.us, SID),
-                   case ejabberd_redis:multi(
-                          fun() ->
-                                  ejabberd_redis:hdel(USKey, [SIDKey]),
-                                  ejabberd_redis:hdel(ServKey, [USSIDKey])
-                          end) of
-                       {ok, _} ->
-                           ok;
-                       Err ->
-                           ?ERROR_MSG("failed to delete session from redis: ~p", [Err])
-                   end,
+                   ejabberd_redis:multi(
+                     fun() ->
+                             ejabberd_redis:hdel(USKey, [SIDKey]),
+                             ejabberd_redis:hdel(ServKey, [USSIDKey])
+                     end),
                    {ok, Session}
            end;
-       Err ->
-           ?ERROR_MSG("failed to delete session from redis: ~p", [Err]),
+       {error, _} ->
            {error, notfound}
     end.
 
@@ -105,8 +95,7 @@ get_sessions(LServer) ->
     case ejabberd_redis:hgetall(ServKey) of
        {ok, Vals} ->
            decode_session_list(Vals);
-       Err ->
-           ?ERROR_MSG("failed to get sessions from redis: ~p", [Err]),
+       {error, _} ->
            []
     end.
 
@@ -116,8 +105,7 @@ get_sessions(LUser, LServer) ->
     case ejabberd_redis:hgetall(USKey) of
        {ok, Vals} ->
            decode_session_list(Vals);
-       Err ->
-           ?ERROR_MSG("failed to get sessions from redis: ~p", [Err]),
+       {error, _} ->
            []
     end.
 
@@ -129,8 +117,7 @@ get_sessions(LUser, LServer, LResource) ->
        {ok, Vals} ->
            [S || S <- decode_session_list(Vals),
                  element(3, S#session.usr) == LResource];
-       Err ->
-           ?ERROR_MSG("failed to get sessions from redis: ~p", [Err]),
+       {error, _} ->
            []
     end.
 
@@ -179,8 +166,8 @@ clean_table() ->
                                  end, Vals)
                        end)
          end, ejabberd_sm:get_vh_by_backend(?MODULE))
-    catch _:{badmatch, {error, _} = Err} ->
-           ?ERROR_MSG("failed to clean redis c2s sessions: ~p", [Err])
+    catch _:{badmatch, {error, _}} ->
+           ?ERROR_MSG("failed to clean redis c2s sessions", [])
     end.
 
 opt_type(redis_connect_timeout) ->

diff --git a/src/mod_bosh_redis.erl b/src/mod_bosh_redis.erl
index ca23f72bb3baa545bedf9d2e88119a13575958f1..156df368b80fe1f71b11c7673d409b167882ee2d 100644 (file)
--- a/src/mod_bosh_redis.erl
+++ b/src/mod_bosh_redis.erl
@@ -28,18 +28,13 @@ open_session(SID, Pid) ->
     case ejabberd_redis:hset(?BOSH_KEY, SID, PidBin) of
        {ok, _} ->
            ok;
-       Err ->
-           ?ERROR_MSG("failed to register bosh session in redis: ~p", [Err]),
-           Err
+       {error, _} ->
+           {error, db_failure}
     end.
 
 close_session(SID) ->
-    case ejabberd_redis:hdel(?BOSH_KEY, [SID]) of
-       {ok, _} ->
-           ok;
-       Err ->
-           ?ERROR_MSG("failed to delete bosh session in redis: ~p", [Err])
-    end.
+    ejabberd_redis:hdel(?BOSH_KEY, [SID]),
+    ok.
 
 find_session(SID) ->
     case ejabberd_redis:hget(?BOSH_KEY, SID) of
@@ -51,10 +46,7 @@ find_session(SID) ->
                               [SID, Pid]),
                    error
            end;
-       {ok, _} ->
-           error;
-       Err ->
-           ?ERROR_MSG("failed to lookup bosh session in redis: ~p", [Err]),
+       _ ->
            error
     end.
 
@@ -65,20 +57,16 @@ clean_table() ->
     ?INFO_MSG("Cleaning Redis BOSH sessions...", []),
     case ejabberd_redis:hgetall(?BOSH_KEY) of
        {ok, Vals} ->
-           case ejabberd_redis:multi(
-                  fun() ->
-                          lists:foreach(
-                            fun({SID, Pid}) when node(Pid) == node() ->
-                                    ejabberd_redis:hdel(?BOSH_KEY, [SID]);
-                               (_) ->
-                                    ok
-                            end, Vals)
-                  end) of
-               {ok, _} ->
-                   ok;
-               Err ->
-                   ?ERROR_MSG("failed to clean bosh sessions in redis: ~p", [Err])
-           end;
-       Err ->
-           ?ERROR_MSG("failed to clean bosh sessions in redis: ~p", [Err])
+           ejabberd_redis:multi(
+             fun() ->
+                     lists:foreach(
+                       fun({SID, Pid}) when node(Pid) == node() ->
+                               ejabberd_redis:hdel(?BOSH_KEY, [SID]);
+                          (_) ->
+                               ok
+                       end, Vals)
+             end),
+           ok;
+       {error, _} ->
+           ?ERROR_MSG("failed to clean bosh sessions in redis", [])
     end.

diff --git a/src/mod_carboncopy_redis.erl b/src/mod_carboncopy_redis.erl
index 4562e68eb3c788c9f9c5ad197246cec3fe453977..8ed33468b8bd283eef6c7c597ab7aa32f7bc5ea0 100644 (file)
--- a/src/mod_carboncopy_redis.erl
+++ b/src/mod_carboncopy_redis.erl
@@ -46,9 +46,8 @@ enable(LUser, LServer, LResource, NS) ->
           end) of
        {ok, _} ->
            ok;
-       Err ->
-           ?ERROR_MSG("failed to write in redis: ~p", [Err]),
-           Err
+       {error, _} ->
+           {error, db_failure}
     end.
 
 disable(LUser, LServer, LResource) ->
@@ -62,9 +61,8 @@ disable(LUser, LServer, LResource) ->
           end) of
        {ok, _} ->
            ok;
-       Err ->
-           ?ERROR_MSG("failed to delete from redis: ~p", [Err]),
-           Err
+       {error, _} ->
+           {error, db_failure}
     end.
 
 list(LUser, LServer) ->
@@ -72,8 +70,7 @@ list(LUser, LServer) ->
     case ejabberd_redis:hgetall(USKey) of
        {ok, Vals} ->
            Vals;
-       Err ->
-           ?ERROR_MSG("failed to read from redis: ~p", [Err]),
+       {error, _} ->
            []
     end.
 
@@ -85,27 +82,20 @@ clean_table() ->
     NodeKey = node_key(),
     case ejabberd_redis:smembers(NodeKey) of
        {ok, JIDs} ->
-           case ejabberd_redis:multi(
-                  fun() ->
-                          lists:foreach(
-                            fun(JID) ->
-                                    {U, S, R} = jid:split(jid:decode(JID)),
-                                    USKey = us_key(U, S),
-                                    ejabberd_redis:hdel(USKey, [R])
-                            end, JIDs)
-                  end) of
-               {ok, _} ->
-                   ok;
-               Err ->
-                   ?ERROR_MSG("failed to delete from redis: ~p", [Err])
-           end;
-       Err ->
-           ?ERROR_MSG("failed to read from redis: ~p", [Err])
+           ejabberd_redis:multi(
+             fun() ->
+                     lists:foreach(
+                       fun(JID) ->
+                               {U, S, R} = jid:split(jid:decode(JID)),
+                               USKey = us_key(U, S),
+                               ejabberd_redis:hdel(USKey, [R])
+                       end, JIDs)
+             end);
+       {error, _} ->
+           ok
     end,
-    case ejabberd_redis:del([NodeKey]) of
-       {ok, _} -> ok;
-       Error -> ?ERROR_MSG("failed to delete from redis: ~p", [Error])
-    end.
+    ejabberd_redis:del([NodeKey]),
+    ok.
 
 us_key(LUser, LServer) ->
     <<"ejabberd:carboncopy:users:", LUser/binary, $@, LServer/binary>>.

diff --git a/src/mod_proxy65_redis.erl b/src/mod_proxy65_redis.erl
index 4086574e676323c9a7391d5766dce7d8d7f8138e..84e95906a076d9091b28c0a8d9ae6a98da93fae8 100644 (file)
--- a/src/mod_proxy65_redis.erl
+++ b/src/mod_proxy65_redis.erl
@@ -73,8 +73,7 @@ init() ->
                      ejabberd_redis:del([NodeKey])
              end),
            ok;
-       Err ->
-           ?ERROR_MSG("redis request failed: ~p", [Err]),
+       {error, _} ->
            {error, db_failure}
     end.
 
@@ -101,8 +100,7 @@ register_stream(SID, Pid) ->
                           [SIDKey, Val]),
                {error, db_failure}
        end
-    catch _:{badmatch, Err} ->
-           ?ERROR_MSG("redis request failed: ~p", [Err]),
+    catch _:{badmatch, {error, _}} ->
            {error, db_failure}
     end.
 
@@ -135,8 +133,7 @@ unregister_stream(SID) ->
                           [SIDKey, Val]),
                {error, db_failure}
        end
-    catch _:{badmatch, Err} ->
-           ?ERROR_MSG("redis request failed: ~p", [Err]),
+    catch _:{badmatch, {error, _}} ->
            {error, db_failure}
     end.
 
@@ -171,8 +168,7 @@ activate_stream(SID, IJID, MaxConnections, _Node) ->
                           [SIDKey, Val]),
                {error, db_failure}
        end
-    catch _:{badmatch, Err} ->
-           ?ERROR_MSG("redis request failed: ~p", [Err]),
+    catch _:{badmatch, {error, _}} ->
            {error, db_failure}
     end.