[reSIProcate] Why TransactionState set mDnsRestult as 0

[Scott Godin]

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->copyOutboundDecoratorsToStackFailureAck(*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/20170321/a1aff492/attachment.htm>