[kamailio/kamailio] dmq_usrloc: sync with multi contacts per message (#1054)

List overview All Threads
Download

newer

older

git:master:f51f4df7: dmq_usrloc:...

git:5.0:fdb4538e: topos: free...

Julien Chavanton

3 Apr 2017 3 Apr '17

4:42 p.m.

This was tested with 100K and 4 servers, when restarting a server we get a full sync from the 3 nodes in a few seconds :

```show registrations: first x "usrloc:location-contacts = 100000", show registrations: second x "usrloc:location-contacts = 100000", show registrations: third x "usrloc:location-contacts = 100000", show registrations: fourth x "usrloc:location-contacts = 100000", postgres database: 100000 ``` You can view, comment on, or merge this pull request online at:

https://github.com/kamailio/kamailio/pull/1054

-- Commit Summary --

* dmq_usrloc: sync with multi contacts per message

-- File Changes --

M src/modules/dmq_usrloc/README (24) M src/modules/dmq_usrloc/dmq_usrloc.c (7) M src/modules/dmq_usrloc/usrloc_sync.c (264)

-- Patch Links --

https://github.com/kamailio/kamailio/pull/1054.patch https://github.com/kamailio/kamailio/pull/1054.diff

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054

Attachments:

attachment.html (text/html — 3.2 KB)

Show replies by date

Julien Chavanton

4 Apr 4 Apr

7:58 p.m.

I think, I should add a check for maximum packet length. to make sure we will never try to send a datagram larger than 60K. Next step, we could add compression :)

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#issuecomment-291614449

Charles Chance

8:36 p.m.

I am reviewing the patch currently but as for limiting to 60KB, I'm not sure that for this application it is necessary. It will be difficult to be 100% accurate, anyway, since you only have control over the body - therefore, the only option is to assume a sensible size for the rest of the message and set that aside in your calculation.

I think Kamailio handles fragmentation just fine and since this is really just a few packets on startup/initial sync, I don't think we need to be concerned about it (although it may be worth a mention in the readme). Others may have a different opinion of course.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#issuecomment-291624187

Julien Chavanton

8:54 p.m.

This is now deployed on cluster running on AWS, fragmentation is taking place and everything is performing much better, no more transactions storm.

OK to update the readme, changing the example to 50 contacts and adding a comment about considering 1024 Bytes / contact in order to stay bellow 65536 UDP send

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#issuecomment-291628736

Julien Chavanton

9:12 p.m.

@jchavanton pushed 1 commit.

1226c0f dmq_usrloc: readme batch_msg_contacts

-- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/kamailio/kamailio/pull/1054/files/75eee5f84953ebe53399fe5...

Daniel-Constantin Mierla

9:23 p.m.

@jchavanton: the documentation has to be edited in docbook xml files located in `doc/` subfolder of each module. The readme file must not be edited directly, it is generate on server from those file.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#issuecomment-291636160

Julien Chavanton

9:48 p.m.

ok, corrected the documentation, I squashed to avoid too many commits

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#issuecomment-291642146

Julien Chavanton

5 Apr 5 Apr

3:21 a.m.

The overhead per contact is 188 chars, this is quite a lot, we may select an alternate format later. `1,:{"action":,"aor":"","ruid":"","c":"","received":"","path":"","callid":"","user_agent":"","instance":"","expires":,"cseq":,"flags":,"cflags":,"q":,"last_modified":,"methods":,"reg_id":},`

I think we could validate the size of the packet while building it to automatically send > 60000 for example.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#issuecomment-291737963

Charles Chance

8 a.m.

Are there other changes you'd like to make to this PR?

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#issuecomment-291784434

Julien Chavanton

7:34 p.m.

Hi Charles, I am implementing the size check, I think it would be nightmarish no to know if sometimes contacts could be missing because of oversized messages, this will simply things for anyone using this feature.

I am verifying that the calculation matches and I will test and commit later today.

Thanks for your review so far

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#issuecomment-291972354

Julien Chavanton

10:32 p.m.

@jchavanton pushed 1 commit.

8ed3190 dmq_usrloc: param batch_max_msg_size

-- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/kamailio/kamailio/pull/1054/files/f51f4df75cb347f1180c009...

Julien Chavanton

10:36 p.m.

I added `dmq_usrloc_batch_msg_size` to make sure we are never trying to send messages too large and to simplify the configuration. I did run a bunch of tests again.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#issuecomment-292017173

Julien Chavanton

7 Apr 7 Apr

6:16 a.m.

Hi @charlesrchance I am not planning any other modifications on this PR, the performance of sync is now acceptable for our need at this point.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#issuecomment-292450951

Julien Chavanton

6:24 a.m.

One thing that could help would be to increase the retransmission time-out only for the sync traffic, I did not see any obvious way to do this. I guess this is off topic and I was planning to discuss this in the mailing list.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#issuecomment-292452172

Daniel-Constantin Mierla

18 Apr 18 Apr

12:22 p.m.

@charlesrchance I guess this is ready to be merged if you are fine with everything so far.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#issuecomment-294816291

Charles Chance

19 Apr 19 Apr

7:19 p.m.

Returning after an extended Easter break - therefore, have not tested the recent changes, but it looks ok from source. If it has been tested already and everyone else is happy then it can be merged. Otherwise, I will do it tomorrow.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#issuecomment-295399976

Julien Chavanton

11:46 p.m.

I did test the latest version in my lab, not in prod yet.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#issuecomment-295497207

Julien Chavanton

20 Apr 20 Apr

12:26 a.m.

The lab test I did was load testing with 4 nodes, before going to prod we do integration testing, in this case see no reason why they would fail. The only reason why this is not in prodyet, is because instead of back porting to 4.5 we will start using 5.x

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#issuecomment-295509154

Charles Chance

21 Apr 21 Apr

9:51 p.m.

Merged #1054.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1054#event-1053150563

2862

Age (days ago)

2880

Last active (days ago)

sr-dev@lists.kamailio.org

18 comments

3 participants

tags (0)

participants (3)

Charles Chance
Daniel-Constantin Mierla
Julien Chavanton