Skip to content

Commit 978ae51

Browse files
Reduce SQL Server queue length monitoring query volume (#5555)
* ✨ Reduce SQL Server queue length monitoring query volume Replace the per-queue length probe (one IF EXISTS + max-min(RowVersion) statement per tracked queue, every 200ms) with a single catalog-view query per catalog that reads approximate row counts from sys.partitions. This reads no queue table data, takes no locks, and needs only SELECT on the queue tables. Also make the poll interval configurable via the QueueLengthQueryDelayInterval connection string part (default 200ms), and add optional adaptive back-off up to QueueLengthQueryMaxDelayInterval while all monitored queues are empty (disabled by default; base interval is always used while any queue has work, preserving the fix for #4556). * ♻️ Address PR review: Unquoted* naming, primary ctor, concurrent pacing SqlTable: rename Name/Schema/Catalog to Unquoted* to make the quoted-vs-unquoted contract explicit (review feedback), and convert to a primary constructor. The now-dead per-table LengthQuery's duplicated if/else is collapsed into BuildFullTableName/BuildLengthQuery helpers. QueueLengthProvider: pace the poll interval concurrently with the query so the effective cadence is max(interval, queryDuration) instead of interval + queryDuration. The additive query-time term was what let the cadence drift past the 1s monitoring bucket and starve buckets of samples (#4556 false-zero sawtooth). With drift removed, restore sane defaults — base 1s (matches the finest monitoring bucket) ramping to a 10s idle ceiling — superseding the #4557 200ms oversampling workaround. Adaptive back-off is now ON by default. * ♻️ Address review: throttle error loop, immediate back-off snap-back, NOLOCK, validation - Pace after the query (subtracting iteration time) instead of pre-starting Task.Delay: a fast persistent failure no longer spins the loop with zero delay, and a back-off snap-back to base takes effect on the current wait instead of one backed-off wait later. - Reject non-positive QueueLengthQuery(Max)DelayInterval up front (a negative value previously threw inside Task.Delay mid-loop; zero meant no pacing). - Record 0 for a not-found table so a dropped queue no longer pins the cadence at base. - Statement-scoped WITH (NOLOCK) on the catalog views instead of a connection-scoped SET TRANSACTION ISOLATION LEVEL that would persist on the pooled connection. - Guard BuildBulkLengthQuery against an empty table set (would emit invalid `AND ()`). - Remove now-dead per-table LengthQuery/BuildLengthQuery. Co-authored-by: Mauro Servienti <mauro.servienti@gmail.com>
1 parent c2a00a5 commit 978ae51

5 files changed

Lines changed: 388 additions & 69 deletions

File tree

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
namespace ServiceControl.Transport.Tests;
2+
3+
using System;
4+
using System.Linq;
5+
using NUnit.Framework;
6+
using Transports.SqlServer;
7+
8+
[TestFixture]
9+
class QueueLengthQueryTests
10+
{
11+
[Test]
12+
public void BuildBulkLengthQuery_emits_a_single_catalog_view_query_for_all_tables()
13+
{
14+
var tables = new[]
15+
{
16+
SqlTable.Parse("Sales", "dbo"),
17+
SqlTable.Parse("Billing", "dbo"),
18+
};
19+
20+
var query = SqlTable.BuildBulkLengthQuery(catalog: null, tables);
21+
22+
// One statement, read from the catalog views — no per-table IF EXISTS, no scan of the queue tables.
23+
Assert.That(query, Does.Contain("sys.partitions"));
24+
Assert.That(query, Does.Contain("p.index_id IN (0, 1)"));
25+
Assert.That(query, Does.Not.Contain("INFORMATION_SCHEMA"));
26+
Assert.That(query, Does.Not.Contain("RowVersion"));
27+
28+
// Dirty reads are scoped to this statement via NOLOCK hints, not a connection-scoped SET that
29+
// would leak READ UNCOMMITTED onto the pooled connection.
30+
Assert.That(query, Does.Contain("WITH (NOLOCK)"));
31+
Assert.That(query, Does.Not.Contain("ISOLATION LEVEL"));
32+
33+
// Both tracked tables are covered by the single query.
34+
Assert.That(query, Does.Contain("t.name = 'Sales'"));
35+
Assert.That(query, Does.Contain("t.name = 'Billing'"));
36+
37+
// Exactly one result-producing statement.
38+
Assert.That(System.Text.RegularExpressions.Regex.Matches(query, "SELECT").Count, Is.EqualTo(1));
39+
}
40+
41+
[Test]
42+
public void BuildBulkLengthQuery_with_no_tables_stays_valid_and_returns_no_rows()
43+
{
44+
var query = SqlTable.BuildBulkLengthQuery(catalog: null, []);
45+
46+
// No predicate terms must not produce the invalid `AND ()`; a constant-false predicate keeps the
47+
// query syntactically valid while matching nothing.
48+
Assert.That(query, Does.Not.Contain("AND ()"));
49+
Assert.That(query, Does.Contain("1 = 0"));
50+
}
51+
52+
[Test]
53+
public void BuildBulkLengthQuery_prefixes_the_catalog_when_supplied()
54+
{
55+
var tables = new[] { SqlTable.Parse("Sales@dbo@MyCatalog", "dbo") };
56+
57+
var query = SqlTable.BuildBulkLengthQuery(catalog: "MyCatalog", tables);
58+
59+
Assert.That(query, Does.Contain("[MyCatalog].sys.partitions"));
60+
Assert.That(query, Does.Contain("[MyCatalog].sys.tables"));
61+
Assert.That(query, Does.Contain("[MyCatalog].sys.schemas"));
62+
}
63+
64+
[Test]
65+
public void RemoveQueueLengthQueryDelayInterval_parses_and_strips_the_custom_part()
66+
{
67+
const string connectionString = "Data Source=.;Initial Catalog=nsb;Integrated Security=true;QueueLengthQueryDelayInterval=1000";
68+
69+
var cleaned = connectionString.RemoveQueueLengthQueryDelayInterval(out var interval);
70+
71+
Assert.That(interval, Is.EqualTo(TimeSpan.FromSeconds(1)));
72+
// The custom key must be stripped so SqlConnection never sees the unknown keyword.
73+
Assert.That(cleaned, Does.Not.Contain("QueueLengthQueryDelayInterval").IgnoreCase);
74+
Assert.That(cleaned, Does.Contain("Initial Catalog=nsb").IgnoreCase);
75+
}
76+
77+
[Test]
78+
public void RemoveQueueLengthQueryDelayInterval_defaults_to_null_when_absent()
79+
{
80+
const string connectionString = "Data Source=.;Initial Catalog=nsb;Integrated Security=true";
81+
82+
connectionString.RemoveQueueLengthQueryDelayInterval(out var interval);
83+
84+
Assert.That(interval, Is.Null);
85+
}
86+
87+
[Test]
88+
public void RemoveQueueLengthQueryDelayInterval_throws_on_non_numeric_value()
89+
{
90+
const string connectionString = "Data Source=.;Initial Catalog=nsb;QueueLengthQueryDelayInterval=soon";
91+
92+
Assert.That(() => connectionString.RemoveQueueLengthQueryDelayInterval(out _),
93+
Throws.Exception.With.Message.Contains("QueueLengthQueryDelayInterval"));
94+
}
95+
96+
[Test]
97+
public void RemoveQueueLengthQueryDelayInterval_throws_on_negative_value()
98+
{
99+
const string connectionString = "Data Source=.;Initial Catalog=nsb;QueueLengthQueryDelayInterval=-5";
100+
101+
// A negative interval would flow through to Task.Delay and throw mid-loop; reject it up front.
102+
Assert.That(() => connectionString.RemoveQueueLengthQueryDelayInterval(out _),
103+
Throws.Exception.With.Message.Contains("QueueLengthQueryDelayInterval"));
104+
}
105+
106+
[Test]
107+
public void RemoveQueueLengthQueryDelayInterval_throws_on_zero_value()
108+
{
109+
const string connectionString = "Data Source=.;Initial Catalog=nsb;QueueLengthQueryDelayInterval=0";
110+
111+
// Zero would poll with no pacing at all; reject it.
112+
Assert.That(() => connectionString.RemoveQueueLengthQueryDelayInterval(out _),
113+
Throws.Exception.With.Message.Contains("QueueLengthQueryDelayInterval"));
114+
}
115+
116+
[Test]
117+
public void RemoveQueueLengthQueryMaxDelayInterval_parses_and_strips_the_custom_part()
118+
{
119+
const string connectionString = "Data Source=.;Initial Catalog=nsb;QueueLengthQueryMaxDelayInterval=60000";
120+
121+
var cleaned = connectionString.RemoveQueueLengthQueryMaxDelayInterval(out var interval);
122+
123+
Assert.That(interval, Is.EqualTo(TimeSpan.FromSeconds(60)));
124+
Assert.That(cleaned, Does.Not.Contain("QueueLengthQueryMaxDelayInterval").IgnoreCase);
125+
}
126+
127+
static readonly TimeSpan Base = TimeSpan.FromMilliseconds(200);
128+
static readonly TimeSpan Max = TimeSpan.FromSeconds(60);
129+
130+
[Test]
131+
public void NextDelay_snaps_back_to_base_when_any_queue_has_work()
132+
{
133+
// Even fully backed off, a single non-empty queue returns to full speed immediately.
134+
Assert.That(QueueLengthProvider.NextDelay(Max, Base, Max, maxObservedLength: 1), Is.EqualTo(Base));
135+
}
136+
137+
[Test]
138+
public void NextDelay_doubles_while_idle()
139+
{
140+
Assert.That(QueueLengthProvider.NextDelay(Base, Base, Max, maxObservedLength: 0),
141+
Is.EqualTo(TimeSpan.FromMilliseconds(400)));
142+
}
143+
144+
[Test]
145+
public void NextDelay_caps_at_max_while_idle()
146+
{
147+
Assert.That(QueueLengthProvider.NextDelay(TimeSpan.FromSeconds(40), Base, Max, maxObservedLength: 0),
148+
Is.EqualTo(Max));
149+
}
150+
151+
[Test]
152+
public void NextDelay_never_drops_below_base()
153+
{
154+
Assert.That(QueueLengthProvider.NextDelay(TimeSpan.FromMilliseconds(50), Base, Max, maxObservedLength: 0),
155+
Is.EqualTo(Base));
156+
}
157+
158+
[Test]
159+
public void NextDelay_with_max_equal_to_base_disables_backoff()
160+
{
161+
// Operator did not opt in (max == base) -> cadence is constant at base, even while idle.
162+
Assert.That(QueueLengthProvider.NextDelay(Base, Base, Base, maxObservedLength: 0), Is.EqualTo(Base));
163+
}
164+
}

src/ServiceControl.Transports.SqlServer/ConnectionStringExtensions.cs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
namespace ServiceControl.Transports.SqlServer
22
{
3+
using System;
34
using System.Data.Common;
45

56
static class ConnectionStringExtensions
@@ -11,6 +12,39 @@ public static string RemoveCustomConnectionStringParts(this string connectionStr
1112
.RemoveCustomConnectionStringPart(subscriptionsTableName, out subscriptionTable);
1213
}
1314

15+
// Extracts the optional, ServiceControl-specific 'QueueLengthQueryDelayInterval' (milliseconds) from
16+
// the connection string and removes it so it is never handed to SqlConnection (which would reject the
17+
// unknown keyword). Mirrors the existing convention used by the Azure Service Bus transport.
18+
// This is the BASE interval used while any monitored queue has messages.
19+
public static string RemoveQueueLengthQueryDelayInterval(this string connectionString, out TimeSpan? interval) =>
20+
connectionString.RemoveIntervalMilliseconds(queueLengthQueryDelayInterval, out interval);
21+
22+
// Extracts the optional 'QueueLengthQueryMaxDelayInterval' (milliseconds) — the upper bound the adaptive
23+
// back-off ramps to while every monitored queue is empty. When omitted a default ceiling applies; set it
24+
// equal to the base interval to disable back-off.
25+
public static string RemoveQueueLengthQueryMaxDelayInterval(this string connectionString, out TimeSpan? interval) =>
26+
connectionString.RemoveIntervalMilliseconds(queueLengthQueryMaxDelayInterval, out interval);
27+
28+
static string RemoveIntervalMilliseconds(this string connectionString, string key, out TimeSpan? interval)
29+
{
30+
interval = null;
31+
32+
var builder = new DbConnectionStringBuilder { ConnectionString = connectionString };
33+
34+
if (builder.TryGetValue(key, out var value))
35+
{
36+
if (!int.TryParse(value.ToString(), out var milliseconds) || milliseconds <= 0)
37+
{
38+
throw new Exception($"Can't parse '{value}' as a valid {key} (expected an integer number of milliseconds greater than zero).");
39+
}
40+
41+
interval = TimeSpan.FromMilliseconds(milliseconds);
42+
builder.Remove(key);
43+
}
44+
45+
return builder.ConnectionString;
46+
}
47+
1448
public static string RemoveCustomConnectionStringPart(this string connectionString, string partName, out string schema)
1549
{
1650
var builder = new DbConnectionStringBuilder
@@ -30,5 +64,7 @@ public static string RemoveCustomConnectionStringPart(this string connectionStri
3064

3165
const string queueSchemaName = "Queue Schema";
3266
const string subscriptionsTableName = "Subscriptions Table";
67+
const string queueLengthQueryDelayInterval = "QueueLengthQueryDelayInterval";
68+
const string queueLengthQueryMaxDelayInterval = "QueueLengthQueryMaxDelayInterval";
3369
}
3470
}

0 commit comments

Comments
 (0)