[Home]

Summary:ASTERISK-16937: [patch] devicestate is not aggregated from all received events
Reporter:klaus3000 (klaus3000)Labels:
Date Opened:2010-11-09 11:29:01.000-0600Date Closed:2010-11-19 18:52:49.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) issue18284.rev1.txt
Description:When distributed devicestate (res_ais) is used, Asterisk should calculate an aggregated device state based on local and received device states. This is currently not done.

See
http://lists.digium.com/pipermail/asterisk-dev/2010-November/046926.html
and
http://lists.digium.com/pipermail/asterisk-dev/2010-November/046915.html

****** ADDITIONAL INFORMATION ******

Please find the debug log at http://pastebin.com/HxXxcriX

0. watcher (BLF) klaus3 is SUBSCRIBEd to SIP/klaus2 on server1

1. SIP/klaus2 on server2 calls Milliwatt
  -> event is distributed to server1
  -> NOTIFY
  -> BLF is on

2. on server1, SIP/klaus9 calls SIP/klaus2
  -> BLF is still on

3. on server1, SIP/klaus9->SIP/klaus2 hang up
  -> NOTIFY state=terminated
  -> BLF is off although SIP/klaus2 has still an active call on server2

4. SIP/klaus2 on server2 hangs up
  -> event is distributed to server1


What seems strange to me is that in devicestate.c in function
process_collection(...) you iterate over the per-server states:

   for (i = 0; i < collection->num_states; i++) {
     ast_debug(1, "Adding per-server state of '%s' for '%s'\n",
     ast_devstate2str(collection->states[i].state), device);
     ast_devstate_aggregate_add(&agg, collection->states[i].state);
   }

But the debug-message is always seen only once per state change. Thus,
to me it looks like as the processing does not process all states from
all servers but only the last received event.
Comments:By: Russell Bryant (russell) 2010-11-12 15:37:37.000-0600

I think I've got this fixed.  I replicated these same state changes by manually changing custom device states and it seems to be working now for me.  Would you mind trying it out and letting me know if it works for you?
By: klaus3000 (klaus3000) 2010-11-14 12:10:34.000-0600

I did the same tests as before and now it works as expected - at least I couldn't spot a problem. Thanks
By: klaus3000 (klaus3000) 2010-11-14 12:26:59.000-0600

FYI: This fixed also the problems I had with res_xmpp. So I do not know if ASTERISK-16933 is still valid.
By: klaus3000 (klaus3000) 2010-11-14 12:27:01.000-0600

FYI: This fixed also the problems I had with res_xmpp. So I do not know if ASTERISK-16933 is still valid.
By: Digium Subversion (svnbot) 2010-11-19 18:45:53.000-0600

Repository: asterisk
Revision: 295710

U   branches/1.6.2/include/asterisk/event.h
U   branches/1.6.2/main/event.c

------------------------------------------------------------------------
r295710 | russell | 2010-11-19 18:45:52 -0600 (Fri, 19 Nov 2010) | 29 lines

Fix cache of device state changes for multiple servers.

This patch addresses a regression where device states across multiple servers
were not being processing completely correctly.  The code works to determine
the overall state by looking at the last known state of a device on each
server.  However, there was a regression due to some invasive rewrites of how
the cache works that led to the cache only storing the last device state change
for a device, regardless of which server it was on.

The code is set up to cache device state change events by ensuring that each
event in the cache has a unique device name + entity ID (server ID).  The code
that was responsible for comparing raw information elements (which EID is)
always returned a match due to a memcmp() with a length of 0.

There isn't much code to fix the actual bug.  This patch also introduces a new
CLI command that was very useful for debugging this problem.  The command
allows you to dump the contents of the event cache.

(closes issue ASTERISK-16937)
Reported by: klaus3000
Patches:
     issue18284.rev1.txt uploaded by russell (license 2)
Tested by: russell, klaus3000

(closes issue ASTERISK-16933)
Reported by: klaus3000

Review: https://reviewboard.asterisk.org/r/1012/

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=295710
By: Digium Subversion (svnbot) 2010-11-19 18:50:00.000-0600

Repository: asterisk
Revision: 295711

_U  branches/1.8/
U   branches/1.8/include/asterisk/event.h
U   branches/1.8/main/event.c

------------------------------------------------------------------------
r295711 | russell | 2010-11-19 18:50:00 -0600 (Fri, 19 Nov 2010) | 36 lines

Merged revisions 295710 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.6.2

........
 r295710 | russell | 2010-11-19 18:45:51 -0600 (Fri, 19 Nov 2010) | 29 lines
 
 Fix cache of device state changes for multiple servers.
 
 This patch addresses a regression where device states across multiple servers
 were not being processing completely correctly.  The code works to determine
 the overall state by looking at the last known state of a device on each
 server.  However, there was a regression due to some invasive rewrites of how
 the cache works that led to the cache only storing the last device state change
 for a device, regardless of which server it was on.
 
 The code is set up to cache device state change events by ensuring that each
 event in the cache has a unique device name + entity ID (server ID).  The code
 that was responsible for comparing raw information elements (which EID is)
 always returned a match due to a memcmp() with a length of 0.
 
 There isn't much code to fix the actual bug.  This patch also introduces a new
 CLI command that was very useful for debugging this problem.  The command
 allows you to dump the contents of the event cache.
 
 (closes issue ASTERISK-16937)
 Reported by: klaus3000
 Patches:
       issue18284.rev1.txt uploaded by russell (license 2)
 Tested by: russell, klaus3000
 
 (closes issue ASTERISK-16933)
 Reported by: klaus3000
 
 Review: https://reviewboard.asterisk.org/r/1012/
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=295711
By: Digium Subversion (svnbot) 2010-11-19 18:52:48.000-0600

Repository: asterisk
Revision: 295712

_U  trunk/
U   trunk/include/asterisk/event.h
U   trunk/main/event.c

------------------------------------------------------------------------
r295712 | russell | 2010-11-19 18:52:48 -0600 (Fri, 19 Nov 2010) | 43 lines

Merged revisions 295711 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

................
 r295711 | russell | 2010-11-19 18:50:00 -0600 (Fri, 19 Nov 2010) | 36 lines
 
 Merged revisions 295710 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.6.2
 
 ........
   r295710 | russell | 2010-11-19 18:45:51 -0600 (Fri, 19 Nov 2010) | 29 lines
   
   Fix cache of device state changes for multiple servers.
   
   This patch addresses a regression where device states across multiple servers
   were not being processing completely correctly.  The code works to determine
   the overall state by looking at the last known state of a device on each
   server.  However, there was a regression due to some invasive rewrites of how
   the cache works that led to the cache only storing the last device state change
   for a device, regardless of which server it was on.
   
   The code is set up to cache device state change events by ensuring that each
   event in the cache has a unique device name + entity ID (server ID).  The code
   that was responsible for comparing raw information elements (which EID is)
   always returned a match due to a memcmp() with a length of 0.
   
   There isn't much code to fix the actual bug.  This patch also introduces a new
   CLI command that was very useful for debugging this problem.  The command
   allows you to dump the contents of the event cache.
   
   (closes issue ASTERISK-16937)
   Reported by: klaus3000
   Patches:
         issue18284.rev1.txt uploaded by russell (license 2)
   Tested by: russell, klaus3000
   
   (closes issue ASTERISK-16933)
   Reported by: klaus3000
   
   Review: https://reviewboard.asterisk.org/r/1012/
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=295712