[asterisk-dev] [Code Review] func_srv and explicit specification of destination for SIP outgoing INVITEs

Tue Apr 13 11:29:03 CDT 2010

> On 2010-04-07 12:22:48, Tilghman Lesher wrote:
> > /trunk/funcs/func_srv.c, lines 55-57
> > <https://reviewboard.asterisk.org/r/608/diff/2/?file=9146#file9146line55>
> >
> >     I would suggest allowing the service name as the ID.  This would permit you to get rid of the initiation query altogether, by checking to see if you have a result in your cache and doing the query if not; otherwise, return the particular result from the query cache.
> 
> Mark Michelson wrote:
>     This is certainly easy to accomplish, but I have two reasons to not want to do this, one contrived and one rational.
>     
>     Contrived reason: By doing as you suggested, it would not be possible to perform more than one lookup for the same service on a single call.
>     Rational reason: Consistency. We have two similar constructs in use with ENUMQUERY and ENUMRESULT, and DUNDIQUERY and DUNDIRESULT. My intention here was to mirror those constructs as much as possible.
> 
> Mark Michelson wrote:
>     Tilghman and I had a discussion offline and came to a conclusion that works as a compromise for both our views.
>     
>     SRVQUERY will return, as an ID, not an integer, but rather the name of the service being queried. Then, this gets passed as the ID to SRVRESULT.
>     SRVRESULT will then be "smart" enough to do the SRV lookup itself if results from a previous SRVQUERY could not be found.
>     
>     The result is that people who want to use SRVQUERY and SRVRESULT together can do so, but those who wish to bypass SRVQUERY and just use SRVRESULT on its own may also do so. I'll upload a new patch with this behavior shortly.
> 
> Russell Bryant wrote:
>     I don't really like this change that much.
>     
>     Using the service as the ID means it is only possible to do a single lookup.  What if a dialplan author would like to be able to do the lookup more than once?  The results may not always be the same for each query.  Mark, you suggested this and I don't think it's a contrived example at all.
> 
> Tilghman Lesher wrote:
>     Then you would additionally need another function, something that would tell the channel to purge its cache of the query data.  We discussed this a bit, mainly to note that DNS has a TTL on each record, and it would be helpful if a) the record could be cached globally, because the dialplan is likely to be doing the same queries with multiple channels.  The only problem with this approach is that you would, at the same time, b) need to ensure that the result in cache did not last beyond the TTL in the DNS result.
>     
>     We decided this would be done at a later time.  The probability that a query result would change in the time that a single call is executing in the dialplan is extremely small.
> 
> Russell Bryant wrote:
>     I'm not sure that TTL is directly related to this.  It would be nice to have that exposed as one of the parameters you can get from the result.  I suppose a timestamp of when the lookup was done would be good, too.  Whether or not our code does some automatic TTL handling is not dependent on whether or not it is possible to do more than one lookup for the same service string in the same call.
>     
>     The probability of the list of records changing may be small, but it's non-zero.  Perhaps more relevant is that the order of the records will very likely change every time.  Records of the same weight and priority are given a random sort.  (See process_weights() in main/srv.c).

Let me throw my hat in the ring here for this discussion and address points brought forth by Tilghman and Russell.

First of all, caching SRV lookups is a good idea, but it seems outside the scope of this. Tilghman and I already discussed this earlier.
Second, Exposing the TTL for an SRV record would not be especially difficult to do, so I can add that if desired.
Third, the sorting of the records does not seem especially important to me in this case since the user will have access to all returned records and can inspect all their properties before taking action.

Now, so as not to be choosing sides here as far as the implementation of the SRVQUERY and SRVRESULT functions go, let me offer another compromise on the matter. We can still use the service as the ID. This way, if someone wants to bypass the SRVQUERY and just use SRVRESULT, he can. I can also modify SRVQUERY so that it will first clear any existing SRV result datastores on the channel that correspond to the service being queried, then perform the lookup and save the results. This way, whenever someone executes an SRVQUERY, a new query will be done. The limiting factor here will be that if you choose to bypass the SRVQUERY phase and go straight to SRVRESULT, you won't be able to perform multiple SRV lookups on the same service.

- Mark

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/608/#review1812
-----------------------------------------------------------

On 2010-04-07 15:28:47, Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/608/
> -----------------------------------------------------------
> 
> (Updated 2010-04-07 15:28:47)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> There are two interrelated changes here.
> 
> First, there is the introduction of func_srv. This adds two new read-only dialplan functions, SRVQUERY and SRVRESULT. They work very similarly to the ENUMQUERY and ENUMRESULT functions, except that this allows one to query SRV records instead. In order to facilitate this work, I added a couple of new API calls to srv.h. ast_srv_get_record_count tells the number of records returned by an SRV lookup. This number is calculated at the time of the SRV lookup. ast_srv_get_nth_record allows one to get a numbered SRV record.
> 
> Second, there is the modification to chan_sip that allows one to specify a hostname or IP address (along with a port) to send an outgoing INVITE to when dialing a SIP peer. This goes hand-in-hand with func_srv. You can query SRV records and then use the host and port from the results to dial via a specific host instead of what is configured in sip.conf.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 256421 
>   /trunk/funcs/func_srv.c PRE-CREATION 
>   /trunk/include/asterisk/srv.h 256421 
>   /trunk/main/srv.c 256421 
> 
> Diff: https://reviewboard.asterisk.org/r/608/diff
> 
> 
> Testing
> -------
> 
> I have written two external tests which exercise the individual components of this patch. They will be uploaded in a separate code review.
> 
> 
> Thanks,
> 
> Mark
> 
>