Unknown opened 2 years ago
|
|
Thank you for analysing this and the comments. Indeed, in this case the distribution may be sub-optimal. What's more, in mobile environments stream opening may be more frequent than actual stanza exchanges due to frequent reconnections, which could further emphasise the issue. Idea behind |
|
Thanks @woj-tek I am preparing a pull request. Do you have a suggestion for verifying order of the packets? It would be great to know must have test scenarios and maybe a recommendation to automate the process. |
|
We do have https://github.com/tigase/tigase-tts-ng (and also older, legacy and not maintained https://github.com/tigase/tigase-testsuite) which could catch those. Though, in this case, using packetFrom should still maintain correct ordering of packets as incoming ones from the same connection would be processed by the same thread (and queue) |
|
In my tests, |
|
@woj-tek PR is ready for review PR 179 |
|
Simple test statistics: 2500 connections, plain connect/disconnectbefore change:
with proposed change
2500 connections, 10 messages send:before changes:
With included changes
|
|
@woj-tek I fixed the unit tests. If you wish,I can improve readability of the tests by changing values with more readable values too. As your test results states, thread distribution is better. But I notice increased number of queue overflows. I am still analyzing but I think we should be more skeptical about this change. |
|
@karapirinc Yes, please |
|
Merged |
|
After merging TTS tests started to fail with missing packets, which would indicated concurrency issues. For how I reverted the change. |
|
@woj-tek Could you please share reference to the failed tests? |
|
Referenced from commit 1 year ago
|
|
wojciech.kapcia@tigase.net added "Related" #1342 7 months ago
|
Type |
Bug
|
Components distribute the load to the different threads and queues according to the hash code calculation of a packet. Session manager respectively checks not null values of
packet.getPacketFrom
,packet.getPacketTo
andpacket.getStanzaTo().getBareJID()
.For the initial connection establishment process, starting from the
STREAM_OPENED
to theTLS_HANDSHAKE_COMPLETE
commands, the same thread processes all connection attempts. The reason for that ishashCodeForPacket
calculated according to the same valuesess-man@HOSTNAME
.Example
tigase.stats.sess_man_Processed_packets_thread_IN
results in our test env after some period of time.To resolve this bottleneck, shouldn't we use connection id for better thread distribution?
An example from my local machine.
AbstractMessageReceiver.addPacket
andaddPacketNB
logs while opening a stream. QueueIdx is calculated bysess-man@localhost
. (I have 10 CPU core in my machine). Every connection request will end up in queues 6 and 46. Instead, queueIdx could be calculated according toc2s@localhost/127.0.0.1_5222_127.0.0.1_53424
. This would speed up connection establishment process. Better thread utilization Less risk for queue overflow.QueueIdx: 6 NAME: message-router TYPE: set ELEM_NAME: iq COMMAND: STREAM_OPENED PACKET_FROM: null PACKET_TO: null STANZA_FROM: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 STANZA_TO: sess-man@localhost
QueueIdx: 46 NAME: sess-man TYPE: set ELEM_NAME: iq COMMAND: STREAM_OPENED PACKET_FROM: null PACKET_TO: null STANZA_FROM: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 STANZA_TO: sess-man@localhost
QueueIdx: 16 NAME: message-router TYPE: result ELEM_NAME: iq COMMAND: null PACKET_FROM: sess-man@localhost PACKET_TO: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 STANZA_FROM: sess-man@localhost STANZA_TO: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424
QueueIdx: 16 NAME: c2s TYPE: result ELEM_NAME: iq COMMAND: null PACKET_FROM: sess-man@localhost PACKET_TO: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 STANZA_FROM: sess-man@localhost STANZA_TO: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424
QueueIdx: 6 NAME: message-router TYPE: get ELEM_NAME: iq COMMAND: GETFEATURES PACKET_FROM: null PACKET_TO: null STANZA_FROM: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 STANZA_TO: sess-man@localhost
QueueIdx: 46 NAME: sess-man TYPE: get ELEM_NAME: iq COMMAND: GETFEATURES PACKET_FROM: null PACKET_TO: null STANZA_FROM: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 STANZA_TO: sess-man@localhost
QueueIdx: 16 NAME: message-router TYPE: result ELEM_NAME: iq COMMAND: GETFEATURES PACKET_FROM: sess-man@localhost PACKET_TO: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 STANZA_FROM: sess-man@localhost STANZA_TO: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424
QueueIdx: 16 NAME: c2s TYPE: result ELEM_NAME: iq COMMAND: GETFEATURES PACKET_FROM: sess-man@localhost PACKET_TO: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 STANZA_FROM: sess-man@localhost STANZA_TO: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424
QueueIdx: 16 NAME: message-router TYPE: null ELEM_NAME: starttls PACKET_FROM: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 PACKET_TO: sess-man@localhost STANZA_FROM: null STANZA_TO: null
QueueIdx: 96 NAME: sess-man TYPE: null ELEM_NAME: starttls PACKET_FROM: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 PACKET_TO: sess-man@localhost STANZA_FROM: null STANZA_TO: null
QueueIdx: 39 NAME: message-router TYPE: set ELEM_NAME: iq COMMAND: STARTTLS PACKET_FROM: sess-man@localhost PACKET_TO: null STANZA_FROM: sess-man@localhost STANZA_TO: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424
QueueIdx: 16 NAME: c2s TYPE: set ELEM_NAME: iq COMMAND: STARTTLS PACKET_FROM: sess-man@localhost PACKET_TO: null STANZA_FROM: sess-man@localhost STANZA_TO: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424
QueueIdx: 6 NAME: message-router TYPE: set ELEM_NAME: iq COMMAND: TLS_HANDSHAKE_COMPLETE PACKET_FROM: null PACKET_TO: null STANZA_FROM: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 STANZA_TO: sess-man@localhost
QueueIdx: 46 NAME: sess-man TYPE: set ELEM_NAME: iq COMMAND: TLS_HANDSHAKE_COMPLETE PACKET_FROM: null PACKET_TO: null STANZA_FROM: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 STANZA_TO: sess-man@localhost
QueueIdx: 6 NAME: message-router TYPE: get ELEM_NAME: iq COMMAND: GETFEATURES PACKET_FROM: null PACKET_TO: null STANZA_FROM: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 STANZA_TO: sess-man@localhost
QueueIdx: 46 NAME: sess-man TYPE: get ELEM_NAME: iq COMMAND: GETFEATURES PACKET_FROM: null PACKET_TO: null STANZA_FROM: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 STANZA_TO: sess-man@localhost
QueueIdx: 16 NAME: c2s TYPE: result ELEM_NAME: iq COMMAND: GETFEATURES PACKET_FROM: sess-man@localhost PACKET_TO: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 STANZA_FROM: sess-man@localhost STANZA_TO: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424
18.QueueIdx: 16 NAME: message-router TYPE: result ELEM_NAME: iq COMMAND: GETFEATURES PACKET_FROM: sess-man@localhost PACKET_TO: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424 STANZA_FROM: sess-man@localhost STANZA_TO: c2s@localhost/127.0.0.1_5222_127.0.0.1_53424
Tigase version: 8.1.0 JVM flavour and version : openjdk 11.0.7 2020-04-14 Operating system/distribution/version : Ubuntu 18.04.1 LTS INTEL(R) XEON(R) PLATINUM 8259CL CPU @ 2.50GHZ GNU/Linux - 1 CPU - 2 virtual CPU - 4.06G RAM