Some methods in wsrep-lib still hide/ignore return codes from provider

which complicates diagnostics and debugging.

Don't ignore provider return codes and more verbose error logging for
sst_sent(), sst_received(), set_encryption_key() methods

Refs codership/wsrep-lib#127

include/wsrep/provider.hpp

@@ -186,16 +186,7 @@ namespace wsrep
 
     std::string flags_to_string(int flags);
 
-    static inline
+    std::ostream& operator<<(std::ostream& os, const wsrep::ws_meta& ws_meta);
-    std::ostream& operator<<(std::ostream& os, const wsrep::ws_meta& ws_meta)
-    {
-        os << "gtid: " << ws_meta.gtid()
-           << " server_id: " << ws_meta.server_id()
-           << " client_id: " << ws_meta.client_id()
-           << " trx_id: " << ws_meta.transaction_id()
-           << " flags: " << ws_meta.flags() << " (" << wsrep::flags_to_string(ws_meta.flags()) << ")";
-        return os;
-    }
 
     // Abstract interface for provider implementations
     class provider
@@ -250,6 +241,8 @@ namespace wsrep
             error_unknown
         };
 
+        static std::string to_string(enum status);
+
         struct flag
         {
             static const int start_transaction = (1 << 0);
@@ -385,9 +378,9 @@ namespace wsrep
          * Return last committed GTID.
          */
         virtual wsrep::gtid last_committed_gtid() const = 0;
-        virtual int sst_sent(const wsrep::gtid&, int) = 0;
+        virtual enum status sst_sent(const wsrep::gtid&, int) = 0;
-        virtual int sst_received(const wsrep::gtid&, int) = 0;
+        virtual enum status sst_received(const wsrep::gtid&, int) = 0;
-        virtual int enc_set_key(const wsrep::const_buffer& key) = 0;
+        virtual enum status enc_set_key(const wsrep::const_buffer& key) = 0;
         virtual std::vector<status_variable> status() const = 0;
         virtual void reset_status() = 0;
 

src/provider.cpp

@@ -25,7 +25,6 @@
 #include <dlfcn.h>
 #include <memory>
 
-
 wsrep::provider* wsrep::provider::make_provider(
     wsrep::server_state& server_state,
     const std::string& provider_spec,
@@ -42,7 +41,7 @@ wsrep::provider* wsrep::provider::make_provider(
         wsrep::log_error() << "Failed to create a new provider '"
                            << provider_spec << "'"
                            << " with options '" << provider_options
-                           << "':" << e.what();
+                           << "': " << e.what();
     }
     catch (...)
     {
@@ -54,6 +53,44 @@ wsrep::provider* wsrep::provider::make_provider(
     return 0;
 }
 
+std::string
+wsrep::provider::to_string(enum wsrep::provider::status const val)
+{
+    switch(val)
+    {
+    case success:
+        return "Success";
+    case error_warning:
+        return "Warning";
+    case error_transaction_missing:
+        return "Transaction not registered with provider";
+    case error_certification_failed:
+        return "Certification failed";
+    case error_bf_abort:
+        return "Transaction was BF aborted";
+    case error_size_exceeded:
+        return "Transaction size exceeded";
+    case error_connection_failed:
+        return "Not connected to Primary Component";
+    case error_provider_failed:
+        return "Provider in bad state, needs to be reinitialized.";
+    case error_fatal:
+        return "Fatal error, must abort.";
+    case error_not_implemented:
+        return "Function not implemented";
+    case error_not_allowed:
+        return "Operation not allowed";
+    case error_unknown:
+        return "Unknown error";
+    }
+
+    assert(0);
+
+    std::ostringstream os;
+    os << "Invalid error code: " << val;
+    return os.str();
+}
+
 std::string wsrep::provider::capability::str(int caps)
 {
     std::ostringstream os;
@@ -120,3 +157,14 @@ std::string wsrep::flags_to_string(int flags)
     if (ret.size() > 3) ret.erase(ret.size() - 3);
     return ret;
 }
+
+std::ostream& wsrep::operator<<(std::ostream& os, const wsrep::ws_meta& ws_meta)
+{
+    os <<  "gtid: "      << ws_meta.gtid()
+       << " server_id: " << ws_meta.server_id()
+       << " client_id: " << ws_meta.client_id()
+       << " trx_id: "    << ws_meta.transaction_id()
+       << " flags: "     << ws_meta.flags()
+       << " (" << wsrep::flags_to_string(ws_meta.flags()) << ")";
+    return os;
+}

src/server_state.cpp

@@ -684,10 +684,12 @@ void wsrep::server_state::sst_sent(const wsrep::gtid& gtid, int error)
     wsrep::unique_lock<wsrep::mutex> lock(mutex_);
     state(lock, s_joined);
     lock.unlock();
-    if (provider().sst_sent(gtid, error))
+    enum provider::status const retval(provider().sst_sent(gtid, error));
+    if (retval != provider::success)
     {
-        server_service_.log_message(wsrep::log::warning,
+        std::string msg("wsrep::sst_sent() returned an error: ");
-                                    "Provider sst_sent() returned an error");
+        msg += wsrep::provider::to_string(retval);
+        server_service_.log_message(wsrep::log::warning, msg.c_str());
     }
 }
 
@@ -776,9 +778,12 @@ void wsrep::server_state::sst_received(wsrep::client_service& cs,
         lock.unlock();
     }
 
-    if (provider().sst_received(gtid, error))
+    enum provider::status const retval(provider().sst_received(gtid, error));
+    if (retval != provider::success)
     {
-        throw wsrep::runtime_error("wsrep::sst_received() failed");
+        std::string msg("wsrep::sst_received() failed: ");
+        msg += wsrep::provider::to_string(retval);
+        throw wsrep::runtime_error(msg);
     }
 }
 
@@ -826,8 +831,15 @@ wsrep::server_state::set_encryption_key(std::vector<unsigned char>& key)
     encryption_key_ = key;
     if (state_ != s_disconnected)
     {
-        return provider_->enc_set_key(wsrep::const_buffer(encryption_key_.data(),
+        wsrep::const_buffer const key(encryption_key_.data(),
-                                                          encryption_key_.size()));
+                                      encryption_key_.size());
+        enum provider::status const retval(provider_->enc_set_key(key));
+        if (retval != provider::success)
+        {
+            wsrep::log_error() << "Failed to set encryption key: "
+                               << provider::to_string(retval);
+            return 1;
+        }
     }
     return 0;
 }

src/wsrep_provider_v26.cpp

@@ -37,6 +37,7 @@
 
 #include <iostream>
 #include <sstream>
+#include <cstring> // strerror()
 
 namespace
 {
@@ -71,6 +72,11 @@ namespace
         case WSREP_NOT_ALLOWED:
             return wsrep::provider::error_not_allowed;
         }
+
+        wsrep::log_warning() << "Unexpected value for wsrep_status_t: "
+                             << status << " ("
+                             << (status < 0 ? strerror(-status) : "") << ')';
+
         return wsrep::provider::error_unknown;
     }
 
@@ -661,9 +667,12 @@ wsrep::wsrep_provider_v26::wsrep_provider_v26(
         if (key.size())
         {
             wsrep::const_buffer const_key(key.data(), key.size());
-            if(enc_set_key(const_key))
+            enum status const retval(enc_set_key(const_key));
+            if (retval != success)
             {
-                throw wsrep::runtime_error("Failed to set encryption key");
+                std::string msg("Failed to set encryption key: ");
+                msg += to_string(retval);
+                throw wsrep::runtime_error(msg);
             }
         }
     }
@@ -941,40 +950,31 @@ wsrep::gtid wsrep::wsrep_provider_v26::last_committed_gtid() const
         wsrep::seqno(wsrep_gtid.seqno));
 }
 
-int wsrep::wsrep_provider_v26::sst_sent(const wsrep::gtid& gtid, int err)
+enum wsrep::provider::status
+wsrep::wsrep_provider_v26::sst_sent(const wsrep::gtid& gtid, int err)
 {
     wsrep_gtid_t wsrep_gtid;
     std::memcpy(wsrep_gtid.uuid.data, gtid.id().data(),
                 sizeof(wsrep_gtid.uuid.data));
     wsrep_gtid.seqno = gtid.seqno().get();
-    if (wsrep_->sst_sent(wsrep_, &wsrep_gtid, err) != WSREP_OK)
+    return map_return_value(wsrep_->sst_sent(wsrep_, &wsrep_gtid, err));
-    {
-        return 1;
-    }
-    return 0;
 }
 
-int wsrep::wsrep_provider_v26::sst_received(const wsrep::gtid& gtid, int err)
+enum wsrep::provider::status
+wsrep::wsrep_provider_v26::sst_received(const wsrep::gtid& gtid, int err)
 {
     wsrep_gtid_t wsrep_gtid;
     std::memcpy(wsrep_gtid.uuid.data, gtid.id().data(),
                 sizeof(wsrep_gtid.uuid.data));
     wsrep_gtid.seqno = gtid.seqno().get();
-    if (wsrep_->sst_received(wsrep_, &wsrep_gtid, 0, err) != WSREP_OK)
+    return map_return_value(wsrep_->sst_received(wsrep_, &wsrep_gtid, 0, err));
-    {
-        return 1;
-    }
-    return 0;
 }
 
-int wsrep::wsrep_provider_v26::enc_set_key(const wsrep::const_buffer& key)
+enum wsrep::provider::status
+wsrep::wsrep_provider_v26::enc_set_key(const wsrep::const_buffer& key)
 {
     wsrep_enc_key_t enc_key = {key.data(), key.size()};
-    if (wsrep_->enc_set_key(wsrep_, &enc_key) != WSREP_OK)
+    return map_return_value(wsrep_->enc_set_key(wsrep_, &enc_key));
-    {
-        return 1;
-    }
-    return 0;
 }
 
 std::vector<wsrep::provider::status_variable>

src/wsrep_provider_v26.hpp

@@ -82,9 +82,9 @@ namespace wsrep
         causal_read(int) const;
         enum wsrep::provider::status wait_for_gtid(const wsrep::gtid&, int) const;
         wsrep::gtid last_committed_gtid() const;
-        int sst_sent(const wsrep::gtid&,int);
+        enum wsrep::provider::status sst_sent(const wsrep::gtid&, int);
-        int sst_received(const wsrep::gtid& gtid, int);
+        enum wsrep::provider::status sst_received(const wsrep::gtid& gtid, int);
-        int enc_set_key(const wsrep::const_buffer& key);
+        enum wsrep::provider::status enc_set_key(const wsrep::const_buffer& key);
         std::vector<status_variable> status() const;
         void reset_status();
         std::string options() const;

test/mock_provider.hpp

@@ -276,11 +276,16 @@ namespace wsrep
         { return wsrep::provider::success; }
         wsrep::gtid last_committed_gtid() const WSREP_OVERRIDE
         { return wsrep::gtid(); }
-        int sst_sent(const wsrep::gtid&, int) WSREP_OVERRIDE { return 0; }
+        enum wsrep::provider::status sst_sent(const wsrep::gtid&, int)
-        int sst_received(const wsrep::gtid&, int) WSREP_OVERRIDE { return 0; }
+            WSREP_OVERRIDE
+        { return wsrep::provider::success; }
+        enum wsrep::provider::status sst_received(const wsrep::gtid&, int)
+            WSREP_OVERRIDE
+        { return wsrep::provider::success; }
 
-        int enc_set_key(const wsrep::const_buffer&) WSREP_OVERRIDE 
+        enum wsrep::provider::status enc_set_key(const wsrep::const_buffer&)
-        { return 0; }
+            WSREP_OVERRIDE
+        { return wsrep::provider::success; }
 
         std::vector<status_variable> status() const WSREP_OVERRIDE
         {