[reSIProcate] Why TransactionState set mDnsRestult as 0

[Scott Godin]

Thank Edwin!  I have committed this change to GitHub trunk.

Scott

On Wed, Mar 22, 2017 at 3:35 AM, 海滨 <lhb8859138 at 163.com> wrote:

> I have done the test for this change and it works well now.
> once receive the 503 response, the dns entry is placed in blacklist, and
> all other subsequent requests(i.e. INVITE,OPTION) to this dns entry are
> blocked for the duration retry-after in 503 response specifies.
>
> Thanks
>
>
>
> At 2017-03-22 00:44:56, "Scott Godin" <sgodin at sipspectrum.com> wrote:
>
> Hi Edwin,
>
> I see - nice catch!  I think we should be moving the if(mDnsResult) block
> to after the sendToTU call.  ie:
>
>                   if (mState == Calling || mState == Proceeding)
>                   {
>                      // MUST pass the received response up to the TU, and
> the client
>                      // transaction MUST generate an ACK request, even if
> the transport is
>                      // reliable, if transport is Unreliable then Fire the
> Timer D which
>                      // take care of re-Transmission of ACK
>                      mState = Completed;
>                      mController.mTimers.add(Timer::TimerD, mId,
> Timer::TD );
>                      SipMessage* ack = Helper::makeFailureAck(*mNextTransmission,
> *sip);
>                      mNextTransmission->copyOutboundDecoratorsToStackF
> ailureAck(*ack);
>                      resetNextTransmission(ack);
>                      sendCurrentToWire();
>                      sendToTU(sip); // don't delete msg
>                      if(mDnsResult)
>                      {
>                         mDnsResult->destroy();
>                         mDnsResult=0;
>                         mPendingOperation=None;
>                      }
>                   }
>
> Can you test this change out, and if everything looks good, I will commit
> it.
>
> Thanks for helping out!
> Scott
>
> On Tue, Mar 21, 2017 at 9:32 AM, 海滨 <lhb8859138 at 163.com> wrote:
>
>> If we set the mDnsResult as 0 when receive the 503 response for the udp
>> transaction, the dns entry seems never been blacklisted in sendToTu
>> function even though the retry-after value is more than 0. is it reasonable?
>> Thanks
>> Edwin Li
>>
>>
>>
>>
>>
>>
>>
>> 在 2017-03-21 02:36:57，"Scott Godin" <sgodin at sipspectrum.com> wrote：
>>
>> Here are the check-in notes.  Are you seeing a problem with this change?
>>
>> Thanks,
>> Scott
>>
>> * Two changes that were somewhat tangled together, since they both used
>> the same
>> refactoring of the Transport SendData code.
>>
>> 1) State shedding modifications to TransactionState
>>
>>    In a number of cases, we were preserving state (in the form of
>> SipMessages
>>    and DnsResults) in cases where we did not really need them any more.
>> For
>>    example, once we have transmitted a response, there is no need
>>    to preserve the full SipMessage for this response (the raw retransmit
>> buffer
>>    is sufficient). Also, INVITE requests do not need to be maintained
>> once
>>    a final response comes in (since there is no possibility that we'll
>> need to
>>    send a simulated 408 or 503 to the TU, nor will we need to construct a
>> CANCEL
>>    request using the INVITE, nor will we need to retransmit). Similarly,
>> once we
>>    have received a final response for a NIT transaction, we no longer
>> need to
>>    maintain the original request or the retransmit buffer. Lastly, if we
>> are
>>    using a reliable transport, we do not need to maintain retransmit
>> buffers
>>    (although we may need to maintain full original requests for simulated
>>    responses and such).
>>
>>    This change has basically no impact on reliable NIT performance, but a
>> huge
>>    impact on non-reliable and INVITE performance. Prior to this change,
>> either
>>    NIT UDP or INVITE TCP testStack would exhaust main memory on my laptop
>> (with
>>    4GB of main memory), bringing progress to a complete halt on runs
>> longer than
>>    15 seconds or so. I did not bother trying INVITE UDP, but that works
>> now too.
>>
>> 2) Reduction in buffer reallocations while encoding a SipMessage
>>
>>    TransportSelector now keeps a moving average of the outgoing message
>> size,
>>    which is used to preallocate the buffers into which SipMessages are
>> encoded.
>>
>>    This ends up making a small difference in testStack when linked
>> against google
>>    malloc, but a larger difference when linked against OS X's (horrible)
>> standard
>>    malloc.
>>
>>
>>
>> On Mon, Mar 20, 2017 at 11:29 AM, 海滨 <lhb8859138 at 163.com> wrote:
>>
>>> 1.10.2: TransactionState.cxx line1322~1327
>>> The mDnsResult is set to 0 here, which is different from 1.7.
>>>
>>> Thanks
>>> Edwin Li
>>>
>>>
>>>
>>>
>>>
>>>
>>> 在 2017-03-19 23:13:53，"Scott Godin" <sgodin at sipspectrum.com> wrote：
>>>
>>> What is the source file and line number?
>>>
>>> Thanks,
>>> Scott
>>>
>>> On Sat, Mar 18, 2017 at 8:29 PM, 海滨 <lhb8859138 at 163.com> wrote:
>>>
>>>>
>>>>
>>>> Hi all,
>>>>
>>>>
>>>> Does anyone know why TransactionState set mDnsRestult as 0 when receive
>>>> the 503 response from wire if mIsReliable=false from release 1.8, instead
>>>> of blacklist the last result(which is implemented in 1.7)? I can't see any
>>>> information about this change in the release note.
>>>>
>>>>
>>>> Thanks
>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> resiprocate-devel mailing list
>>>> resiprocate-devel at resiprocate.org
>>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>>
>>>
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> resiprocate-devel mailing list
>>> resiprocate-devel at resiprocate.org
>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>>
>>
>>
>>
>>
>>
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20170322/0e7b12d6/attachment.htm>