Summary:	ASTERISK-19172: Inconstistency for realtime colmn lastms
Reporter:	Marcello Ceschia (marcelloceschia)	Labels:
Date Opened:	2012-01-09 05:20:55.000-0600	Date Closed:	2012-02-07 15:57:56.000-0600
Priority:	Trivial	Regression?
Status:	Closed/Complete	Components:	Resources/res_config_pgsql
Versions:	1.8.8.1	Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) asterisk-19172-set-lastms-unreachable-1.8.patch ( 1) fix_realtime_lastms.diff
Description:	UPDATE sip_conf SET fullcontact = '', ipaddr = '', port = '', regseconds = '0', regserver = '', useragent = '', lastms = '' WHERE name = '20' [Jan  9 11:12:58] DEBUG[32503] res_config_pgsql.c: PostgreSQL RealTime: Query Failed because: FEHLER:  ungültige Eingabesyntax für ganze Zahl: »« The definition of lastms in http://svn.asterisk.org/svn/asterisk/branches/1.8/contrib/realtime/postgresql/realtime.sql is: lastms integer DEFAULT -1 NOT NULL So this will never set the correct state.
Comments:	By: Michael L. Young (elguero) 2012-01-09 12:24:29.890-0600 What leads up to this?  Did the registration expire? Try the attached patch.  It would appear that in PostgreSQL, if you have a default value set for an int column, you will get this message (ERROR: invalid input syntax for integer: <thanks to google translate> ) if the value it is being set to is blank.  This is because the PostgreSQL doesn't know if the value should be zero or null. The patch that I will attach sets lastms to -1 which should indicate to Asterisk that the peer is Unreachable according to what I am getting from the code. Sidenote, I noticed that in the contrib/realtime/mysql/sippeers.sql, the default value is NULL, which is different from the postgresql schema under contrib/realtime/postgresql/realtime.sql which, as you indicated, is -1. By: Marcello Ceschia (marcelloceschia) 2012-01-09 12:52:00.375-0600 Thank you for this patch, I did the same some time ago. ASTERISK-17791 You can see the result - issue is still present. I think the value should be -1, because we know that peer is not responding. Maybe it can also be NULL, but creating an error with the result that no update will be done is not a solution. By: Michael L. Young (elguero) 2012-01-09 13:10:11.113-0600 Ah... I did not know the history on this... next time, it would be helpful to mention or link the prior issue. I agree with you on this.  Asterisk does use peer->lastms in peer_status and if peer->lastms is < 0, it sets it to Unreachable.  I would think that the database should reflect this; especially in a failover situation... how is another asterisk server to know that this peer is unreachable if we do not set it properly before the failover occurs?  Why even have lastms in the database then? Just some quick thoughts on this based on the code checking I did when looking into this.  Perhaps, I am missing something.  Tilghman would know better than I, though on this, and I see he chimed in on the other issue. By: Marcello Ceschia (marcelloceschia) 2012-01-09 13:25:46.879-0600 Thank you for your support. I have a workaround for this by using views, but maybe some other users does not have a solution for this. By: Terry Wilson (twilson) 2012-01-31 18:43:40.415-0600 A cursory examination leads me to believe that the contrib postgresql file is wrong and should not be "not null default -1", but instead should allow NULL fields. It also looks wrong on several other fields that should be nullable. I will try to lab things up to verify this is the case, though. By: Terry Wilson (twilson) 2012-01-31 20:29:59.888-0600 There are also other problems as well. For instance if there is no ipaddr/port chan_sip passes in the results of ast_sockaddr_stringify for each which ends up being the string "(null)" instead of "". We need to check ast_sockaddr_isnull and ast_sockaddr_port() before passing those. Also, there are 4 missing required fields in the sip_conf table, and ipaddr and nat are both too short ('force_rport' won't fit in 5 chars, and ipaddr should be 46ish for IPv6). BUT! Neither the pgsql nor odbc backends handle empty values well. Even if you do have an empty string for an int field, they don't interpret it as a NULL and just pass on the empty string to the database. The database will then happily either choke (e.g. postgres) or work (e.g. sqlite3). So, while we handle *reading* NULL values well in realtime, it looks like we don't handle *writing* them at all. So, at this point my opinion has changed. It would be a major pain to support writing NULLs. Every backend would have to be modified. There would need to be a way to differentiate for actual empty strings and NULLs via the APIs, etc. I'm still not a big fan of there not being a distinction between -1 (unreachable) and NULL (haven't checked). I'll keep looking for a solution I like. By: Terry Wilson (twilson) 2012-01-31 21:45:14.651-0600 Also, it looks like lastms is initialized to 0 by default and that the code in handle_response_peerpoke (where peer->lastms is set) is pretty explicit that lastms == 0 means unknown. So, what the default (and the update to clear) really should be is 0, not -1 or  NULL. Another option would probably be to set the field to a character type (like port), but in any case 0 should be the default. I'll attach a patch that has several of the fixes I've found. By: Terry Wilson (twilson) 2012-01-31 21:50:54.478-0600 Please test fix_realtime_lastms.diff and make sure that it still solves the issue for you. I've tested it with both res_config_pgsql and res_config_odbc. On peer destruction it sets lastms to 0 (unknown), ensures that we don't set ipaddr or port to the string "(null)", and updates contrib/realtime/postgresql/realtime.sql to set the lastms defulat to 0, increases the ipaddr field length, and adds the missing (and required) defaultuser, fullcontact, regserver, and useragent fields. By: Marcello Ceschia (marcelloceschia) 2012-02-08 01:40:07.511-0600 This works for me. By: Maciej Krajewski (jamicque) 2012-02-13 15:24:26.229-0600 works for me also.