Closed vetss closed 7 years ago
2) the second locking using - blocking of a Dialog instanse to guarantee some processing as atomic. The example is - we can jave two different events, one from a SS7 peer (TC-CONTINUE primitive has come), another from a local TCAP user (he wants to send some other message to a peer). We need to make the second processing only after the first one is finished. For this DialogImpl has now "ReentrantLock dialogLock = new ReentrantLock();" object that is locked before any action.
I am thinking of another possible solution. We can run both events types (from peer and from a local user) in one dedicated for a concrete Dialog thread (as it is done in netty). Dedicated means that we have a thread pool that processes TCAP events processing. But for a concrete dialog only one of those therads from the pool can be used. In this case actions from a TCAP user will be processed in these threads and not in threads of a TCAP user.
If you have some remarks / suggestions for discussing of the best design / bugs removing, please feel free to add comments.
i woudl start by implementing microbenchmark
[3:49] so the effort to discover best possible data structure is decreased
[3:49] again, i found JMH quite useful http://openjdk.java.net/projects/code-tools/jmh/
[3:49] so, to create a Junit test case to trigger the benchmark, you may follow my PR on MediaServer
[3:51] https://github.com/RestComm/mediaserver/commit/15067590bd2a9e90fb09036a0eccb7aad9771f38
[3:51] this way we may compare fastmap and CHM
[3:53] regarding suggested solution by @sergey_vetyutnev , its similar to the threadaffinity we have in JAIN SIP now
[3:53] it certainly reduce contention
[3:53] and more if you use a thread local map
[3:54] then if you have say, 50 threads in the pool
[3:54] instead of one big 10K dialog map
[3:54] you will have 50 maps of 200 dialogs
[3:54] becuase every thread has its own map
[3:54] and you know, for a given dialog, only one thread will be used
[3:54] so this concurrency model has big advantages
[3:55] thread affinity executor at https://github.com/RestComm/jain-sip/blob/master/src/gov/nist/javax/sip/ThreadAffinityExecutor.java
just added these comments into the issue for recording :wink: one thing we need to take into account with thread affinity solution, ir error handling by any means we cant allow the thread to be stuck too much time, or be aborted/interrupted specially if we go threadlocal maps since all that info would be lost again, this approach is more suitable if you have a 100% non-blocking system it could mean more thread switching, from IO pool, to worker pool but the concurrency simplification will balance all out this is how traditional Telco SLEEs work, they even did with one thread, now you need more threads to cover the cpu cores or even more threads to balanced a non 100% non-blocking system regarding the non-blocking system, it only applies to low load situations current static thread pool approach, is the same as affinity for high loads since the threads will be busy anyway... the affinity aproach on low-loads for blocking systems is alleviated by adding more threads to the pool, and ensuring good hashing/balancing algotirhm again, dialogIds use to be based in a sequence, so it delivers good balancing naturally
sure, feel free to check my affinity impl at https://github.com/RestComm/jain-sip/blob/master/src/gov/nist/javax/sip/ThreadAffinityExecutor.java it is easier to integrate is you have a task oriented approach (command pattern) so you may just schedule/submit the task into this new executor/pool thread you may insert the events into the threadpool as task we do not have now full task oriented approach, we need to update code for it if you have proper Event classes it will come easy we will possibly need to switch to such approach. May be we can do it when we will introduce a support for SLEE clustering or may be earlier if just a matter to define the event reactor boundary, and inject the threadaffinity executor in the middle of the event handling phase event dispatching,sorry
We have also two iisues / PRs that correspons this topic: https://github.com/RestComm/jss7/issues/107 https://github.com/RestComm/jss7/pull/109
applying Thread affinity concepts it would be easy to save those Dialog maps as threadLocals, distributing the stack objects in the threadPool.
Here are commites for fixing task 1 (with adding of some ideas from other tickets):
https://github.com/RestComm/jss7/commit/8d86765249bcf9cf876ef27614f9ad14efaae9ba - TCAP https://github.com/RestComm/jss7/commit/e054858bb922f4d3ef0e0cc519023e3a74539106 - TCAP-ANSI https://github.com/RestComm/jss7/commit/0da6d6c98e6efd4a69646bfa68c20706ef82f6c9 - MAP and CAP https://github.com/RestComm/jss7/commit/4238099fe0c2511ac89e941e72365a970d1c0860 - TCAP-ANSI - dialogPreviewList part https://github.com/RestComm/jss7/commit/e931dfa86f6b70121560ce92d68505f2c2ace2e9 - TCAP https://github.com/RestComm/jss7/commit/6fc59d427c6fd41bacc73da1788f98101aa04c8e - TCAP-ANSI
Two extra fixes for fixing of "currentDialogId.compareAndSet" issue: https://github.com/RestComm/jss7/commit/ce80d10ab4b45144e6b28602898d3f8ddf06f1c1 - TCAP https://github.com/RestComm/jss7/commit/28fc522758f2d3796191b7cfaa0973cee3bf455c - TCAP ANSI
We need to test if updates are correct and fit needs.
Task 2 will be done later with a separate ticket because it demands much efforts and interfaces update: https://github.com/RestComm/jss7/issues/196
I think we can close this issue and issues 107, 109, 97 because no feedback that made updates works in an unproper way.
We have now in TCAP level following locking cases:
1) we need to guarantee of storing dialogs into a dialog collection (with checking if a dialog with soem dialog id is stored or not). We need to generate available dialog IDs (this is generally a local TC transaction ID value) for creating of new local dialods.
We need to discover if this is a best way and try to remove synching of a global this.dialogs object. At the other side we need to guarantee atomic style of adding / removing / checing of dialogs and providing of a new dialogID. We are using of javolution FastMap because of it says of low access level for a collection access in case of many elemets count (I guess we need to think of values >= 100000 dialogs). But this collection does not allow multithread writing access without external synching (that we want to avoid). Possibly we can use ConcurrentHashMap without extternal blocking, we need to discover what is faster.
As for generating for available dialogID values we need to make in some cases restriction for a dialogID range, for example dialogIDs must be from 2000000 to 4000000. This is not very easy with AtomicLong instanse. We may think of suggested by Jean code for it: if(!atomicLong.compareAndSet(4000000, 2000000)) { value = atomicLong.getAndIncrement(); } else { value = 2000000; }