fix(historian): #683 review — a refused tag ADD INDEX only warns; document index trade-offs

- Bootstrapper: an ALTER ... ADD INDEX refused up front (e.g. no DDL rights)
  logs a Warning and lets the bootstrap succeed (the index is optional); a
  WAL-apply failure remains unobserved, like the TTL ALTER — documented.
  Integration test (VARCHAR tag => refused ALTER) fails with the catch removed.
- Historian.md: a dominant tag can read ~20% slower on long windows (single-
  threaded index scan vs parallel JIT filter); O3 writes rewrite index files;
  sample_modified.tag / event.source deliberately not indexed, and why.
- CLAUDE.md: the concurrent-HistoryUpdate-delete caveat on the range-first order.
This commit is contained in:
Joseph Doherty
2026-10-12 10:15:37 -04:00
parent 549d2d68b3
commit cc0cc8cd89
4 changed files with 54 additions and 7 deletions
+1 -1
View File
@@ -1350,7 +1350,7 @@ historian tagname, which is the tag's driver `FullName`.
### Pluggable backends, SQL Server and HistoryUpdate (2026-10) ### Pluggable backends, SQL Server and HistoryUpdate (2026-10)
- **Selection:** `ServerHistorian:Backend` (`Gateway` default | `SqlServer` | `QuestDb`; unknown fails host start). `HistorianBackendRegistry` → `IHistorianBackendProvider` builds all five seams; the gateway keys are only required for `Gateway`. `SqlServer` uses `ServerHistorian:SqlServer:{ConnectionString (required), Schema=hist, CommandTimeout=30s, AutoCreateSchema=true}`. - **Selection:** `ServerHistorian:Backend` (`Gateway` default | `SqlServer` | `QuestDb`; unknown fails host start). `HistorianBackendRegistry` → `IHistorianBackendProvider` builds all five seams; the gateway keys are only required for `Gateway`. `SqlServer` uses `ServerHistorian:SqlServer:{ConnectionString (required), Schema=hist, CommandTimeout=30s, AutoCreateSchema=true}`.
- **SQL Server** (`Driver.Historian.SqlServer`): `hist.Sample` PK `(TagId, SourceTimeUtc)` — writes are insert-if-absent, so outbox redelivery is idempotent. Stores every value kind with its **real status**; per-sample aggregates (10, simple definitions) are pushed down to T-SQL with bucket math in **100 ns ticks**; the Part 13 time-weighted seven (TimeAverage/TimeAverage2/Total/Total2/Interpolative, #671 — ⚠️ `Total` is now the time-weighted integral, no longer `SUM` — plus the **duration-based** PercentGood/PercentBad, #680) stream window + 4 bound rows into the shared `TimeWeightedAggregator` (both SQL backends, identical results; reverse `Interpolative` anchored at the later time; window capped by `ServerHistorian:MaxTimeWeightedRawRows` ⇒ `BadTooManyOperations`, percentages included). ⚠️ **#680 status rules (client-visible):** TimeAverage/Total are Uncertain if any part of the interval uses an Uncertain value, is bridged across a Bad sample, is extrapolated past the last sample (⇒ a live tag's latest interval is Uncertain) or is not covered; TimeAverage2/Total2 keep the SDK time-based rule (Uncertain time counts as Good) but never extrapolate past the last sample — that interval is Partial, later ones `BadNoData` (a report-by-exception tag with nothing after its last sample reads `BadNoData` until its next sample arrives). `docs/Historian.md` §"Time-weighted aggregates". ⚠️ **#680 per-sample (client-visible):** `Count` counts **Good** samples only (value-based status; an all-Bad interval is generic Bad, value null) and an empty interval **inside the tag's data range** is `0`/Good (outside it still `BadNoData`); `Start`/`End` return the **raw** first/last sample of any quality — its own timestamp and status, value null when Bad; `Delta` is Uncertain only when the interval's first or last sample is unusable; ⚠️ #684: `Average`/`Minimum`/`Maximum`/`Range`/`*ActualTime` use **Good samples only** (was Good + Uncertain; an Uncertain-only interval is `BadNoData`), Min/Max-family status Uncertain on a Bad sample, an Uncertain value beyond the Good extreme (Part 13 — the SDK ignores that) or an Uncertain non-numeric sample, and only a **Good** String/DateTime sample refuses them (`BadAggregateInvalidInputs`; Delta still refuses any usable one); and every per-sample aggregate but `Average` sets **end-of-data Partial** on the interval holding the last sample (forward incl. its earlier edge, reverse strictly inside), never on a Bad result. The data-range lookup is a key seek on SQL Server and a `tag` symbol-index seek on QuestDB (#683 — `{p}sample.tag` is `SYMBOL INDEX`; the bootstrapper ADDs it to a pre-#683 table, idempotent), read BEFORE the bucket statement so a racing append can never yield a Good `Count` of 0. `docs/Historian.md` §"Aggregates". Docker-dev recipe, schema and aggregate semantics: `docs/Historian.md` §"SQL Server backend". Integration tests need `HISTORIAN_SQL_TEST_CONNECTION`. **Retention (#667):** opt-in `SqlServer:RetentionDays` (+ `PurgeInterval` 1h, `PurgeBatchSize` 4000) -> `HistorianRetentionService` purges old `hist.Sample` (per tag) / `hist.Event` in `DELETE TOP(n)` batches, Primary-gated on `IHistorizationRoleView`, one purger per DB via `sp_getapplock`; counter `otopcua.historian.retention.purged{table}`. Optional `IHistorianBackendProvider.CreateRetention` (default null). `docs/Historian.md` §Retention. - **SQL Server** (`Driver.Historian.SqlServer`): `hist.Sample` PK `(TagId, SourceTimeUtc)` — writes are insert-if-absent, so outbox redelivery is idempotent. Stores every value kind with its **real status**; per-sample aggregates (10, simple definitions) are pushed down to T-SQL with bucket math in **100 ns ticks**; the Part 13 time-weighted seven (TimeAverage/TimeAverage2/Total/Total2/Interpolative, #671 — ⚠️ `Total` is now the time-weighted integral, no longer `SUM` — plus the **duration-based** PercentGood/PercentBad, #680) stream window + 4 bound rows into the shared `TimeWeightedAggregator` (both SQL backends, identical results; reverse `Interpolative` anchored at the later time; window capped by `ServerHistorian:MaxTimeWeightedRawRows` ⇒ `BadTooManyOperations`, percentages included). ⚠️ **#680 status rules (client-visible):** TimeAverage/Total are Uncertain if any part of the interval uses an Uncertain value, is bridged across a Bad sample, is extrapolated past the last sample (⇒ a live tag's latest interval is Uncertain) or is not covered; TimeAverage2/Total2 keep the SDK time-based rule (Uncertain time counts as Good) but never extrapolate past the last sample — that interval is Partial, later ones `BadNoData` (a report-by-exception tag with nothing after its last sample reads `BadNoData` until its next sample arrives). `docs/Historian.md` §"Time-weighted aggregates". ⚠️ **#680 per-sample (client-visible):** `Count` counts **Good** samples only (value-based status; an all-Bad interval is generic Bad, value null) and an empty interval **inside the tag's data range** is `0`/Good (outside it still `BadNoData`); `Start`/`End` return the **raw** first/last sample of any quality — its own timestamp and status, value null when Bad; `Delta` is Uncertain only when the interval's first or last sample is unusable; ⚠️ #684: `Average`/`Minimum`/`Maximum`/`Range`/`*ActualTime` use **Good samples only** (was Good + Uncertain; an Uncertain-only interval is `BadNoData`), Min/Max-family status Uncertain on a Bad sample, an Uncertain value beyond the Good extreme (Part 13 — the SDK ignores that) or an Uncertain non-numeric sample, and only a **Good** String/DateTime sample refuses them (`BadAggregateInvalidInputs`; Delta still refuses any usable one); and every per-sample aggregate but `Average` sets **end-of-data Partial** on the interval holding the last sample (forward incl. its earlier edge, reverse strictly inside), never on a Bad result. The data-range lookup is a key seek on SQL Server and a `tag` symbol-index seek on QuestDB (#683 — `{p}sample.tag` is `SYMBOL INDEX`; the bootstrapper ADDs it to a pre-#683 table, idempotent), read BEFORE the bucket statement so a racing append can never yield a Good `Count` of 0 (a concurrent HistoryUpdate delete is the mirror case and still can, transiently). `docs/Historian.md` §"Aggregates". Docker-dev recipe, schema and aggregate semantics: `docs/Historian.md` §"SQL Server backend". Integration tests need `HISTORIAN_SQL_TEST_CONNECTION`. **Retention (#667):** opt-in `SqlServer:RetentionDays` (+ `PurgeInterval` 1h, `PurgeBatchSize` 4000) -> `HistorianRetentionService` purges old `hist.Sample` (per tag) / `hist.Event` in `DELETE TOP(n)` batches, Primary-gated on `IHistorizationRoleView`, one purger per DB via `sp_getapplock`; counter `otopcua.historian.retention.purged{table}`. Optional `IHistorianBackendProvider.CreateRetention` (default null). `docs/Historian.md` §Retention.
- **QuestDB** (`Driver.Historian.QuestDb`, `ServerHistorian:QuestDb:*`, Npgsql over PG wire; docker-dev overlay `docker-compose.questdb.yml`): same kinds/status/aggregates/HistoryUpdate as SqlServer, soft delete by tombstone (INSERT only — never `UPDATE`/`DELETE`; every read filters `NOT deleted`). ⚠️ **Timestamps are µs** (`QuestDbConnection.Round`/`BindTimestamp`; `IHistorianDataSource.TimestampResolutionTicks` = 10 steps HistoryRead paging), **pre-1970 cannot be stored** (writer drops, HistoryUpdate `BadOutOfRange`), every write waits for WAL visibility, TTL is enforced on write commits only, set Npgsql `Maximum Pool Size` < QuestDB's 64. `docs/Historian.md` §"QuestDB backend". - **QuestDB** (`Driver.Historian.QuestDb`, `ServerHistorian:QuestDb:*`, Npgsql over PG wire; docker-dev overlay `docker-compose.questdb.yml`): same kinds/status/aggregates/HistoryUpdate as SqlServer, soft delete by tombstone (INSERT only — never `UPDATE`/`DELETE`; every read filters `NOT deleted`). ⚠️ **Timestamps are µs** (`QuestDbConnection.Round`/`BindTimestamp`; `IHistorianDataSource.TimestampResolutionTicks` = 10 steps HistoryRead paging), **pre-1970 cannot be stored** (writer drops, HistoryUpdate `BadOutOfRange`), every write waits for WAL visibility, TTL is enforced on write commits only, set Npgsql `Maximum Pool Size` < QuestDB's 64. `docs/Historian.md` §"QuestDB backend".
- ⚠️ **Reverse processed reads are Part 13/SDK-anchored (#677, client-visible):** every aggregate on SqlServer/QuestDB now anchors a reverse read at its later time — window `(end, start]`, partial interval earliest, each stamped with its later edge (was: earlier-anchored, stamped earlier edge); on forward reads Partial also marks the interval the data starts inside (not Average — the SDK's rule, forward-only); a non-default client `AggregateConfiguration` ⇒ `BadAggregateConfigurationRejected`; Gateway provisioning sends `IntegralDivisor = 1`. SDK-parity tests: `SdkAggregateParityTests`. `docs/Historian.md` §Aggregates. - ⚠️ **Reverse processed reads are Part 13/SDK-anchored (#677, client-visible):** every aggregate on SqlServer/QuestDB now anchors a reverse read at its later time — window `(end, start]`, partial interval earliest, each stamped with its later edge (was: earlier-anchored, stamped earlier edge); on forward reads Partial also marks the interval the data starts inside (not Average — the SDK's rule, forward-only); a non-default client `AggregateConfiguration` ⇒ `BadAggregateConfigurationRejected`; Gateway provisioning sends `IntegralDivisor = 1`. SDK-parity tests: `SdkAggregateParityTests`. `docs/Historian.md` §Aggregates.
- **Advertised aggregates (#682):** `IHistorianDataSource.SupportedAggregates` (Gateway 4 via `GatewayHistorianDataSource.ServedAggregates`, SqlServer/QuestDB 17 via `AggregateFamilies.Served`, Null **none**; a null set ⇒ all) is the one authority for `HistoryServerCapabilities/AggregateFunctions` (i=11201 — only its folder references are trimmed) and the up-front `BadAggregateNotSupported` in HistoryRead-Processed. ⚠️ `ServerCapabilities/AggregateFunctions` (i=2997) is deliberately NOT trimmed: it keeps the SDK's 37 because live `AggregateFilter` subscriptions still compute them all. ⚠️ Historian disabled ⇒ Processed is now `BadAggregateNotSupported`, not `GoodNoData`. `docs/Historian.md` §Advertised aggregates. - **Advertised aggregates (#682):** `IHistorianDataSource.SupportedAggregates` (Gateway 4 via `GatewayHistorianDataSource.ServedAggregates`, SqlServer/QuestDB 17 via `AggregateFamilies.Served`, Null **none**; a null set ⇒ all) is the one authority for `HistoryServerCapabilities/AggregateFunctions` (i=11201 — only its folder references are trimmed) and the up-front `BadAggregateNotSupported` in HistoryRead-Processed. ⚠️ `ServerCapabilities/AggregateFunctions` (i=2997) is deliberately NOT trimmed: it keeps the SDK's 37 because live `AggregateFilter` subscriptions still compute them all. ⚠️ Historian disabled ⇒ Processed is now `BadAggregateNotSupported`, not `GoodNoData`. `docs/Historian.md` §Advertised aggregates.
+16 -4
View File
@@ -1481,10 +1481,22 @@ tag scan every partition on one side of the window. A table created before #683
bootstrapper adds it on first use (`ALTER TABLE {p}sample ALTER COLUMN tag ADD INDEX`, a WAL transaction; idempotent — re-sending it on an bootstrapper adds it on first use (`ALTER TABLE {p}sample ALTER COLUMN tag ADD INDEX`, a WAL transaction; idempotent — re-sending it on an
indexed column is harmless, so a pair booting together is safe — and a boot that finds the column indexed sends nothing). Building it is a indexed column is harmless, so a pair booting together is safe — and a boot that finds the column indexed sends nothing). Building it is a
one-off WAL apply, measured **~3 s for 68 M rows / 63 daily partitions**, during which writes queue and reads keep scanning. With one-off WAL apply, measured **~3 s for 68 M rows / 63 daily partitions**, during which writes queue and reads keep scanning. With
`AutoCreateSchema=false` a missing index only logs a Warning naming that statement — reads stay correct, just slower. Cost: ~12 % more `AutoCreateSchema=false` a missing index only logs a Warning naming that statement — reads stay correct, just slower. With
disk (0.5 GB on a 4.2 GB 53 M-row table); no measurable change in live-append, O3 back-fill or bulk-load apply time beyond run-to-run noise `AutoCreateSchema=true` an ALTER refused up front (e.g. no DDL rights) is likewise only a Warning — the bootstrap still succeeds and the
(a first bulk load into an empty table measured ~40 % slower), and the per-sample / raw / time-weighted window queries were unchanged or next boot retries; a failure at **WAL-apply** time is not observed by the bootstrapper (QuestDB would suspend the table, visible in
slightly faster (the tag filter becomes an index scan). `wal_tables()`) — the same blind spot as the TTL ALTER. Cost: ~12 % more disk (0.5 GB on a 4.2 GB 53 M-row table); no measurable change
in live-append, O3 back-fill or bulk-load apply time beyond run-to-run noise (a first bulk load into an empty table measured ~40 %
slower). Out-of-order writes (back-fill, HistoryUpdate, outbox redelivery) rewrite the index files of every partition they touch along
with the column files, so their apply cost carries the index too. On a 120-tag table the per-sample / raw / time-weighted window queries
were unchanged or slightly faster (the tag filter becomes an index scan), but **a tag holding a large share of the table can be ~20 %
slower on long windows**: the index scan is single-threaded where the unindexed filter is a multi-threaded JIT scan (a review
measurement: 4 tags / 30 M rows, 245–294 ms indexed vs 195–218 ms unindexed; with 1000 tags, 9 ms vs 13 ms — the index wins as the
tag's share of each partition shrinks).
**Deliberately not indexed:** `{p}sample_modified.tag` — read only by `IsReadModified` HistoryRead over a window, an append-only trail a
tiny fraction of the sample table's size, with no "nearest row outside the window" lookup to make it scan partitions; and
`{p}event.source` — event reads are window-bounded (`ts` range, optionally `source = x`) and never seek beyond the window, and the event table is far
smaller than the sample table. Neither has the unbounded-scan shape #683 fixes, so neither pays the index's disk and O3 cost.
`status` holds the uint OPC UA status code **unsigned** in a `LONG` (so "Bad" is `status >= 2147483648`, not `< 0`); `int_value` also holds `status` holds the uint OPC UA status code **unsigned** in a `LONG` (so "Bad" is `status >= 2147483648`, not `< 0`); `int_value` also holds
Booleans as 0/1; `kind` is the `HistorianValueKind` in a `BYTE`. Tag names are limited to 400 characters (longer is a per-tag failure, Booleans as 0/1; `kind` is the `HistorianValueKind` in a `BYTE`. Tag names are limited to 400 characters (longer is a per-tag failure,
@@ -19,7 +19,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.QuestDb;
/// <item><b>Tag index (#683):</b> a sample table created before the <c>tag</c> symbol index existed gains it with /// <item><b>Tag index (#683):</b> a sample table created before the <c>tag</c> symbol index existed gains it with
/// <see cref="QuestDbTables.AddTagIndex"/> (idempotent; a boot that finds it indexed issues no ALTER, and like the TTL /// <see cref="QuestDbTables.AddTagIndex"/> (idempotent; a boot that finds it indexed issues no ALTER, and like the TTL
/// the ALTER is a WAL transaction the bootstrapper does not re-read). Building it is a one-off WAL apply — ~3 s for /// the ALTER is a WAL transaction the bootstrapper does not re-read). Building it is a one-off WAL apply — ~3 s for
/// 68M rows / 63 daily partitions measured — during which writes queue and reads keep scanning.</item> /// 68M rows / 63 daily partitions measured — during which writes queue and reads keep scanning. A refused ALTER is
/// logged as a Warning, never fails the bootstrap (the index is optional); a failure at WAL-apply time would suspend
/// the table unseen, the same blind spot as the TTL ALTER.</item>
/// <item><b>Failure:</b> not latched. Within <see cref="BootstrapFailureCache.DefaultDuration"/> the last failure is /// <item><b>Failure:</b> not latched. Within <see cref="BootstrapFailureCache.DefaultDuration"/> the last failure is
/// re-thrown without touching QuestDB.</item> /// re-thrown without touching QuestDB.</item>
/// <item><c>AutoCreateSchema=false</c> only verifies the tables exist (no TTL changes) and throws /// <item><c>AutoCreateSchema=false</c> only verifies the tables exist (no TTL changes) and throws
@@ -145,8 +147,21 @@ internal sealed class QuestDbSchemaBootstrapper
return; return;
} }
// A WAL transaction: table_columns() keeps reporting false until it applies, so do NOT re-read to verify. // A WAL transaction: table_columns() keeps reporting false until it applies, so do NOT re-read to verify.
await using (var alter = _conn.CreateCommand(conn, QuestDbTables.AddTagIndex(table))) // The index is optional (reads are correct without it), so a refused ALTER (e.g. no DDL rights) must not fail the
// bootstrap: it is logged and retried on the next boot. A failure at WAL-APPLY time is not seen here (QuestDB would
// suspend the table) — the same blind spot as the TTL ALTER.
try
{
await using var alter = _conn.CreateCommand(conn, QuestDbTables.AddTagIndex(table));
await alter.ExecuteNonQueryAsync(ct).ConfigureAwait(false); await alter.ExecuteNonQueryAsync(ct).ConfigureAwait(false);
}
catch (Exception ex) when (ex is not OperationCanceledException || !ct.IsCancellationRequested)
{
_log.LogWarning("Could not add the tag index to QuestDB table {Table} ({Exception}: {Message}); data-range lookups " +
"of processed reads scan partitions instead of seeking (#683). Run: {Statement}",
table, ex.GetType().Name, ex.Message, QuestDbTables.AddTagIndex(table));
return;
}
_log.LogInformation("Added the tag index to QuestDB table {Table} (#683).", table); _log.LogInformation("Added the tag index to QuestDB table {Table} (#683).", table);
} }
@@ -83,6 +83,26 @@ public sealed class TagIndexTests(QuestDbFixture fx)
(await b.TagIndexedAsync()).ShouldBeTrue(); (await b.TagIndexedAsync()).ShouldBeTrue();
} }
[Fact]
public async Task A_refused_ADD_INDEX_does_not_fail_the_bootstrap()
{
fx.SkipIfUnavailable();
// A sample table whose tag column cannot be indexed (VARCHAR, not SYMBOL): the ALTER is refused up front, which
// stands in for any refusal (e.g. no DDL rights). The index is optional, so the bootstrap must still succeed.
var p = fx.UniquePrefix();
var o = fx.Options(p);
var conn = new QuestDbConnection(o);
await using (var c = await conn.OpenAsync(Ct))
{
var ddl = QuestDbTables.CreateSample(p, null).Replace("tag SYMBOL INDEX", "tag VARCHAR", StringComparison.Ordinal);
await using var cmd = conn.CreateCommand(c, ddl);
await cmd.ExecuteNonQueryAsync(Ct);
await using var alter = conn.CreateCommand(c, QuestDbTables.AddTagIndex(conn.SampleTable));
await Should.ThrowAsync<Exception>(() => alter.ExecuteNonQueryAsync(Ct)); // precondition: really refused
}
await new QuestDbSchemaBootstrapper(conn, o).EnsureAsync(Ct);
}
[Fact] [Fact]
public async Task The_data_range_seeks_the_tag_index() public async Task The_data_range_seeks_the_tag_index()
{ {