SqlClient shows degraded performances with a simple SELECT query when enabling MARS

[joproulx]

This was originally posted in dotnet runtime repo but it was actually meant to be posted here.

Description

Repro steps:
With a sql connection configured with MultipleActiveResultSets=True, execute a simple SELECT query.

Ex: SELECT * FROM FooTable WHERE UserId='XXX'

The problem is more evident with a higher network latency between the client and server (ex: >=10ms ping) and a bigger payload to return (ex: >=5MB)

Actual results:
In my case, MARS-enabled request took ~2.2-2.3 seconds longer to complete vs the same request with MARS-disabled (~75-100ms).

Note that I believe that this number would increase linearly with ping time between client and server. See analysis below.

Expected results:
I would expect the request latency to be equivalent to the same SELECT request executed with MultipleActiveResultSets=False.

Configuration

• Which version of .NET is the code running on? 3.1.19
• What OS version, and what distro if applicable? Linux/Debian
• What is the architecture (x64, x86, ARM, ARM64)? x64
• If relevant, what are the specs of the machine? It doesn't seem to be related to a specific configuration. Tested on Windows and same results.

Regression?

Unknown

Data

Simple SELECT query returning 2000 rows (~4.8MB, ~600 packets of 8KB)
Round-trip time (ping) between client and server: ~11ms

===========================================================
System.Data.SqlClient (MARS enabled)
-----------------------------------------------------------
Execution #1: PacketSize=8192, Elapsed=2857 ms
Execution #2: PacketSize=8192, Elapsed=2368 ms
Execution #3: PacketSize=8192, Elapsed=2404 ms
Execution #4: PacketSize=8192, Elapsed=2356 ms
Execution #5: PacketSize=8192, Elapsed=2337 ms
===========================================================
System.Data.SqlClient (MARS disabled)
-----------------------------------------------------------
Execution #1: PacketSize=8192, Elapsed=260 ms
Execution #2: PacketSize=8192, Elapsed=83 ms
Execution #3: PacketSize=8192, Elapsed=68 ms
Execution #4: PacketSize=8192, Elapsed=73 ms
Execution #5: PacketSize=8192, Elapsed=76 ms
===========================================================
Microsoft.Data.SqlClient (MARS enabled)
-----------------------------------------------------------
Execution #1: PacketSize=8192, Elapsed=2783 ms
Execution #2: PacketSize=8192, Elapsed=2362 ms
Execution #3: PacketSize=8192, Elapsed=2366 ms
Execution #4: PacketSize=8192, Elapsed=2358 ms
Execution #5: PacketSize=8192, Elapsed=2368 ms
===========================================================
Microsoft.Data.SqlClient (MARS disabled)
-----------------------------------------------------------
Execution #1: PacketSize=8192, Elapsed=257 ms
Execution #2: PacketSize=8192, Elapsed=105 ms
Execution #3: PacketSize=8192, Elapsed=101 ms
Execution #4: PacketSize=8192, Elapsed=105 ms
Execution #5: PacketSize=8192, Elapsed=112 ms
===========================================================

Analysis

I sniffed the network and here is the pattern I think is problematic. This image represents a small part where server is transferring data to the client:

• MARS is enabled
• Sql Server 10.1.0.10
• SqlClient: 10.100.0.66
  

Or in other words:

...
   SQLServer -> SqlClient:  [TDS] Data (8K)
   SQLServer -> SqlClient:  [TDS] Data (8K)
   SQLServer -> SqlClient:  [TDS] Data (8K)
   SqlClient -> SQLServer:  [SMP] Ack (Window size = 3)
~11 ms later...
   SQLServer -> SqlClient:  [TDS] Data (8K)
   SQLServer -> SqlClient:  [TDS] Data (8K)
   SQLServer -> SqlClient:  [TDS] Data (8K)
   SqlClient -> SQLServer:  [SMP] Ack (Window size = 3)
~11 ms later
   SQLServer -> SqlClient:  [TDS] Data (8K)
...   

From what I understand of the SMP protocol, there is a flow control mechanism in place that work like this:
The server, after sending X amount of packets to the client, will stop sending more until it receives an ACK message from the client that indicates the new amount of packets that can be sent before stalling again. (I am paraphrasing, but I believe that's the gist of it).

In the implementation of SqlClient, the client only sends the ACK after receiving the last allowed packet in the window. The server will therefore stall for the equivalent of the network latency between client and server (5.5ms in my case) before sending another packet. And before the packet can reach the client, the same delay applies (another 5.5ms). So a total of 11ms between the reception of the two packets.

We seem to use a window size of 3 in SqlClient. So every 3 packets we receive, we have to wait a full round-trip (11ms in my case) before receiving another packet. When you have 600 packets to receive, it would mean 200 stalls of 11ms or a total of 2.2 seconds that would be added to the full request latency.

Even if we had a 1ms ping between client and server, that would still mean a 200ms delay added that could be avoided (if I am correct with my analysis).

From the code, we don't seem to have the possibility to adjust this algorithm with some configurations.

If I disable MARS, SMP is removed from the protocol stack (reference) which make the problem disappear as we don't have this flow control mechanism enabled.

There are probably good reasons why the MARS algorithm is implemented the way it is, but it seems problematic to stall the transfer while waiting for an ACK from the client. Am I wrong? Any idea if this could be avoided?

[Wraith2]

The limit is hardcoded in SNTMarsHandle:

/// <summary>
/// Send ACK if we've hit highwater threshold
/// </summary>
private void SendAckIfNecessary()
{
    uint receiveHighwater;
    uint receiveHighwaterLastAck;

    lock (this)
    {
        receiveHighwater = _receiveHighwater;
        receiveHighwaterLastAck = _receiveHighwaterLastAck;
    }

    if (receiveHighwater - receiveHighwaterLastAck > ACK_THRESHOLD)
    {
        SendControlPacket(SNISMUXFlags.SMUX_ACK);
        SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNIMarsHandle), EventType.INFO, "MARS Session Id {0}, _asyncReceives {1}, _receiveHighwater {2}, _sendHighwater {3}, _receiveHighwaterLastAck {4} Sending acknowledgment ACK_THRESHOLD {5}", args0: ConnectionId, args1: _asyncReceives, args2: _receiveHighwater, args3: _sendHighwater, args4: _receiveHighwaterLastAck, args5: ACK_THRESHOLD);
    }
}

where ACK_THRESHOLD is 2

We don't actually send a window size it's all handled in the code with the highwater mark, so we send the current highwater which will be 3 above the last one that was sent to the server. This seems to be the cause of the window size stall and the way to change it would probably be to add constant value to the highwater we send to the server so it'll send another packet after we reach the trigger to send an ack.

I'm not sure if we want to fiddle with it though. This is all related to the fairness of shared connections over the same physical connection that mars uses and causing one partner to send more data without waiting for others may cause flow issues.

I'm also not sure you should be using mars. Unless you positively absolutely need and and can't design your way around it then I'd advise not using it. It adds complexity and performance issues.

[roji]

Just a thought... I'd be interested in knowing whether the unmanaged Windows networking implementation also suffers from this issue - we've seen cases in the past where issues are specific to the managed implementation. If that's the case here as well, at the very least maybe the unmanaged implementation can provide a direction on how things should be done in the managed implementation as well.

[joproulx]

@Wraith2 We probably enabled MARS for a few cases where it was required and since we are sharing the same connection string everywhere in our project, we got to this issue. I will try to remove it and workaround the cases where we were needing it.

But still, it seems really easy to enable this in a connection string that gets shared in a project and get impacted without knowing it.

I suppose that if you use MARS in the intended way, the problem will be less apparent as you will do the round trip anyway when updating the data. The DATA message sent to the server will contain the new Window so won't need to send the ACK message on every 3 packets (?).

using (SqlDataReader reader = readCmd.ExecuteReader())  
{  
      while (reader.Read())  // ASSUMPTION: Receiving TDS DATA packets for one row. Less than 3 packets if data less than 24KB.
      {
           updateCmd.Parameter["XXX"] = reader["YYY"];
           updateCmd.ExecuteNonQuery();   // ASSUMPTION: Send TDS DATA packet with new SMP Window. ACK won't be necessary then.
      }
}

Some thoughts:

• Based on code example above, my assumption is that sending an ACK on every three packets is not the normal case. If we keep sending an ACK, then maybe we are doing something wrong. Is that a valid assumption? If so, would some kind of warning be adequate to notify that we might be using MARS where we shouldn't?
• If no fix is applied, should we consider documenting this more explicitly to warn of the performance impacts (maybe here)?
  Something like:

With MARS enabled, doing a simple sql SELECT query without any interleaved update will result in an increased latency equal to (ResultPayloadSize/PacketSize)/3*RoundtripTime.

What do you think?

---

@roji Not sure how to I can test with the unmanaged Windows networking implementation. Is the System.Data.SqlClient using the unmanaged implementation? My test gave the same result for System.Data.SqlClient and Microsoft.Data.SqlClient.

[Wraith2]

I'd be interested in knowing whether the unmanaged Windows networking implementation also suffers from this issue

This is a good point. The code I looked at is the managed version which given that you're running on linux is the one you'll be using but the unmanaged version may be different. That component is closed source so one of the MS team will need to investigate that. Long term having only a managed implementation is desirable so if we can change this without breaking anything it would be nice to do so.

Based on code example above, my assumption is that sending an ACK on every three packets is not the normal case.

It's the maximum period you can go without updating the highwater to let more packets flow from the server. As you say a request packet will always have the current value so will be an implicit reset of the period.

If you've got an easy repro test setup for this I can open PR with a +1 to the highwater and you could test the build artifact from it to see if it behaves sensibly.

I think it is worth documenting possible perf problems with MARS. We'll see what the team say. My opinion on general is that you shouldn't use mars if you can avoid it, it's complicated, expensive and a source of bugs.

[roji]

@roji Not sure how to I can test with the unmanaged Windows networking implementation. Is the System.Data.SqlClient using the unmanaged implementation? My test gave the same result for System.Data.SqlClient and Microsoft.Data.SqlClient.

I'm pretty sure that Microsoft.Data.SqlClient also uses unmanaged networking when running on Windows. For an apples-to-apples comparison, you can run use Microsoft.Data.SqlClient on Windows, once running as default (unmanaged networking), and once with the following AppContext switch which enables managed networking (which is used by default on Linux/Mac, but not on Windows):

var ManagedNetworkingAppContextSwitch = "Switch.Microsoft.Data.SqlClient.UseManagedNetworkingOnWindows";
AppContext.SetSwitch(ManagedNetworkingAppContextSwitch, true);

@Wraith2 or others can probably correct me if I wrote anything inaccurate above...

[Wraith2]

I'm pretty sure that Microsoft.Data.SqlClient also uses unmanaged networking when running on Windows.

Yes. By default on windows the native implementation is used,
That switch is correct. Be careful to use it in Main() or somewhere close to the start of the process so it's present the first time any Sql classes are used.

[JRahnama]

@joproulx thanks for bringing this up. MARS and its related issues are on our to-do list. We will resume working on them soon. I will take a closer look to this issue on Monday and we will discuss it internally and will update here next week,

[joproulx]

Thanks everyone for your input. I'll test the proposed configurations on Monday and will post the results if it can be of any help.

[joproulx]

For info, running the test on Windows with and without the UseManagedNetworkingOnWindows option give the same benchmark results as the one presented in the bug.

I tested with System.Data.SqlClient and Microsoft.Data.SqlClient.