Re: [devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-12-19 Thread Hans Nordebäck

Hi Vu,

ack, review only. I noticed Anders comments and below are some my 
additional comments.


/Thanks HansN


On 12/04/2017 04:34 PM, Vu Minh Nguyen wrote:

Make generic C++ decorator for handling SA_AIS_ERR_TRY_AGAIN return code
of AIS APIs.
---
  src/ais/Makefile.am|   5 +-
  src/ais/try_again_decorator.h  | 110 +
  src/base/Makefile.am   |   4 +-
  src/base/tests/try_again_decorator_test.cc |  69 ++
  4 files changed, 185 insertions(+), 3 deletions(-)
  create mode 100644 src/ais/try_again_decorator.h
  create mode 100644 src/base/tests/try_again_decorator_test.cc

diff --git a/src/ais/Makefile.am b/src/ais/Makefile.am
index 77ea2d8..50e4cb6 100644
--- a/src/ais/Makefile.am
+++ b/src/ais/Makefile.am
@@ -12,7 +12,7 @@
  # licensing terms.
  #
  
-CORE_INCLUDES += -I$(top_srcdir)/src/ais/include

+CORE_INCLUDES += -I$(top_srcdir)/src/ais/include -I$(top_srcdir)/src/ais
  
  EXTRA_DIST += \

src/ais/lib/libSaAmf.map \
@@ -54,7 +54,8 @@ include_HEADERS += \
src/ais/include/saMsg.h \
src/ais/include/saNtf.h \
src/ais/include/saPlm.h \
-   src/ais/include/saSmf.h
+   src/ais/include/saSmf.h \
+   src/ais/try_again_decorator.h
  
  pkgconfig_DATA += \

src/ais/lib/opensaf-amf.pc \
diff --git a/src/ais/try_again_decorator.h b/src/ais/try_again_decorator.h
new file mode 100644
index 000..19606f0
--- /dev/null
+++ b/src/ais/try_again_decorator.h
@@ -0,0 +1,110 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ */
+
+#ifndef AIS_TRY_AGAIN_DECORATOR_H_
+#define AIS_TRY_AGAIN_DECORATOR_H_
+

[HansN] #include   is missing, (implicitly included)

+#include 
+#include 
+#include 
+#include "base/time.h"
+
+namespace base {
+
+//>
+// C++ decorator which escapsulates try again handling.

[HansN] encapsulates

+//
+// E.g:
+// 1) If user wants to call saClmInitialize() which has try again
+// handling inside using this decorator, do this:
+//
+// auto saClmInitialize = base::make_decorator(::saClmInitialize);
+// if (saClmInitialize(handle, cbs, version) != SA_AIS_OK) {
+//// error handling
+// }
+//
+// 2) If user wants using other retry control policy than default ones,
+// then define your own policy like below. The best way is copy the default
+// policy, then modify to your own values.
+//
+// class MyOwnTryAgain {
+// public:
+//   constexpr static bool is_ais_code_accepted(SaAisErrorT code) {
+// return (code != SA_AIS_ERR_TRY_AGAIN && code != SA_AIS_ERR_UNAVAILABLE);
+//  }
+//
+//  public:
+//   constexpr static uint64_t interval_ms = 10;
+//   constexpr static uint64_t timeout_ms  = 10 * 1000;
+// };
+//
+// using MyOwnPolicy = base::UseMyPolicy;
+// auto saClmInitialize = MyOwnPolicy::make_decorator(::saClmInitialize);
+// if (saClmInitialize(handle, cbs, version) != SA_AIS_OK) {
+//// error handling
+// }
+//
+//<
+
[HansN] not sure I understand the second example, where is e.g 
MyOwnTryAgaing used?

+class DefaultRetryPolicy {
+ public:
+  // Which error code you want to do the retry.
+  constexpr static bool is_ais_code_accepted(SaAisErrorT code) {
+return (code != SA_AIS_ERR_TRY_AGAIN);
+  }
+
+ public:
+  // Sleep time between retries (ms)
+  constexpr static uint64_t interval_ms = 40;
+  // Timeout for the retry (ms)
+  constexpr static uint64_t timeout_ms  = 10 * 1000ull;
+};
+
+template  class Decorator;
+template 
+class Decorator {
+ public:
+  explicit Decorator(const std::function& f) : f_{f} {}
+  T operator()(Args ... args) {
+T ais_error = SA_AIS_OK;
+base::Timer wtime(Policy::timeout_ms);
+while (wtime.is_timeout() == false) {
+  ais_error = f_(args...);
+  if (Policy::is_ais_code_accepted(ais_error) == true) break;
+  base::Sleep({0, Policy::interval_ms * 1000 * 1000ull});
+}
+return ais_error;
+  }
+
+ private:
+  const std::function f_;
+};
+
+template
+Decorator make_decorator(T (*f)(Args ...)) {
+  return Decorator(std::function(f));
+}
+
[HansN] not sure I understand what is the UseMyPolicy used for? Isn't it 
enough with the make_decorator above? If user wants to

add another policy it should not be declared here?

+template 
+class UseMyPolicy {
+ public:
+  template
+  static Decorator make_decorator(T (*f)(Args ...)) {
+

Re: [devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-12-13 Thread Anders Widell

Ack with comments, marked AndersW> below.

regards,

Anders Widell


On 12/04/2017 04:34 PM, Vu Minh Nguyen wrote:

Make generic C++ decorator for handling SA_AIS_ERR_TRY_AGAIN return code
of AIS APIs.
---
  src/ais/Makefile.am|   5 +-
  src/ais/try_again_decorator.h  | 110 +
  src/base/Makefile.am   |   4 +-
  src/base/tests/try_again_decorator_test.cc |  69 ++
  4 files changed, 185 insertions(+), 3 deletions(-)
  create mode 100644 src/ais/try_again_decorator.h
  create mode 100644 src/base/tests/try_again_decorator_test.cc

diff --git a/src/ais/Makefile.am b/src/ais/Makefile.am
index 77ea2d8..50e4cb6 100644
--- a/src/ais/Makefile.am
+++ b/src/ais/Makefile.am
@@ -12,7 +12,7 @@
  # licensing terms.
  #
  
-CORE_INCLUDES += -I$(top_srcdir)/src/ais/include

+CORE_INCLUDES += -I$(top_srcdir)/src/ais/include -I$(top_srcdir)/src/ais


AndersW> Don't add src/ais to CORE_INCLUDES. Instead, you should use 
#include "ais/try_again_decorator.h" when using it.
  
  EXTRA_DIST += \

src/ais/lib/libSaAmf.map \
@@ -54,7 +54,8 @@ include_HEADERS += \
src/ais/include/saMsg.h \
src/ais/include/saNtf.h \
src/ais/include/saPlm.h \
-   src/ais/include/saSmf.h
+   src/ais/include/saSmf.h \
+   src/ais/try_again_decorator.h


AndersW> The try_again_decorator.h header file shall not be installed - 
it is for internal OpenSAF use only. So put it in noinst_HEADERS instead 
of include_HEADERS.
  
  pkgconfig_DATA += \

src/ais/lib/opensaf-amf.pc \
diff --git a/src/ais/try_again_decorator.h b/src/ais/try_again_decorator.h
new file mode 100644
index 000..19606f0
--- /dev/null
+++ b/src/ais/try_again_decorator.h
@@ -0,0 +1,110 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ */
+
+#ifndef AIS_TRY_AGAIN_DECORATOR_H_
+#define AIS_TRY_AGAIN_DECORATOR_H_
+
+#include 
+#include 
+#include 
+#include "base/time.h"


AndersW> Include file ordering shall be: C header files (saAis.h), then 
C++ header files in alphabetical order (functional, iostream) and 
finally project headers (base/time.h).

+
+namespace base {
+
+//>
+// C++ decorator which escapsulates try again handling.
+//
+// E.g:
+// 1) If user wants to call saClmInitialize() which has try again
+// handling inside using this decorator, do this:
+//
+// auto saClmInitialize = base::make_decorator(::saClmInitialize);
+// if (saClmInitialize(handle, cbs, version) != SA_AIS_OK) {
+//// error handling
+// }
+//
+// 2) If user wants using other retry control policy than default ones,
+// then define your own policy like below. The best way is copy the default
+// policy, then modify to your own values.
+//
+// class MyOwnTryAgain {
+// public:
+//   constexpr static bool is_ais_code_accepted(SaAisErrorT code) {
+// return (code != SA_AIS_ERR_TRY_AGAIN && code != SA_AIS_ERR_UNAVAILABLE);
+//  }
+//
+//  public:

AndersW> Remove public: (the second one).

+//   constexpr static uint64_t interval_ms = 10;
+//   constexpr static uint64_t timeout_ms  = 10 * 1000;
+// };
+//
+// using MyOwnPolicy = base::UseMyPolicy;
+// auto saClmInitialize = MyOwnPolicy::make_decorator(::saClmInitialize);
+// if (saClmInitialize(handle, cbs, version) != SA_AIS_OK) {
+//// error handling
+// }
+//
+//<
+
+class DefaultRetryPolicy {
+ public:
+  // Which error code you want to do the retry.
+  constexpr static bool is_ais_code_accepted(SaAisErrorT code) {
+return (code != SA_AIS_ERR_TRY_AGAIN);
+  }
+
+ public:

AndersW> Remove public: (the second one).

+  // Sleep time between retries (ms)
+  constexpr static uint64_t interval_ms = 40;
AndersW> Constants shall be named with camel case, and starting with a 
lower-case "k". I.e. kIntervalMs



+  // Timeout for the retry (ms)
+  constexpr static uint64_t timeout_ms  = 10 * 1000ull;
+};
+
+template  class Decorator;
+template 
+class Decorator {
+ public:
+  explicit Decorator(const std::function& f) : f_{f} {}
+  T operator()(Args ... args) {
+T ais_error = SA_AIS_OK;
+base::Timer wtime(Policy::timeout_ms);
+while (wtime.is_timeout() == false) {
+  ais_error = f_(args...);
+  if (Policy::is_ais_code_accepted(ais_error) == true) break;
+  base::Sleep({0, Policy::interval_ms * 1000 * 1000ull});
+}
+return ais_error;
+  }
+
+ private:
+  const std::function 

[devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-12-04 Thread Vu Minh Nguyen
Make generic C++ decorator for handling SA_AIS_ERR_TRY_AGAIN return code
of AIS APIs.
---
 src/ais/Makefile.am|   5 +-
 src/ais/try_again_decorator.h  | 110 +
 src/base/Makefile.am   |   4 +-
 src/base/tests/try_again_decorator_test.cc |  69 ++
 4 files changed, 185 insertions(+), 3 deletions(-)
 create mode 100644 src/ais/try_again_decorator.h
 create mode 100644 src/base/tests/try_again_decorator_test.cc

diff --git a/src/ais/Makefile.am b/src/ais/Makefile.am
index 77ea2d8..50e4cb6 100644
--- a/src/ais/Makefile.am
+++ b/src/ais/Makefile.am
@@ -12,7 +12,7 @@
 # licensing terms.
 #
 
-CORE_INCLUDES += -I$(top_srcdir)/src/ais/include
+CORE_INCLUDES += -I$(top_srcdir)/src/ais/include -I$(top_srcdir)/src/ais
 
 EXTRA_DIST += \
src/ais/lib/libSaAmf.map \
@@ -54,7 +54,8 @@ include_HEADERS += \
src/ais/include/saMsg.h \
src/ais/include/saNtf.h \
src/ais/include/saPlm.h \
-   src/ais/include/saSmf.h
+   src/ais/include/saSmf.h \
+   src/ais/try_again_decorator.h
 
 pkgconfig_DATA += \
src/ais/lib/opensaf-amf.pc \
diff --git a/src/ais/try_again_decorator.h b/src/ais/try_again_decorator.h
new file mode 100644
index 000..19606f0
--- /dev/null
+++ b/src/ais/try_again_decorator.h
@@ -0,0 +1,110 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ */
+
+#ifndef AIS_TRY_AGAIN_DECORATOR_H_
+#define AIS_TRY_AGAIN_DECORATOR_H_
+
+#include 
+#include 
+#include 
+#include "base/time.h"
+
+namespace base {
+
+//>
+// C++ decorator which escapsulates try again handling.
+//
+// E.g:
+// 1) If user wants to call saClmInitialize() which has try again
+// handling inside using this decorator, do this:
+//
+// auto saClmInitialize = base::make_decorator(::saClmInitialize);
+// if (saClmInitialize(handle, cbs, version) != SA_AIS_OK) {
+//// error handling
+// }
+//
+// 2) If user wants using other retry control policy than default ones,
+// then define your own policy like below. The best way is copy the default
+// policy, then modify to your own values.
+//
+// class MyOwnTryAgain {
+// public:
+//   constexpr static bool is_ais_code_accepted(SaAisErrorT code) {
+// return (code != SA_AIS_ERR_TRY_AGAIN && code != SA_AIS_ERR_UNAVAILABLE);
+//  }
+//
+//  public:
+//   constexpr static uint64_t interval_ms = 10;
+//   constexpr static uint64_t timeout_ms  = 10 * 1000;
+// };
+//
+// using MyOwnPolicy = base::UseMyPolicy;
+// auto saClmInitialize = MyOwnPolicy::make_decorator(::saClmInitialize);
+// if (saClmInitialize(handle, cbs, version) != SA_AIS_OK) {
+//// error handling
+// }
+//
+//<
+
+class DefaultRetryPolicy {
+ public:
+  // Which error code you want to do the retry.
+  constexpr static bool is_ais_code_accepted(SaAisErrorT code) {
+return (code != SA_AIS_ERR_TRY_AGAIN);
+  }
+
+ public:
+  // Sleep time between retries (ms)
+  constexpr static uint64_t interval_ms = 40;
+  // Timeout for the retry (ms)
+  constexpr static uint64_t timeout_ms  = 10 * 1000ull;
+};
+
+template  class Decorator;
+template 
+class Decorator {
+ public:
+  explicit Decorator(const std::function& f) : f_{f} {}
+  T operator()(Args ... args) {
+T ais_error = SA_AIS_OK;
+base::Timer wtime(Policy::timeout_ms);
+while (wtime.is_timeout() == false) {
+  ais_error = f_(args...);
+  if (Policy::is_ais_code_accepted(ais_error) == true) break;
+  base::Sleep({0, Policy::interval_ms * 1000 * 1000ull});
+}
+return ais_error;
+  }
+
+ private:
+  const std::function f_;
+};
+
+template
+Decorator make_decorator(T (*f)(Args ...)) {
+  return Decorator(std::function(f));
+}
+
+template 
+class UseMyPolicy {
+ public:
+  template
+  static Decorator make_decorator(T (*f)(Args ...)) {
+return Decorator(std::function(f));
+  }
+};
+}  // namespace base
+
+#endif  //< AIS_TRY_AGAIN_DECORATOR_H_
diff --git a/src/base/Makefile.am b/src/base/Makefile.am
index 956cce6..feafdde 100644
--- a/src/base/Makefile.am
+++ b/src/base/Makefile.am
@@ -208,7 +208,9 @@ bin_libbase_test_SOURCES = \
src/base/tests/time_compare_test.cc \
src/base/tests/time_convert_test.cc \
src/base/tests/time_subtract_test.cc \
-   src/base/tests/unix_socket_test.cc
+   

Re: [devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-11-30 Thread Hans Nordebäck

Hi Vu,

 I think it would be good if we can use the policy pattern for the 
retry, I reworked my previous sample a bit:



#include 
#include 
#include 
#include 
// only to illustrate, the existing SAF C functions with some dummy 
arguments

#include 
extern "C" int saImmOmInitialize(int i) {
  printf("saImmOmInitialize called, i = %d\n", i);
  return 2;
}

extern "C" int saImmOmSelectionObjectGet(int i, const char* str) {
  printf("saImmOmSelectionObjectGet called, i = %d, str = %s\n", i, str);
  return 3;
}

class DefaultRetryPolicy {
 public:
  static constexpr int kNumTries = 100;
  static constexpr int kRetryInterval = 10;
};

class AnotherRetryPolicy {
 public:
  static constexpr int kNumTries = 500;
  static constexpr int kRetryInterval = 12;
};

int SA_AIS_ERR_TRY_AGAIN = 0;
//
template  class Decorator;

template 
class Decorator {
 public:
  explicit Decorator(std::function f)
  : f_(f) {}

  T operator()(Args... args) {
    std::cout << "Call the decorated function " << std::endl;
    std::cout << "num retries: " << Policy::kNumTries << std::endl;
    std::cout << "retry interval: " << Policy::kRetryInterval << std::endl;

    int rc = f_(args...);
    int n_tries = 1;

    while (rc == SA_AIS_ERR_TRY_AGAIN &&
   n_tries < Policy::kNumTries) {
  usleep(Policy::kRetryInterval);
  rc = f_(args...);
  n_tries++;
    }

    std::cout << "return code is " << rc << std::endl;
    std::cout << "add all common retry logic here, etc " << std::endl;
    return rc;
  }
  std::function f_;
};


template
Decorator make_decorator(T (*f)(Args...)) {
   return Decorator(std::function(f));
}

template
Decorator make_another_decorator(T 
(*f)(Args...)) {
  return Decorator(std::function(f));

}

int main() {


  int rc = 0;
    auto saImmOmInitialize_d = make_decorator(saImmOmInitialize);
    std::cout << typeid(saImmOmInitialize_d).name() << std::endl;
    rc = saImmOmInitialize_d(4711);
    std::cout << "rc: " << rc << std::endl;
    auto saImmOmSelectionObjectGet_d = 
make_another_decorator(saImmOmSelectionObjectGet);

    rc = saImmOmSelectionObjectGet_d(4712, "hello world!");
    std::cout << "rc: " << rc << std::endl;

    return 0;
}

/Regards HansN


On 11/30/2017 04:23 PM, Vu Minh Nguyen wrote:

Make generic C++ decorator for handling SA_AIS_ERR_TRY_AGAIN return code
of AIS APIs.
---
  src/ais/Makefile.am|   5 +-
  src/ais/try_again_decorator.h  | 111 +
  src/base/Makefile.am   |   4 +-
  src/base/tests/try_again_decorator_test.cc |  54 ++
  4 files changed, 171 insertions(+), 3 deletions(-)
  create mode 100644 src/ais/try_again_decorator.h
  create mode 100644 src/base/tests/try_again_decorator_test.cc

diff --git a/src/ais/Makefile.am b/src/ais/Makefile.am
index 77ea2d8..50e4cb6 100644
--- a/src/ais/Makefile.am
+++ b/src/ais/Makefile.am
@@ -12,7 +12,7 @@
  # licensing terms.
  #
  
-CORE_INCLUDES += -I$(top_srcdir)/src/ais/include

+CORE_INCLUDES += -I$(top_srcdir)/src/ais/include -I$(top_srcdir)/src/ais
  
  EXTRA_DIST += \

src/ais/lib/libSaAmf.map \
@@ -54,7 +54,8 @@ include_HEADERS += \
src/ais/include/saMsg.h \
src/ais/include/saNtf.h \
src/ais/include/saPlm.h \
-   src/ais/include/saSmf.h
+   src/ais/include/saSmf.h \
+   src/ais/try_again_decorator.h
  
  pkgconfig_DATA += \

src/ais/lib/opensaf-amf.pc \
diff --git a/src/ais/try_again_decorator.h b/src/ais/try_again_decorator.h
new file mode 100644
index 000..a7967e9
--- /dev/null
+++ b/src/ais/try_again_decorator.h
@@ -0,0 +1,111 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ */
+
+#ifndef AIS_TRY_AGAIN_DECORATOR_H_
+#define AIS_TRY_AGAIN_DECORATOR_H_
+
+#include 
+#include 
+#include 
+#include "base/time.h"
+
+namespace base {
+
+//>
+// C++ decorator which escapsulates try again handling.
+//
+// E.g:
+// 1) If user wants to call saClmInitialize() which has try again
+// handling inside using this decorator, do this:
+//
+// auto saClmInitialize = base::TryAgainDecorator(::saClmInitialize);
+// if (saClmInitialize(handle, cbs, version) != SA_AIS_OK) {
+//// error handling
+// }
+//
+// 2) If user wants using other retry control than default ones,

Re: [devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-11-30 Thread Vu Minh Nguyen
Thanks Hans and Anders for your good comments.

The updated patch is just sent out. Please help to have a look. Thanks a lot!

Regards, Vu

> -Original Message-
> From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
> Sent: Thursday, November 30, 2017 1:07 PM
> To: Anders Widell ; Vu Minh Nguyen
> 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 1/1] base: create generic try-again handling decorator for
> AIS APIs [#2702]
> 
> agree, constexpr should be fine. /HansN
> 
> -Original Message-
> From: Anders Widell
> Sent: den 30 november 2017 13:05
> To: Hans Nordebäck ; Vu Minh Nguyen
> 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1/1] base: create generic try-again handling decorator for
> AIS APIs [#2702]
> 
> I have two comments:
> 
> 1) I think this should be put under src/ais (but not src/ais/include) instead 
> of
> putting it in src/base.
> 
> 2) Could you think about if it is possible to make the retry time and interval
> constexpr?
> 
> regards,
> 
> Anders Widell
> 
> 
> On 11/30/2017 01:01 PM, Hans Nordebäck wrote:
> > Hi Vu,
> >
> > please see some comments inlined below/Regards HansN
> >
> >
> > On 11/28/2017 02:13 PM, Vu Minh Nguyen wrote:
> >> Make generic C++ decorator for handling SA_AIS_ERR_TRY_AGAIN return
> >> code of AIS APIs.
> >> ---
> >>   src/base/Makefile.am   |   5 +-
> >>   src/base/tests/try_again_decorator_test.cc |  48 +
> >>   src/base/try_again_decorator.h | 109
> >> +
> >>   3 files changed, 161 insertions(+), 1 deletion(-)
> >>   create mode 100644 src/base/tests/try_again_decorator_test.cc
> >>   create mode 100644 src/base/try_again_decorator.h
> >>
> >> diff --git a/src/base/Makefile.am b/src/base/Makefile.am index
> >> 956cce6..f11b738 100644
> >> --- a/src/base/Makefile.am
> >> +++ b/src/base/Makefile.am
> >> @@ -150,6 +150,7 @@ noinst_HEADERS += \
> >>   src/base/unix_client_socket.h \
> >>   src/base/unix_server_socket.h \
> >>   src/base/unix_socket.h \
> >> +src/base/try_again_decorator.h \
> >>   src/base/usrbuf.h
> >> TESTS += bin/testleap bin/libbase_test bin/core_common_test @@
> >> -208,7 +209,9 @@ bin_libbase_test_SOURCES = \
> >>   src/base/tests/time_compare_test.cc \
> >>   src/base/tests/time_convert_test.cc \
> >>   src/base/tests/time_subtract_test.cc \
> >> -src/base/tests/unix_socket_test.cc
> >> +src/base/tests/unix_socket_test.cc \
> >> +src/base/tests/try_again_decorator_test.cc
> >> +
> >> bin_libbase_test_LDADD = \
> >>   $(GTEST_DIR)/lib/libgtest.la \
> >> diff --git a/src/base/tests/try_again_decorator_test.cc
> >> b/src/base/tests/try_again_decorator_test.cc
> >> new file mode 100644
> >> index 000..ac8595d
> >> --- /dev/null
> >> +++ b/src/base/tests/try_again_decorator_test.cc
> >> @@ -0,0 +1,48 @@
> >> +/*  -*- OpenSAF  -*-
> >> + *
> >> + * (C) Copyright 2017 The OpenSAF Foundation
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> +but
> >> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> >> MERCHANTABILITY
> >> + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are
> >> licensed
> >> + * under the GNU Lesser General Public License Version 2.1, February
> >> 1999.
> >> + * The complete license can be accessed from the following location:
> >> + * http://opensource.org/licenses/lgpl-license.php
> >> + * See the Copying file included with the OpenSAF distribution for
> >> + full
> >> + * licensing terms.
> >> + *
> >> + * Author(s): Ericsson AB
> >> + *
> >> + */
> >> +
> >> +#include "base/try_again_decorator.h"
> >> +#include "gtest/gtest.h"
> >> +
> >> +static unsigned counter = 0;
> > [HansN] use an unnamed namespace instead of static:
> > namespace {
> >   unsigned counter = 0;
> > }
> >
> >> +
> >> +SaAisErrorT TestMethod() {
> >> +  ++counter;
> >> +  return SA_AIS_ERR_TRY_AGAIN;
> >> +}
> > [HansN] shouldn't this function be declared extern "C" ?
> >
> >> +
> >> +TEST(TryAgainDecorator, DefaultRetryControl) {
> >> +  // default interval = 40ms, timeout = 10 * 1000ms
> >> +  using DefaultDecorator = base::TryAgainDecorator<>;
> >> +  auto DecorTestMethod =
> >> DefaultDecorator::DecorateFunction(TestMethod);
> >> +
> >> +  DecorTestMethod();
> >
> > [HansN] the return code from the DecorTestMethod should be validated
> >> +  EXPECT_GE(counter, 200);
> >> +  EXPECT_LE(counter, 250);
> >> +  counter = 0;
> >> +}
> >> +
> >> +TEST(TryAgainDecorator, GivenRetryControl) {
> >> +  // interval = 10ms, timeout = 1000ms
> >> +  using TryAgainDecorator = base::TryAgainDecorator<10, 1 * 1000>;
> >> +  auto DecorTestMethod  =
> >> TryAgainDecorator::DecorateFunction(TestMethod);
> >> +
> >> +  DecorTestMethod();
> >
> > [HansN] the return code from the DecorTestMethod should be 

[devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-11-30 Thread Vu Minh Nguyen
Make generic C++ decorator for handling SA_AIS_ERR_TRY_AGAIN return code
of AIS APIs.
---
 src/ais/Makefile.am|   5 +-
 src/ais/try_again_decorator.h  | 111 +
 src/base/Makefile.am   |   4 +-
 src/base/tests/try_again_decorator_test.cc |  54 ++
 4 files changed, 171 insertions(+), 3 deletions(-)
 create mode 100644 src/ais/try_again_decorator.h
 create mode 100644 src/base/tests/try_again_decorator_test.cc

diff --git a/src/ais/Makefile.am b/src/ais/Makefile.am
index 77ea2d8..50e4cb6 100644
--- a/src/ais/Makefile.am
+++ b/src/ais/Makefile.am
@@ -12,7 +12,7 @@
 # licensing terms.
 #
 
-CORE_INCLUDES += -I$(top_srcdir)/src/ais/include
+CORE_INCLUDES += -I$(top_srcdir)/src/ais/include -I$(top_srcdir)/src/ais
 
 EXTRA_DIST += \
src/ais/lib/libSaAmf.map \
@@ -54,7 +54,8 @@ include_HEADERS += \
src/ais/include/saMsg.h \
src/ais/include/saNtf.h \
src/ais/include/saPlm.h \
-   src/ais/include/saSmf.h
+   src/ais/include/saSmf.h \
+   src/ais/try_again_decorator.h
 
 pkgconfig_DATA += \
src/ais/lib/opensaf-amf.pc \
diff --git a/src/ais/try_again_decorator.h b/src/ais/try_again_decorator.h
new file mode 100644
index 000..a7967e9
--- /dev/null
+++ b/src/ais/try_again_decorator.h
@@ -0,0 +1,111 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ */
+
+#ifndef AIS_TRY_AGAIN_DECORATOR_H_
+#define AIS_TRY_AGAIN_DECORATOR_H_
+
+#include 
+#include 
+#include 
+#include "base/time.h"
+
+namespace base {
+
+//>
+// C++ decorator which escapsulates try again handling.
+//
+// E.g:
+// 1) If user wants to call saClmInitialize() which has try again
+// handling inside using this decorator, do this:
+//
+// auto saClmInitialize = base::TryAgainDecorator(::saClmInitialize);
+// if (saClmInitialize(handle, cbs, version) != SA_AIS_OK) {
+//// error handling
+// }
+//
+// 2) If user wants using other retry control than default ones,
+// such as interval = 10ms, timeout = 10second, then do:
+//
+// class MyOwnTryAgain : public base::RetryControl {
+//  public:
+//   constexpr uint64_t interval_ms() const { return 10;  }
+//   constexpr uint64_t timeout_ms()  const { return 10 * 1000; }
+// };
+//
+// const MyOwnTryAgain my_own_retry {};
+// auto saClmInitialize = TryAgainDecorator(::saClmInitialize, my_own_retry);
+// if (saClmInitialize(handle, cbs, version) != SA_AIS_OK) {
+//// error handling
+// }
+//
+//<
+
+// Default sleep time b/w retries (ms)
+static const uint64_t kInterval = 40ull;
+// Default maximum time for retry loop (ms)
+static const uint64_t kTimeout  = 10 * 1000ull;
+
+class RetryControl {
+ public:
+  virtual uint64_t interval_ms() const = 0;
+  virtual uint64_t timeout_ms()  const = 0;
+};
+
+class DefaultRetryControl : public RetryControl {
+ public:
+  constexpr uint64_t interval_ms() const { return  kInterval; }
+  constexpr uint64_t timeout_ms()  const { return kTimeout;  }
+};
+
+template  class Decorator;
+template 
+class Decorator {
+ public:
+  explicit Decorator(const std::function& f,
+ const RetryControl& ctrl)
+  : f_{f},
+timeout_ms_{ctrl.timeout_ms()},
+interval_ms_{ctrl.interval_ms()} {}
+
+  T operator()(Args ... args) {
+T ais_error;
+timespec interval;
+interval.tv_sec = 0;
+interval.tv_nsec = (interval_ms_ * 1000 * 1000);
+
+base::Timer wtime(timeout_ms_);
+while (wtime.is_timeout() == false) {
+  ais_error = f_(args...);
+  if (ais_error != SA_AIS_ERR_TRY_AGAIN) break;
+  base::Sleep(interval);
+}
+return ais_error;
+  }
+
+ private:
+  const std::function f_;
+  const uint64_t timeout_ms_;
+  const uint64_t interval_ms_;
+};
+
+const static DefaultRetryControl default_retry {}; // NOLINT(*)
+template Decorator
+TryAgainDecorator(T (*f)(Args ...), const RetryControl& ctrl = default_retry) {
+  return Decorator(std::function(f), ctrl);
+}
+
+}  // namespace base
+
+#endif  //< AIS_TRY_AGAIN_DECORATOR_H_
diff --git a/src/base/Makefile.am b/src/base/Makefile.am
index 956cce6..feafdde 100644
--- a/src/base/Makefile.am
+++ b/src/base/Makefile.am
@@ -208,7 +208,9 @@ bin_libbase_test_SOURCES = \
src/base/tests/time_compare_test.cc \
src/base/tests/time_convert_test.cc \
src/base/tests/time_subtract_test.cc \

Re: [devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-11-30 Thread Hans Nordebäck
agree, constexpr should be fine. /HansN

-Original Message-
From: Anders Widell 
Sent: den 30 november 2017 13:05
To: Hans Nordebäck ; Vu Minh Nguyen 

Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] base: create generic try-again handling decorator for 
AIS APIs [#2702]

I have two comments:

1) I think this should be put under src/ais (but not src/ais/include) instead 
of putting it in src/base.

2) Could you think about if it is possible to make the retry time and interval 
constexpr?

regards,

Anders Widell


On 11/30/2017 01:01 PM, Hans Nordebäck wrote:
> Hi Vu,
>
> please see some comments inlined below/Regards HansN
>
>
> On 11/28/2017 02:13 PM, Vu Minh Nguyen wrote:
>> Make generic C++ decorator for handling SA_AIS_ERR_TRY_AGAIN return 
>> code of AIS APIs.
>> ---
>>   src/base/Makefile.am   |   5 +-
>>   src/base/tests/try_again_decorator_test.cc |  48 +
>>   src/base/try_again_decorator.h | 109
>> +
>>   3 files changed, 161 insertions(+), 1 deletion(-)
>>   create mode 100644 src/base/tests/try_again_decorator_test.cc
>>   create mode 100644 src/base/try_again_decorator.h
>>
>> diff --git a/src/base/Makefile.am b/src/base/Makefile.am index 
>> 956cce6..f11b738 100644
>> --- a/src/base/Makefile.am
>> +++ b/src/base/Makefile.am
>> @@ -150,6 +150,7 @@ noinst_HEADERS += \
>>   src/base/unix_client_socket.h \
>>   src/base/unix_server_socket.h \
>>   src/base/unix_socket.h \
>> +    src/base/try_again_decorator.h \
>>   src/base/usrbuf.h
>>     TESTS += bin/testleap bin/libbase_test bin/core_common_test @@ 
>> -208,7 +209,9 @@ bin_libbase_test_SOURCES = \
>>   src/base/tests/time_compare_test.cc \
>>   src/base/tests/time_convert_test.cc \
>>   src/base/tests/time_subtract_test.cc \
>> -    src/base/tests/unix_socket_test.cc
>> +    src/base/tests/unix_socket_test.cc \
>> +    src/base/tests/try_again_decorator_test.cc
>> +
>>     bin_libbase_test_LDADD = \
>>   $(GTEST_DIR)/lib/libgtest.la \
>> diff --git a/src/base/tests/try_again_decorator_test.cc
>> b/src/base/tests/try_again_decorator_test.cc
>> new file mode 100644
>> index 000..ac8595d
>> --- /dev/null
>> +++ b/src/base/tests/try_again_decorator_test.cc
>> @@ -0,0 +1,48 @@
>> +/*  -*- OpenSAF  -*-
>> + *
>> + * (C) Copyright 2017 The OpenSAF Foundation
>> + *
>> + * This program is distributed in the hope that it will be useful, 
>> +but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> MERCHANTABILITY
>> + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are
>> licensed
>> + * under the GNU Lesser General Public License Version 2.1, February
>> 1999.
>> + * The complete license can be accessed from the following location:
>> + * http://opensource.org/licenses/lgpl-license.php
>> + * See the Copying file included with the OpenSAF distribution for 
>> + full
>> + * licensing terms.
>> + *
>> + * Author(s): Ericsson AB
>> + *
>> + */
>> +
>> +#include "base/try_again_decorator.h"
>> +#include "gtest/gtest.h"
>> +
>> +static unsigned counter = 0;
> [HansN] use an unnamed namespace instead of static:
> namespace {
>   unsigned counter = 0;
> }
>
>> +
>> +SaAisErrorT TestMethod() {
>> +  ++counter;
>> +  return SA_AIS_ERR_TRY_AGAIN;
>> +}
> [HansN] shouldn't this function be declared extern "C" ?
>
>> +
>> +TEST(TryAgainDecorator, DefaultRetryControl) {
>> +  // default interval = 40ms, timeout = 10 * 1000ms
>> +  using DefaultDecorator = base::TryAgainDecorator<>;
>> +  auto DecorTestMethod =
>> DefaultDecorator::DecorateFunction(TestMethod);
>> +
>> +  DecorTestMethod();
>
> [HansN] the return code from the DecorTestMethod should be validated
>> +  EXPECT_GE(counter, 200);
>> +  EXPECT_LE(counter, 250);
>> +  counter = 0;
>> +}
>> +
>> +TEST(TryAgainDecorator, GivenRetryControl) {
>> +  // interval = 10ms, timeout = 1000ms
>> +  using TryAgainDecorator = base::TryAgainDecorator<10, 1 * 1000>;
>> +  auto DecorTestMethod  =
>> TryAgainDecorator::DecorateFunction(TestMethod);
>> +
>> +  DecorTestMethod();
>
> [HansN] the return code from the DecorTestMethod should be validated
>> +  EXPECT_GE(counter, 50);
>> +  EXPECT_LE(counter, 100);
>> +  counter = 0;
>> +}
>> diff --git a/src/base/try_again_decorator.h 
>> b/src/base/try_again_decorator.h new file mode 100644 index 
>> 000..e676697
>> --- /dev/null
>> +++ b/src/base/try_again_decorator.h
>> @@ -0,0 +1,109 @@
>> +/*  -*- OpenSAF  -*-
>> + *
>> + * Copyright Ericsson AB 2017 - All Rights Reserved.
>> + *
>> + * This program is distributed in the hope that it will be useful, 
>> +but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> MERCHANTABILITY
>> + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are
>> licensed
>> + * under the GNU Lesser General Public License Version 2.1, February
>> 1999.
>> + * The complete license can be 

Re: [devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-11-30 Thread Anders Widell

I have two comments:

1) I think this should be put under src/ais (but not src/ais/include) 
instead of putting it in src/base.


2) Could you think about if it is possible to make the retry time and 
interval constexpr?


regards,

Anders Widell


On 11/30/2017 01:01 PM, Hans Nordebäck wrote:

Hi Vu,

please see some comments inlined below/Regards HansN


On 11/28/2017 02:13 PM, Vu Minh Nguyen wrote:

Make generic C++ decorator for handling SA_AIS_ERR_TRY_AGAIN return code
of AIS APIs.
---
  src/base/Makefile.am   |   5 +-
  src/base/tests/try_again_decorator_test.cc |  48 +
  src/base/try_again_decorator.h | 109 
+

  3 files changed, 161 insertions(+), 1 deletion(-)
  create mode 100644 src/base/tests/try_again_decorator_test.cc
  create mode 100644 src/base/try_again_decorator.h

diff --git a/src/base/Makefile.am b/src/base/Makefile.am
index 956cce6..f11b738 100644
--- a/src/base/Makefile.am
+++ b/src/base/Makefile.am
@@ -150,6 +150,7 @@ noinst_HEADERS += \
  src/base/unix_client_socket.h \
  src/base/unix_server_socket.h \
  src/base/unix_socket.h \
+    src/base/try_again_decorator.h \
  src/base/usrbuf.h
    TESTS += bin/testleap bin/libbase_test bin/core_common_test
@@ -208,7 +209,9 @@ bin_libbase_test_SOURCES = \
  src/base/tests/time_compare_test.cc \
  src/base/tests/time_convert_test.cc \
  src/base/tests/time_subtract_test.cc \
-    src/base/tests/unix_socket_test.cc
+    src/base/tests/unix_socket_test.cc \
+    src/base/tests/try_again_decorator_test.cc
+
    bin_libbase_test_LDADD = \
  $(GTEST_DIR)/lib/libgtest.la \
diff --git a/src/base/tests/try_again_decorator_test.cc 
b/src/base/tests/try_again_decorator_test.cc

new file mode 100644
index 000..ac8595d
--- /dev/null
+++ b/src/base/tests/try_again_decorator_test.cc
@@ -0,0 +1,48 @@
+/*  -*- OpenSAF  -*-
+ *
+ * (C) Copyright 2017 The OpenSAF Foundation
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of 
MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are 
licensed
+ * under the GNU Lesser General Public License Version 2.1, February 
1999.

+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ * Author(s): Ericsson AB
+ *
+ */
+
+#include "base/try_again_decorator.h"
+#include "gtest/gtest.h"
+
+static unsigned counter = 0;

[HansN] use an unnamed namespace instead of static:
namespace {
  unsigned counter = 0;
}


+
+SaAisErrorT TestMethod() {
+  ++counter;
+  return SA_AIS_ERR_TRY_AGAIN;
+}

[HansN] shouldn't this function be declared extern "C" ?


+
+TEST(TryAgainDecorator, DefaultRetryControl) {
+  // default interval = 40ms, timeout = 10 * 1000ms
+  using DefaultDecorator = base::TryAgainDecorator<>;
+  auto DecorTestMethod = 
DefaultDecorator::DecorateFunction(TestMethod);

+
+  DecorTestMethod();


[HansN] the return code from the DecorTestMethod should be validated

+  EXPECT_GE(counter, 200);
+  EXPECT_LE(counter, 250);
+  counter = 0;
+}
+
+TEST(TryAgainDecorator, GivenRetryControl) {
+  // interval = 10ms, timeout = 1000ms
+  using TryAgainDecorator = base::TryAgainDecorator<10, 1 * 1000>;
+  auto DecorTestMethod  = 
TryAgainDecorator::DecorateFunction(TestMethod);

+
+  DecorTestMethod();


[HansN] the return code from the DecorTestMethod should be validated

+  EXPECT_GE(counter, 50);
+  EXPECT_LE(counter, 100);
+  counter = 0;
+}
diff --git a/src/base/try_again_decorator.h 
b/src/base/try_again_decorator.h

new file mode 100644
index 000..e676697
--- /dev/null
+++ b/src/base/try_again_decorator.h
@@ -0,0 +1,109 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of 
MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are 
licensed
+ * under the GNU Lesser General Public License Version 2.1, February 
1999.

+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ */
+
+#ifndef BASE_TRY_AGAIN_DECORATOR_H_
+#define BASE_TRY_AGAIN_DECORATOR_H_
+
+#include 
+#include 
+#include 
+#include "base/time.h"
+
+namespace base {
+
[HansN] perhaps we can use a strategy/policy like pattern instead? 
What do you say about something like the following:



struct RetryStrategy {

  virtual int nTries() = 0;

  virtual int retryInterval() = 0;

};

struct DefaultRetryStrategy : public RetryStrategy {

  int nTries() {return 10;}

  int retryInterval() {return 10;}

};

struct 

Re: [devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-11-30 Thread Hans Nordebäck

Hi Vu,

please see some comments inlined below/Regards HansN


On 11/28/2017 02:13 PM, Vu Minh Nguyen wrote:

Make generic C++ decorator for handling SA_AIS_ERR_TRY_AGAIN return code
of AIS APIs.
---
  src/base/Makefile.am   |   5 +-
  src/base/tests/try_again_decorator_test.cc |  48 +
  src/base/try_again_decorator.h | 109 +
  3 files changed, 161 insertions(+), 1 deletion(-)
  create mode 100644 src/base/tests/try_again_decorator_test.cc
  create mode 100644 src/base/try_again_decorator.h

diff --git a/src/base/Makefile.am b/src/base/Makefile.am
index 956cce6..f11b738 100644
--- a/src/base/Makefile.am
+++ b/src/base/Makefile.am
@@ -150,6 +150,7 @@ noinst_HEADERS += \
src/base/unix_client_socket.h \
src/base/unix_server_socket.h \
src/base/unix_socket.h \
+   src/base/try_again_decorator.h \
src/base/usrbuf.h
  
  TESTS += bin/testleap bin/libbase_test bin/core_common_test

@@ -208,7 +209,9 @@ bin_libbase_test_SOURCES = \
src/base/tests/time_compare_test.cc \
src/base/tests/time_convert_test.cc \
src/base/tests/time_subtract_test.cc \
-   src/base/tests/unix_socket_test.cc
+   src/base/tests/unix_socket_test.cc \
+   src/base/tests/try_again_decorator_test.cc
+
  
  bin_libbase_test_LDADD = \

$(GTEST_DIR)/lib/libgtest.la \
diff --git a/src/base/tests/try_again_decorator_test.cc 
b/src/base/tests/try_again_decorator_test.cc
new file mode 100644
index 000..ac8595d
--- /dev/null
+++ b/src/base/tests/try_again_decorator_test.cc
@@ -0,0 +1,48 @@
+/*  -*- OpenSAF  -*-
+ *
+ * (C) Copyright 2017 The OpenSAF Foundation
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ * Author(s): Ericsson AB
+ *
+ */
+
+#include "base/try_again_decorator.h"
+#include "gtest/gtest.h"
+
+static unsigned counter = 0;

[HansN] use an unnamed namespace instead of static:
namespace {
  unsigned counter = 0;
}


+
+SaAisErrorT TestMethod() {
+  ++counter;
+  return SA_AIS_ERR_TRY_AGAIN;
+}

[HansN] shouldn't this function be declared extern "C" ?


+
+TEST(TryAgainDecorator, DefaultRetryControl) {
+  // default interval = 40ms, timeout = 10 * 1000ms
+  using DefaultDecorator = base::TryAgainDecorator<>;
+  auto DecorTestMethod = DefaultDecorator::DecorateFunction(TestMethod);
+
+  DecorTestMethod();


[HansN] the return code from the DecorTestMethod should be validated

+  EXPECT_GE(counter, 200);
+  EXPECT_LE(counter, 250);
+  counter = 0;
+}
+
+TEST(TryAgainDecorator, GivenRetryControl) {
+  // interval = 10ms, timeout = 1000ms
+  using TryAgainDecorator = base::TryAgainDecorator<10, 1 * 1000>;
+  auto DecorTestMethod  = TryAgainDecorator::DecorateFunction(TestMethod);
+
+  DecorTestMethod();


[HansN] the return code from the DecorTestMethod should be validated

+  EXPECT_GE(counter, 50);
+  EXPECT_LE(counter, 100);
+  counter = 0;
+}
diff --git a/src/base/try_again_decorator.h b/src/base/try_again_decorator.h
new file mode 100644
index 000..e676697
--- /dev/null
+++ b/src/base/try_again_decorator.h
@@ -0,0 +1,109 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ */
+
+#ifndef BASE_TRY_AGAIN_DECORATOR_H_
+#define BASE_TRY_AGAIN_DECORATOR_H_
+
+#include 
+#include 
+#include 
+#include "base/time.h"
+
+namespace base {
+
[HansN] perhaps we can use a strategy/policy like pattern instead? What 
do you say about something like the following:



struct RetryStrategy {

  virtual int nTries() = 0;

  virtual int retryInterval() = 0;

};

struct DefaultRetryStrategy : public RetryStrategy {

  int nTries() {return 10;}

  int retryInterval() {return 10;}

};

struct AnotherRetryStrategy : public RetryStrategy {

  int nTries() {return 100;}

  int retryInterval() {return 100;}

};

DefaultRetryStrategy DfltStrategy;

AnotherRetryStrategy AnotherStrategy;

int SA_AIS_ERR_TRY_AGAIN = 0;

//

template  class Decorator;

template 

class Decorator

Re: [devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-11-28 Thread Vu Minh Nguyen
Hi all,

I have added static assertion for validating retry control parameters,
so that user will get complier error if giving invalid inputs. Below is the
diff.

diff --git a/src/base/try_again_decorator.h b/src/base/try_again_decorator.h
index e676697..b11194d 100644
--- a/src/base/try_again_decorator.h
+++ b/src/base/try_again_decorator.h
@@ -98,6 +98,8 @@ class TryAgainDecorator {
  public:
   template
   static Decorator DecorateFunction(T (*f)(Args ...)) {
+static_assert((timeout_ms / interval_ms) > 1,
+  "timeout must be larger than interval!");
 RetryControl ctrl({0, interval_ms * 1000 * 1000}, timeout_ms);
 return Decorator
 (std::function(f), ctrl);

Regards, Vu

> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: Tuesday, November 28, 2017 2:13 PM
> To: anders.wid...@ericsson.com; hans.nordeb...@ericsson.com
> Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen
> 
> Subject: [PATCH 1/1] base: create generic try-again handling decorator for
AIS
> APIs [#2702]
> 
> Make generic C++ decorator for handling SA_AIS_ERR_TRY_AGAIN return
> code
> of AIS APIs.
> ---
>  src/base/Makefile.am   |   5 +-
>  src/base/tests/try_again_decorator_test.cc |  48 +
>  src/base/try_again_decorator.h | 109
> +
>  3 files changed, 161 insertions(+), 1 deletion(-)
>  create mode 100644 src/base/tests/try_again_decorator_test.cc
>  create mode 100644 src/base/try_again_decorator.h
> 
> diff --git a/src/base/Makefile.am b/src/base/Makefile.am
> index 956cce6..f11b738 100644
> --- a/src/base/Makefile.am
> +++ b/src/base/Makefile.am
> @@ -150,6 +150,7 @@ noinst_HEADERS += \
>   src/base/unix_client_socket.h \
>   src/base/unix_server_socket.h \
>   src/base/unix_socket.h \
> + src/base/try_again_decorator.h \
>   src/base/usrbuf.h
> 
>  TESTS += bin/testleap bin/libbase_test bin/core_common_test
> @@ -208,7 +209,9 @@ bin_libbase_test_SOURCES = \
>   src/base/tests/time_compare_test.cc \
>   src/base/tests/time_convert_test.cc \
>   src/base/tests/time_subtract_test.cc \
> - src/base/tests/unix_socket_test.cc
> + src/base/tests/unix_socket_test.cc \
> + src/base/tests/try_again_decorator_test.cc
> +
> 
>  bin_libbase_test_LDADD = \
>   $(GTEST_DIR)/lib/libgtest.la \
> diff --git a/src/base/tests/try_again_decorator_test.cc
> b/src/base/tests/try_again_decorator_test.cc
> new file mode 100644
> index 000..ac8595d
> --- /dev/null
> +++ b/src/base/tests/try_again_decorator_test.cc
> @@ -0,0 +1,48 @@
> +/*  -*- OpenSAF  -*-
> + *
> + * (C) Copyright 2017 The OpenSAF Foundation
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are
> licensed
> + * under the GNU Lesser General Public License Version 2.1, February
1999.
> + * The complete license can be accessed from the following location:
> + * http://opensource.org/licenses/lgpl-license.php
> + * See the Copying file included with the OpenSAF distribution for full
> + * licensing terms.
> + *
> + * Author(s): Ericsson AB
> + *
> + */
> +
> +#include "base/try_again_decorator.h"
> +#include "gtest/gtest.h"
> +
> +static unsigned counter = 0;
> +
> +SaAisErrorT TestMethod() {
> +  ++counter;
> +  return SA_AIS_ERR_TRY_AGAIN;
> +}
> +
> +TEST(TryAgainDecorator, DefaultRetryControl) {
> +  // default interval = 40ms, timeout = 10 * 1000ms
> +  using DefaultDecorator = base::TryAgainDecorator<>;
> +  auto DecorTestMethod = DefaultDecorator::DecorateFunction(TestMethod);
> +
> +  DecorTestMethod();
> +  EXPECT_GE(counter, 200);
> +  EXPECT_LE(counter, 250);
> +  counter = 0;
> +}
> +
> +TEST(TryAgainDecorator, GivenRetryControl) {
> +  // interval = 10ms, timeout = 1000ms
> +  using TryAgainDecorator = base::TryAgainDecorator<10, 1 * 1000>;
> +  auto DecorTestMethod  =
> TryAgainDecorator::DecorateFunction(TestMethod);
> +
> +  DecorTestMethod();
> +  EXPECT_GE(counter, 50);
> +  EXPECT_LE(counter, 100);
> +  counter = 0;
> +}
> diff --git a/src/base/try_again_decorator.h
b/src/base/try_again_decorator.h
> new file mode 100644
> index 000..e676697
> --- /dev/null
> +++ b/src/base/try_again_decorator.h
> @@ -0,0 +1,109 @@
> +/*  -*- OpenSAF  -*-
> + *
> + * Copyright Ericsson AB 2017 - All Rights Reserved.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are
> licensed
> + * under the GNU Lesser General Public License Version 2.1, February
1999.
> + * The complete license can be accessed from the following location:
> + * 

Re: [devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-11-28 Thread Vu Minh Nguyen
Hi Anders,

Thanks a lot for your very good comments.  I have just sent out the updated 
patch V2. 

Regards, Vu

> -Original Message-
> From: Anders Widell [mailto:anders.wid...@ericsson.com]
> Sent: Monday, November 27, 2017 10:45 PM
> To: Vu Minh Nguyen ;
> hans.nordeb...@ericsson.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1/1] base: create generic try-again handling decorator for
> AIS APIs [#2702]
> 
> Hi!
> 
> I think this is a very good and generic approach. I will do some more
> reviewing and testing, but here are my initial comments (see below,
> marked AndersW>). My major comment is that I don't like the RetryControl
> as it is done here. Maybe a better approach would be to provide the
> retry control as template parameter(s)?
> 
> regards,
> 
> Anders Widell
> 
> 
> On 11/24/2017 11:30 AM, Vu Minh Nguyen wrote:
> > Make generic C++ python-like decorator handling SA_AIS_ERR_TRY_AGAIN
> > return code of AIS APIs.
> > ---
> >   src/base/Makefile.am   |   5 +-
> >   src/base/tests/try_again_decorator_test.cc |  69 +++
> >   src/base/try_again_decorator.h | 131
> +
> >   3 files changed, 204 insertions(+), 1 deletion(-)
> >   create mode 100644 src/base/tests/try_again_decorator_test.cc
> >   create mode 100644 src/base/try_again_decorator.h
> >
> > diff --git a/src/base/Makefile.am b/src/base/Makefile.am
> > index 956cce6..f11b738 100644
> > --- a/src/base/Makefile.am
> > +++ b/src/base/Makefile.am
> > @@ -150,6 +150,7 @@ noinst_HEADERS += \
> > src/base/unix_client_socket.h \
> > src/base/unix_server_socket.h \
> > src/base/unix_socket.h \
> > +   src/base/try_again_decorator.h \
> > src/base/usrbuf.h
> >
> >   TESTS += bin/testleap bin/libbase_test bin/core_common_test
> > @@ -208,7 +209,9 @@ bin_libbase_test_SOURCES = \
> > src/base/tests/time_compare_test.cc \
> > src/base/tests/time_convert_test.cc \
> > src/base/tests/time_subtract_test.cc \
> > -   src/base/tests/unix_socket_test.cc
> > +   src/base/tests/unix_socket_test.cc \
> > +   src/base/tests/try_again_decorator_test.cc
> > +
> >
> >   bin_libbase_test_LDADD = \
> > $(GTEST_DIR)/lib/libgtest.la \
> > diff --git a/src/base/tests/try_again_decorator_test.cc
> b/src/base/tests/try_again_decorator_test.cc
> > new file mode 100644
> > index 000..50ddf15
> > --- /dev/null
> > +++ b/src/base/tests/try_again_decorator_test.cc
> > @@ -0,0 +1,69 @@
> > +/*  -*- OpenSAF  -*-
> > + *
> > + * (C) Copyright 2017 The OpenSAF Foundation
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY
> > + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are
> licensed
> > + * under the GNU Lesser General Public License Version 2.1, February 1999.
> > + * The complete license can be accessed from the following location:
> > + * http://opensource.org/licenses/lgpl-license.php
> > + * See the Copying file included with the OpenSAF distribution for full
> > + * licensing terms.
> > + *
> > + * Author(s): Ericsson AB
> > + *
> > + */
> > +
> > +#define private public
> AndersW> Even though this is just a test case, it is not so good to do
> things like this. Not sure exactly why you need it, but if it is only
> because you need read access to the retry_ctrl_ variable then why not
> add a public getter method for it?
> 
> > +#include "base/try_again_decorator.h"
> > +#include "gtest/gtest.h"
> > +
> > +static unsigned count = 0;
> > +
> > +SaAisErrorT TestMethod() {
> > +  ++count;
> > +  return SA_AIS_ERR_TRY_AGAIN;
> > +}
> > +
> > +TEST(TryAgainDecorator, DecoratorFunction) {
> > +  auto DecorTestMethod = base::TryAgainDecorator(TestMethod);
> > +  DecorTestMethod();
> > +  EXPECT_GE(count, 200);
> > +  EXPECT_LE(count, 250);
> > +  count = 0;
> > +}
> > +
> > +TEST(TryAgainDecorator, DefaultRetryControl) {
> > +  auto DecorTestMethod = base::TryAgainDecorator(TestMethod);
> > +  timespec interval = DecorTestMethod.retry_ctrl_->interval;
> > +  uint64_t timeout = DecorTestMethod.retry_ctrl_->timeout;
> > +
> > +  EXPECT_EQ(interval.tv_sec, 0);
> > +  EXPECT_EQ(interval.tv_nsec, 40*1000*1000);
> > +  EXPECT_EQ(timeout, 10*1000);
> > +}
> > +
> > +TEST(TryAgainDecorator, UseLocalRetryControl) {
> > +  base::RetryControl ctrl({0, 60*1000}, 60);
> > +  auto DecorTestMethod = base::TryAgainDecorator(TestMethod, ctrl);
> > +  timespec interval = DecorTestMethod.retry_ctrl_->interval;
> > +  uint64_t timeout = DecorTestMethod.retry_ctrl_->timeout;
> > +
> > +  EXPECT_EQ(interval.tv_sec, 0);
> > +  EXPECT_EQ(interval.tv_nsec, 60*1000);
> > +  EXPECT_EQ(timeout, 60);
> > +}
> > +
> > +TEST(TryAgainDecorator, ChangeGlobalRetryControl) {
> > +  base::RetryControl ctrl({0, 100*1000}, 200);
> > +  auto DecorTestMethod = base::TryAgainDecorator(TestMethod);
> > +
> > +  

[devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-11-28 Thread Vu Minh Nguyen
Make generic C++ decorator for handling SA_AIS_ERR_TRY_AGAIN return code
of AIS APIs.
---
 src/base/Makefile.am   |   5 +-
 src/base/tests/try_again_decorator_test.cc |  48 +
 src/base/try_again_decorator.h | 109 +
 3 files changed, 161 insertions(+), 1 deletion(-)
 create mode 100644 src/base/tests/try_again_decorator_test.cc
 create mode 100644 src/base/try_again_decorator.h

diff --git a/src/base/Makefile.am b/src/base/Makefile.am
index 956cce6..f11b738 100644
--- a/src/base/Makefile.am
+++ b/src/base/Makefile.am
@@ -150,6 +150,7 @@ noinst_HEADERS += \
src/base/unix_client_socket.h \
src/base/unix_server_socket.h \
src/base/unix_socket.h \
+   src/base/try_again_decorator.h \
src/base/usrbuf.h
 
 TESTS += bin/testleap bin/libbase_test bin/core_common_test
@@ -208,7 +209,9 @@ bin_libbase_test_SOURCES = \
src/base/tests/time_compare_test.cc \
src/base/tests/time_convert_test.cc \
src/base/tests/time_subtract_test.cc \
-   src/base/tests/unix_socket_test.cc
+   src/base/tests/unix_socket_test.cc \
+   src/base/tests/try_again_decorator_test.cc
+
 
 bin_libbase_test_LDADD = \
$(GTEST_DIR)/lib/libgtest.la \
diff --git a/src/base/tests/try_again_decorator_test.cc 
b/src/base/tests/try_again_decorator_test.cc
new file mode 100644
index 000..ac8595d
--- /dev/null
+++ b/src/base/tests/try_again_decorator_test.cc
@@ -0,0 +1,48 @@
+/*  -*- OpenSAF  -*-
+ *
+ * (C) Copyright 2017 The OpenSAF Foundation
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ * Author(s): Ericsson AB
+ *
+ */
+
+#include "base/try_again_decorator.h"
+#include "gtest/gtest.h"
+
+static unsigned counter = 0;
+
+SaAisErrorT TestMethod() {
+  ++counter;
+  return SA_AIS_ERR_TRY_AGAIN;
+}
+
+TEST(TryAgainDecorator, DefaultRetryControl) {
+  // default interval = 40ms, timeout = 10 * 1000ms
+  using DefaultDecorator = base::TryAgainDecorator<>;
+  auto DecorTestMethod = DefaultDecorator::DecorateFunction(TestMethod);
+
+  DecorTestMethod();
+  EXPECT_GE(counter, 200);
+  EXPECT_LE(counter, 250);
+  counter = 0;
+}
+
+TEST(TryAgainDecorator, GivenRetryControl) {
+  // interval = 10ms, timeout = 1000ms
+  using TryAgainDecorator = base::TryAgainDecorator<10, 1 * 1000>;
+  auto DecorTestMethod  = TryAgainDecorator::DecorateFunction(TestMethod);
+
+  DecorTestMethod();
+  EXPECT_GE(counter, 50);
+  EXPECT_LE(counter, 100);
+  counter = 0;
+}
diff --git a/src/base/try_again_decorator.h b/src/base/try_again_decorator.h
new file mode 100644
index 000..e676697
--- /dev/null
+++ b/src/base/try_again_decorator.h
@@ -0,0 +1,109 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ */
+
+#ifndef BASE_TRY_AGAIN_DECORATOR_H_
+#define BASE_TRY_AGAIN_DECORATOR_H_
+
+#include 
+#include 
+#include 
+#include "base/time.h"
+
+namespace base {
+
+//>
+// C++ decorator which escapsulates try again handling.
+//
+// E.g:
+// 1) If user wants to call saClmInitialize() which has try again
+// handling inside using this decorator, do this:
+//
+// using TryAgainDecorator = base::TryAgainDecorator<>;
+// auto saClmInitialize = TryAgainDecorator(::saClmInitialize);
+// if (saClmInitialize(handle, cbs, version) != SA_AIS_OK) {
+//// error handling
+// }
+//
+// 2) If user wants other retry control than default ones,
+// such as interval = 10ms, timeout = 10second, do this:
+//
+// using TryAgainDecorator = base::TryAgainDecorator<
+//  {0, 10 * 1000 * 1000},
+//  10 * 1000
+//  >
+// auto saClmInitialize = TryAgainDecorator(::saClmInitialize);
+// if (saClmInitialize(handle, cbs, version) != SA_AIS_OK) {
+//// error handling
+// }
+//
+//<
+
+// Default sleep time b/w retries {second, ns}
+static const timespec kInterval = {0, 40 * 1000 * 1000};
+// Default maximum time for retry loop 

Re: [devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-11-27 Thread Anders Widell

Hi!

I think this is a very good and generic approach. I will do some more 
reviewing and testing, but here are my initial comments (see below, 
marked AndersW>). My major comment is that I don't like the RetryControl 
as it is done here. Maybe a better approach would be to provide the 
retry control as template parameter(s)?


regards,

Anders Widell


On 11/24/2017 11:30 AM, Vu Minh Nguyen wrote:

Make generic C++ python-like decorator handling SA_AIS_ERR_TRY_AGAIN
return code of AIS APIs.
---
  src/base/Makefile.am   |   5 +-
  src/base/tests/try_again_decorator_test.cc |  69 +++
  src/base/try_again_decorator.h | 131 +
  3 files changed, 204 insertions(+), 1 deletion(-)
  create mode 100644 src/base/tests/try_again_decorator_test.cc
  create mode 100644 src/base/try_again_decorator.h

diff --git a/src/base/Makefile.am b/src/base/Makefile.am
index 956cce6..f11b738 100644
--- a/src/base/Makefile.am
+++ b/src/base/Makefile.am
@@ -150,6 +150,7 @@ noinst_HEADERS += \
src/base/unix_client_socket.h \
src/base/unix_server_socket.h \
src/base/unix_socket.h \
+   src/base/try_again_decorator.h \
src/base/usrbuf.h
  
  TESTS += bin/testleap bin/libbase_test bin/core_common_test

@@ -208,7 +209,9 @@ bin_libbase_test_SOURCES = \
src/base/tests/time_compare_test.cc \
src/base/tests/time_convert_test.cc \
src/base/tests/time_subtract_test.cc \
-   src/base/tests/unix_socket_test.cc
+   src/base/tests/unix_socket_test.cc \
+   src/base/tests/try_again_decorator_test.cc
+
  
  bin_libbase_test_LDADD = \

$(GTEST_DIR)/lib/libgtest.la \
diff --git a/src/base/tests/try_again_decorator_test.cc 
b/src/base/tests/try_again_decorator_test.cc
new file mode 100644
index 000..50ddf15
--- /dev/null
+++ b/src/base/tests/try_again_decorator_test.cc
@@ -0,0 +1,69 @@
+/*  -*- OpenSAF  -*-
+ *
+ * (C) Copyright 2017 The OpenSAF Foundation
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ * Author(s): Ericsson AB
+ *
+ */
+
+#define private public
AndersW> Even though this is just a test case, it is not so good to do 
things like this. Not sure exactly why you need it, but if it is only 
because you need read access to the retry_ctrl_ variable then why not 
add a public getter method for it?



+#include "base/try_again_decorator.h"
+#include "gtest/gtest.h"
+
+static unsigned count = 0;
+
+SaAisErrorT TestMethod() {
+  ++count;
+  return SA_AIS_ERR_TRY_AGAIN;
+}
+
+TEST(TryAgainDecorator, DecoratorFunction) {
+  auto DecorTestMethod = base::TryAgainDecorator(TestMethod);
+  DecorTestMethod();
+  EXPECT_GE(count, 200);
+  EXPECT_LE(count, 250);
+  count = 0;
+}
+
+TEST(TryAgainDecorator, DefaultRetryControl) {
+  auto DecorTestMethod = base::TryAgainDecorator(TestMethod);
+  timespec interval = DecorTestMethod.retry_ctrl_->interval;
+  uint64_t timeout = DecorTestMethod.retry_ctrl_->timeout;
+
+  EXPECT_EQ(interval.tv_sec, 0);
+  EXPECT_EQ(interval.tv_nsec, 40*1000*1000);
+  EXPECT_EQ(timeout, 10*1000);
+}
+
+TEST(TryAgainDecorator, UseLocalRetryControl) {
+  base::RetryControl ctrl({0, 60*1000}, 60);
+  auto DecorTestMethod = base::TryAgainDecorator(TestMethod, ctrl);
+  timespec interval = DecorTestMethod.retry_ctrl_->interval;
+  uint64_t timeout = DecorTestMethod.retry_ctrl_->timeout;
+
+  EXPECT_EQ(interval.tv_sec, 0);
+  EXPECT_EQ(interval.tv_nsec, 60*1000);
+  EXPECT_EQ(timeout, 60);
+}
+
+TEST(TryAgainDecorator, ChangeGlobalRetryControl) {
+  base::RetryControl ctrl({0, 100*1000}, 200);
+  auto DecorTestMethod = base::TryAgainDecorator(TestMethod);
+
+  base::ChangeGlobalRetryControl(ctrl);
+  timespec interval = DecorTestMethod.retry_ctrl_->interval;
+  uint64_t timeout = DecorTestMethod.retry_ctrl_->timeout;
+
+  EXPECT_EQ(interval.tv_sec, 0);
+  EXPECT_EQ(interval.tv_nsec, 100*1000);
+  EXPECT_EQ(timeout, 200);
+}
diff --git a/src/base/try_again_decorator.h b/src/base/try_again_decorator.h
new file mode 100644
index 000..1b648f4
--- /dev/null
+++ b/src/base/try_again_decorator.h
@@ -0,0 +1,131 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The 

[devel] [PATCH 1/1] base: create generic try-again handling decorator for AIS APIs [#2702]

2017-11-24 Thread Vu Minh Nguyen
Make generic C++ python-like decorator handling SA_AIS_ERR_TRY_AGAIN
return code of AIS APIs.
---
 src/base/Makefile.am   |   5 +-
 src/base/tests/try_again_decorator_test.cc |  69 +++
 src/base/try_again_decorator.h | 131 +
 3 files changed, 204 insertions(+), 1 deletion(-)
 create mode 100644 src/base/tests/try_again_decorator_test.cc
 create mode 100644 src/base/try_again_decorator.h

diff --git a/src/base/Makefile.am b/src/base/Makefile.am
index 956cce6..f11b738 100644
--- a/src/base/Makefile.am
+++ b/src/base/Makefile.am
@@ -150,6 +150,7 @@ noinst_HEADERS += \
src/base/unix_client_socket.h \
src/base/unix_server_socket.h \
src/base/unix_socket.h \
+   src/base/try_again_decorator.h \
src/base/usrbuf.h
 
 TESTS += bin/testleap bin/libbase_test bin/core_common_test
@@ -208,7 +209,9 @@ bin_libbase_test_SOURCES = \
src/base/tests/time_compare_test.cc \
src/base/tests/time_convert_test.cc \
src/base/tests/time_subtract_test.cc \
-   src/base/tests/unix_socket_test.cc
+   src/base/tests/unix_socket_test.cc \
+   src/base/tests/try_again_decorator_test.cc
+
 
 bin_libbase_test_LDADD = \
$(GTEST_DIR)/lib/libgtest.la \
diff --git a/src/base/tests/try_again_decorator_test.cc 
b/src/base/tests/try_again_decorator_test.cc
new file mode 100644
index 000..50ddf15
--- /dev/null
+++ b/src/base/tests/try_again_decorator_test.cc
@@ -0,0 +1,69 @@
+/*  -*- OpenSAF  -*-
+ *
+ * (C) Copyright 2017 The OpenSAF Foundation
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ * Author(s): Ericsson AB
+ *
+ */
+
+#define private public
+#include "base/try_again_decorator.h"
+#include "gtest/gtest.h"
+
+static unsigned count = 0;
+
+SaAisErrorT TestMethod() {
+  ++count;
+  return SA_AIS_ERR_TRY_AGAIN;
+}
+
+TEST(TryAgainDecorator, DecoratorFunction) {
+  auto DecorTestMethod = base::TryAgainDecorator(TestMethod);
+  DecorTestMethod();
+  EXPECT_GE(count, 200);
+  EXPECT_LE(count, 250);
+  count = 0;
+}
+
+TEST(TryAgainDecorator, DefaultRetryControl) {
+  auto DecorTestMethod = base::TryAgainDecorator(TestMethod);
+  timespec interval = DecorTestMethod.retry_ctrl_->interval;
+  uint64_t timeout = DecorTestMethod.retry_ctrl_->timeout;
+
+  EXPECT_EQ(interval.tv_sec, 0);
+  EXPECT_EQ(interval.tv_nsec, 40*1000*1000);
+  EXPECT_EQ(timeout, 10*1000);
+}
+
+TEST(TryAgainDecorator, UseLocalRetryControl) {
+  base::RetryControl ctrl({0, 60*1000}, 60);
+  auto DecorTestMethod = base::TryAgainDecorator(TestMethod, ctrl);
+  timespec interval = DecorTestMethod.retry_ctrl_->interval;
+  uint64_t timeout = DecorTestMethod.retry_ctrl_->timeout;
+
+  EXPECT_EQ(interval.tv_sec, 0);
+  EXPECT_EQ(interval.tv_nsec, 60*1000);
+  EXPECT_EQ(timeout, 60);
+}
+
+TEST(TryAgainDecorator, ChangeGlobalRetryControl) {
+  base::RetryControl ctrl({0, 100*1000}, 200);
+  auto DecorTestMethod = base::TryAgainDecorator(TestMethod);
+
+  base::ChangeGlobalRetryControl(ctrl);
+  timespec interval = DecorTestMethod.retry_ctrl_->interval;
+  uint64_t timeout = DecorTestMethod.retry_ctrl_->timeout;
+
+  EXPECT_EQ(interval.tv_sec, 0);
+  EXPECT_EQ(interval.tv_nsec, 100*1000);
+  EXPECT_EQ(timeout, 200);
+}
diff --git a/src/base/try_again_decorator.h b/src/base/try_again_decorator.h
new file mode 100644
index 000..1b648f4
--- /dev/null
+++ b/src/base/try_again_decorator.h
@@ -0,0 +1,131 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ */
+
+#ifndef BASE_TRY_AGAIN_DECORATOR_H_
+#define BASE_TRY_AGAIN_DECORATOR_H_
+
+#include 
+#include 
+#include "base/time.h"
+#include "ais/include/saAis.h"
+
+namespace base {
+
+struct RetryControl {
+  // Sleep time b/w retries
+  timespec interval;
+  // Maximum time for retries (ms)
+  uint64_t timeout;
+
+  RetryControl() {
+interval = {0, 40*1000*1000};  // 40 miliseconds
+timeout  = 10*1000;// 10 seconds
+  }
+
+  explicit