Re: [reSIProcate] Why TransactionState set mDnsRestult as 0

[海滨 <lhb8859138@xxxxxxx>]

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@xxxxxxxxxxxxxxx> 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@xxxxxxx> 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@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@xxxxxxx> 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@xxxxxxxxxxxxxxx> wrote：

What is the source file and line number?

Thanks,

Scott

On Sat, Mar 18, 2017 at 8:29 PM, 海滨 <lhb8859138@xxxxxxx> 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@resiprocate.org
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel

 

_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@resiprocate.org
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel