[reSIProcate] Why TransactionState set mDnsRestult as 0

[海滨]

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->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/20170322/91356474/attachment.htm>