Architectural changes to use boost thread_pool, boost::future, std::promise, std::nested_exception, connection local invocation and event handler maps #584

ihsandemir · 2020-04-13T07:53:51Z

Changed the event processing executor usage to use asio::thread_pool.

Changed the ResponseThread to ResponseProcessor and it now processes the response message directly by default and will use a thread_pool if the newly introduced parameter hazelcast.client.response.executor.thread.count is set to a positive value.

Removed custom Thread, Future implementations and used std::thread, boost::asio::thread_pool and boost::future/boost::promise.

boost::future supports continuation (.then support).

Changed the invocation and event handler map design. Using an unordered_map which which only being accessed from the same io thread for the connection. This eliminated the need for using sync maps for invocations and events. If any other thread needs to access the invocations, it passes this request to io thread. This is only needed when closing the socket for notifying the outstanding invocations. If an invocation can not be queued into the connection (i.e. no connection exists for the invocation and it is being retried), then it is registered in a sync map at the invocation service only for the first retry and cleared from the service when a response is finally received. This ensures that the user thread is always being notified.

Exception architecture is compeletely changed so that we use std exception_ptr capabilities. Also integrated std::nested exceptions for exceptions which has a cause exception. The cause is obtained by using std::rethrow_if_nested method.

The project now depends on boost::thread for boost::future. Changed cmake so that the boost installation is user's responsibility. Also using the boost Thread library version latest which is version 5.

Changed the ResponseThread to ResponseProcessor and it now processes the response message directly by default and will use a thread_pool if the newly introduced parameter `hazelcast.client.response.executor.thread.count` is set to a positive value. Removed custom Thread, Future implementations and used std::thread, boost::asio::thread_pool and boost::future/boost::promise. boost::future supports continuation (`.then` support). Changed the invocation and event handler map design. Using an `unordered_map` which which only being accessed from the same `io` thread for the connection. This eliminated the need for using sync maps for invocations and events. If any other thread needs to access the invocations, it passes this request to io thread. This is only needed when closing the socket for notifying the outstanding invocations. If an invocation can not be queued into the connection (i.e. no connection exists for the invocation and it is being retried), then it is registered in a sync map at the invocation service only for the first retry and cleared from the service when a response is finally received. This ensures that the user thread is always being notified. Exception architecture is compeletely changed so that we use std exception_ptr capabilities. Also integrated std::nested exceptions for exceptions which has a cause exception. The cause is obtained by using `std::rethrow_if_nested` method. The project now depends on boost::thread for boost::future. Changed cmake so that the boost installation is user's responsibility. Also using the boost Thread library version latest which is version 5.

devOpsHazelcast · 2020-04-13T07:53:54Z

All committers have signed the CLA.

devOpsHazelcast · 2020-04-13T07:57:47Z

Linux test FAILed.

devOpsHazelcast · 2020-04-13T07:58:14Z

Windows test FAILed.

sancar · 2020-04-13T12:01:29Z

hazelcast/include/hazelcast/client/IAtomicLong.h

@@ -190,7 +190,7 @@ namespace hazelcast {
             * @return an {@link ICompletableFuture} with the current value
             * @since cluster version 3.7
             */
-            std::shared_ptr<ICompletableFuture<int64_t> > getAsync();
+            future<std::shared_ptr<int64_t>> getAsync();


Should not these be future<int64_t> ? Do we need shared ptr's for primitives ?

No, we don't but I wıll change it when I change SerializationService to not use shared_ptr, this pointer is due to current serializationservice toObject method.

sancar · 2020-04-13T12:23:20Z

hazelcast/include/hazelcast/client/IMap.h

@@ -1034,7 +1033,7 @@ namespace hazelcast {
             * @see ICompletableFuture
             * @see #setAsync(Object, Object)
             */
-            std::shared_ptr<ICompletableFuture<V> > putAsync(const K &key, const V &value) {
+            future<std::shared_ptr<V>> putAsync(const K &key, const V &value) {


Would it make sense to use shared_future here ?

I try not to use shared_future where future is enough. the user can convert it to shared_future using future::share() method anyway.

devOpsHazelcast · 2020-04-13T12:33:12Z

Windows test FAILed.

devOpsHazelcast · 2020-04-13T12:34:47Z

Linux test FAILed.

devOpsHazelcast · 2020-04-13T13:28:13Z

Windows test FAILed.

devOpsHazelcast · 2020-04-13T13:36:43Z

Linux test FAILed.

devOpsHazelcast · 2020-04-13T15:48:15Z

Windows test FAILed.

devOpsHazelcast · 2020-04-13T16:14:07Z

Linux test FAILed.

devOpsHazelcast · 2020-04-13T17:38:14Z

Windows test FAILed.

devOpsHazelcast · 2020-04-13T17:58:32Z

Linux test FAILed.

…ConditionVariable`. Replaced them with standard and boost based implementations. Changed required boost version to 1.72.

devOpsHazelcast · 2020-04-14T12:48:16Z

Windows test FAILed.

devOpsHazelcast · 2020-04-14T12:49:34Z

Linux test FAILed.

devOpsHazelcast · 2020-04-14T17:08:13Z

Windows test FAILed.

devOpsHazelcast · 2020-04-14T17:16:02Z

Linux test FAILed.

Removed unneded include for windows (`WinSock2.h`) which causes error in compilation.

devOpsHazelcast · 2020-04-14T17:48:39Z

Windows test FAILed.

devOpsHazelcast · 2020-04-15T14:04:10Z

Windows test FAILed.

devOpsHazelcast · 2020-04-15T14:46:51Z

Linux test PASSed.

… invocation service. Changed the CallIdSequence to use std::array of atomic integers.

devOpsHazelcast · 2020-04-15T20:38:10Z

Windows test FAILed.

… for compiling successfully at Windows (fails due to `ambiguous symbol` error which can be both from std or boost namespaces).

devOpsHazelcast · 2020-04-15T21:14:46Z

Linux test FAILed.

devOpsHazelcast · 2020-04-15T21:16:23Z

Linux test FAILed.

devOpsHazelcast · 2020-04-15T21:20:48Z

Windows test FAILed.

devOpsHazelcast · 2020-04-16T06:51:56Z

Windows test FAILed.

devOpsHazelcast · 2020-04-16T07:18:56Z

Linux test FAILed.

devOpsHazelcast · 2020-04-16T08:04:19Z

Windows test FAILed.

devOpsHazelcast · 2020-04-16T08:45:15Z

Linux test PASSed.

Corrected the time string print utility. Replaced some of the time usages with std::chrono related ones. Made the timer cancel operations non-exception throwing. Replaced the system_clock usages with steady_clock usages. Warning fixes for windows for unused exceptions.

devOpsHazelcast · 2020-04-17T09:45:59Z

Windows test FAILed.

devOpsHazelcast · 2020-04-17T12:27:38Z

Linux test FAILed.

devOpsHazelcast · 2020-04-17T13:02:00Z

Windows test FAILed.

devOpsHazelcast · 2020-04-17T13:31:16Z

Linux test PASSed.

Some fixes so that the exceptions do not abort the io threads. Also, destroying the timer and pools upon shutdown so that no associated resource is left (which may happen for windows).

devOpsHazelcast · 2020-04-18T21:50:28Z

Windows test PASSed.

devOpsHazelcast · 2020-04-18T22:13:51Z

Linux test PASSed.

ihsandemir added the Type: Enhancement label Apr 13, 2020

ihsandemir added this to the 4.0 milestone Apr 13, 2020

ihsandemir requested a review from sancar April 13, 2020 07:53

ihsandemir self-assigned this Apr 13, 2020

sancar reviewed Apr 13, 2020

View reviewed changes

Linux build fixes.

1df90c0

ihsandemir force-pushed the threadFutureChanges branch from 85511bb to 1df90c0 Compare April 13, 2020 13:28

Fixed the examples per changes in the APIs.

8f253aa

linux test fix.

b292015

Removed custom implementations of `CountDownLatch, Mutex, LockGuard, …

00bf6ec

…ConditionVariable`. Replaced them with standard and boost based implementations. Changed required boost version to 1.72.

Fix to IExecutorService.

73f9ae9

Fix to code examples.

556c063

Removed unneded include for windows (`WinSock2.h`) which causes error in compilation.

Eliminated the extra invocations registration at the invocation service.

50ea0e0

Removed all invocation and even handler registrations in listener and…

5103406

… invocation service. Changed the CallIdSequence to use std::array of atomic integers.

Put boost:: namespace to all implicit boost usages which was needed…

805d17a

… for compiling successfully at Windows (fails due to `ambiguous symbol` error which can be both from std or boost namespaces).

Fixed the boost::latch usages in the examples.

01abe93

ihsandemir added 2 commits April 16, 2020 10:49

test fix for wait time.

e56926b

Removed unneded AtomicArray class.

2426100

ihsandemir requested a review from sancar April 17, 2020 12:33

Added hz_thread_pool for graceful shutdown of thread pools.

5f9c3ac

Some fixes so that the exceptions do not abort the io threads. Also, destroying the timer and pools upon shutdown so that no associated resource is left (which may happen for windows).

ihsandemir force-pushed the threadFutureChanges branch from 55f9a03 to 5f9c3ac Compare April 18, 2020 21:20

sancar approved these changes Apr 20, 2020

View reviewed changes

ihsandemir merged commit 4dd823c into hazelcast:master Apr 20, 2020

ihsandemir deleted the threadFutureChanges branch April 20, 2020 07:52

ihsandemir mentioned this pull request Apr 21, 2020

ClientExceptionFactory::createException does not instantiate the cause exceptions #499

Closed

Architectural changes to use boost thread_pool, boost::future, std::promise, std::nested_exception, connection local invocation and event handler maps #584

Architectural changes to use boost thread_pool, boost::future, std::promise, std::nested_exception, connection local invocation and event handler maps #584

Conversation

ihsandemir commented Apr 13, 2020

devOpsHazelcast commented Apr 13, 2020 • edited Loading

devOpsHazelcast commented Apr 13, 2020

devOpsHazelcast commented Apr 13, 2020

sancar Apr 13, 2020

Choose a reason for hiding this comment

ihsandemir Apr 13, 2020

Choose a reason for hiding this comment

sancar Apr 13, 2020

Choose a reason for hiding this comment

ihsandemir Apr 13, 2020

Choose a reason for hiding this comment

devOpsHazelcast commented Apr 13, 2020

devOpsHazelcast commented Apr 13, 2020

devOpsHazelcast commented Apr 13, 2020

devOpsHazelcast commented Apr 13, 2020

devOpsHazelcast commented Apr 13, 2020

devOpsHazelcast commented Apr 13, 2020

devOpsHazelcast commented Apr 13, 2020

devOpsHazelcast commented Apr 13, 2020

devOpsHazelcast commented Apr 14, 2020

devOpsHazelcast commented Apr 14, 2020

devOpsHazelcast commented Apr 14, 2020

devOpsHazelcast commented Apr 14, 2020

devOpsHazelcast commented Apr 14, 2020

devOpsHazelcast commented Apr 15, 2020

devOpsHazelcast commented Apr 15, 2020

devOpsHazelcast commented Apr 15, 2020

devOpsHazelcast commented Apr 15, 2020

devOpsHazelcast commented Apr 15, 2020

devOpsHazelcast commented Apr 15, 2020

devOpsHazelcast commented Apr 16, 2020

devOpsHazelcast commented Apr 16, 2020

devOpsHazelcast commented Apr 16, 2020

devOpsHazelcast commented Apr 16, 2020

devOpsHazelcast commented Apr 17, 2020

devOpsHazelcast commented Apr 17, 2020

devOpsHazelcast commented Apr 17, 2020

devOpsHazelcast commented Apr 17, 2020

devOpsHazelcast commented Apr 18, 2020

devOpsHazelcast commented Apr 18, 2020

devOpsHazelcast commented Apr 13, 2020 •

edited

Loading