You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[ARO-29321] Make the Cosmos DB change feed resilient to a page it cannot read - #5059
A Cosmos DB change feed is read by passing the Etag of each response back as the next
request's If-None-Match. The generated iterators store that position only after a
successful read, so a page which fails deterministically is requested again from the same
position on every poll, for the life of the process, and nothing downstream sees another
update.
One undecodable document is enough. pkg/database/extensions.go panics when a secure field
cannot be decrypted, and the codec reports that as a decode error; a document written in a
shape the reader does not understand fails the same way, with no encryption involved.
Readiness checks expose the condition at best. The scheduler and the monitor check change
feed freshness, so a stalled instance reports itself unready, but it stays stalled until it
is restarted. The gateway and the frontend check only that each feed has ever succeeded, and
nothing checks the gateway auth cache. This PR makes the condition recoverable everywhere.
Recovery. A hand-written iterator in pkg/database/cosmosdb re-implements Next with
the error path corrected. A failed read still leaves the next page's Etag in hand: _do
returns the response alongside a decode error, and do copies its headers into the
caller's map. After thirty-six consecutive failures at one position, the iterator advances
to that Etag. Without one (after a transport error, or a response which was not JSON) it
stays put, since a feed with no position starts again from the beginning, re-reads the
whole collection and arrives back at the same page.
Any success resets the count, so the threshold measures persistence, not frequency: a page
is skipped only when it cannot be read at all. Cluster documents are the only ones with
encrypted fields, and the scheduler and the monitor read them every ten seconds, so there
the thirty-sixth failure comes about six minutes after the first. That is longer than any
throttling or transient fault, and longer than the five minutes #5058 allows between
encryption key refreshes, so a page which fails only because the reader's keys are stale is
retried with fresh keys before it can be skipped. That depends on #5058, which should merge
first. Without it such a page is unreadable until the process restarts, as on master, but
the feed moves past it rather than stopping there.
Coverage. Every change feed wrapper in pkg/database now returns the resilient
iterator, with no change to any interface or call site. That covers each feed in use:
clusters and subscriptions (scheduler, monitor), gateway (gateway auth cache, gateway), and
OpenShift versions and platform workload identity role sets (frontend). The maintenance
manifest and schedule feeds have no reader but are converted too, so the rule has no
exceptions; a test enforces it.
Visibility.RunChangefeed now takes a metrics emitter and a feed name, and on every
poll emits the seconds since the last successful poll (changefeed.poll.staleness) and the
consecutive failures (changefeed.poll.failures), plus the pages skipped
(changefeed.pages.skipped) where the iterator can skip. That covers the scheduler, the
monitor and the gateway auth cache. The frontend and pkg/gateway read their feeds in
hand-written loops rather than RunChangefeed, so for now they get recovery without these
metrics; pkg/gateway already reports its last successful poll as gateway.lastchangefeed.
Shutdown. The MIMO scheduler's Run now waits for its change feeds before it reports
done, including when it panics, and they stop on its context rather than its stop channel,
so they exit however Run does.
The generated files are untouched. The generator has since moved into this repository
(cmd/gencosmosdb), so the correction could later move into its template, and this file
would go.
Test plan for issue:
pkg/database/cosmosdb/changefeed_test.go, against httptest.NewTLSServer: normal
iteration, recovery from an undecodable page, no advance without an Etag, and the
failure threshold. Disabling the recovery path fails three of the four.
TestChangefeedEmitsFailureMetrics checks the exact sequence of the failure counter
across a scripted feed.
TestChangefeedGivesEachEmissionItsOwnDimensions fails if RunChangefeed hands the
metrics emitter the same dimensions map twice. The statsd emitter writes into the map it
is given and encodes it on another goroutine, so a shared map can crash the process.
TestEveryChangeFeedIsResilient checks that every feed pkg/database hands out can skip
a page. Reverting any one wrapper fails its case.
TestSchedulerWaitsForChangefeedsBeforeReturning holds a feed inside Next and checks
that Run does not report done until the feed is released, whether Run exits through
its stop channel, its context or a panic. Without the wait every case fails; with the
feeds still watching the stop channel the context case hangs; with the wait at the end of Run rather than deferred, the panic case fails.
The affected packages pass, lint is clean, and the new tests pass under -race. Under -race, pkg/util/changefeed, pkg/monitor and pkg/mimo/scheduler also report races in
the generated fake clients and existing tests, at the same rate as on master. There is no
e2e coverage: the e2e suite cannot plant an undecodable document in Cosmos DB.
Is there any documentation that needs to be updated for this PR?
No. Everything sits behind the existing DocumentIterator interface. The metrics follow the changefeed.* naming and the {"name": "<DocumentType>"} dimension already used by changefeed.caches.size. Only iterators which can skip report changefeed.pages.skipped,
so a feed which cannot emits nothing, rather than a zero which would claim nothing was lost.
How do you know this will function as expected in production?
A stalled feed now shows as rising changefeed.poll.staleness with non-zero changefeed.poll.failures, per feed, rather than as undistinguished log lines. Both reset
when the feed recovers, so changefeed.pages.skipped only ever rises: a page whose
documents were never delivered is a fault which should outlast the recovery, and something
to alert on.
The recovery is deliberately conservative: it skips only after thirty-six consecutive
failures, and only to a position known to move forward. The residual risk is that a skipped
page's updates never reach the consumer. Every consumer is a cache built from a full
initial read, and the alternative today is that it receives nothing further at all.
Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR hardens Cosmos DB change feed consumption so deterministic decode failures (e.g., an unreadable/undecryptable document) don’t permanently stall downstream caches, and it adds per-feed observability to detect stalls quickly in production.
Changes:
Introduces a hand-written “resilient” Cosmos DB change feed iterator that can advance past an unreadable page after a consecutive-failure threshold, using the response Etag as the forward continuation.
Switches the pkg/database change feed wrappers (subscriptions, clusters, gateway) to use the resilient iterator without changing consumer call sites or interfaces.
Extends RunChangefeed to emit per-feed staleness and consecutive-failure gauges, and updates affected call sites/tests accordingly.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
File
Description
pkg/util/changefeed/subscriptioncache_test.go
Updates tests for the new RunChangefeed signature by passing a noop metrics emitter and feed name.
pkg/util/changefeed/changefeed.go
Adds per-poll staleness/failure metrics emission and feed naming for logs/metrics.
pkg/util/changefeed/changefeed_test.go
Adds a scripted test validating failure counter behavior and that staleness is emitted every poll.
pkg/otel/gatewayauth/gatewayauth.go
Passes metrics emitter and feed name into the gateway auth cache changefeed runner.
pkg/monitor/monitor.go
Passes metrics emitter and distinct feed names for cluster/subscription monitor changefeeds.
pkg/mimo/scheduler/service.go
Passes metrics emitter and feed names for scheduler changefeeds.
pkg/mimo/scheduler/service_test.go
Accounts for the newly emitted changefeed gauges in metrics assertions.
pkg/mimo/scheduler/manager_test.go
Accounts for the newly emitted changefeed gauges in metrics assertions.
pkg/database/subscriptions.go
Switches subscription change feed wrapper to the resilient iterator.
pkg/database/openshiftclusters.go
Switches cluster change feed wrapper to the resilient iterator.
pkg/database/gateway.go
Switches gateway change feed wrapper to the resilient iterator.
pkg/database/cosmosdb/changefeed.go
Adds the resilient iterator implementation and constructors for the three affected document types.
pkg/database/cosmosdb/changefeed_test.go
Adds unit tests covering unreadable page recovery, “no Etag => don’t advance”, reset-on-success, and periodic stall logging.
Suppressed comments (1)
pkg/util/changefeed/changefeed.go:26
The PR description calls out a changefeed.consecutive_failures metric, but MetricConsecutiveFailures is currently changefeed.poll.failures. Please align the metric name with the stated intent to avoid confusion and ensure monitoring queries use the correct series.
// MetricConsecutiveFailures is the number of polls which have failed in
// succession. It tells a stalled feed apart from a quiet one.
MetricConsecutiveFailures = "changefeed.poll.failures"
The reason will be displayed to describe this comment to others. Learn more.
🟢 Approval recommended
The change is well-scoped, updates all impacted call sites, and includes focused unit tests covering the critical failure/recovery and observability behaviors.
The reason will be displayed to describe this comment to others. Learn more.
The comment on changefeeds (lines 69–71) says the WaitGroup exists “so that a caller which closes their stop channel can wait for them to exit rather than leaving them running,” but Run() returns without calling s.changefeeds.Wait(). The goroutines do exit on their own once stop is closed, but Run() returning first means a caller that tears down resources immediately after (DB connections, etc.) races with two still-running changefeed goroutines. A s.changefeeds.Wait() after s.b.StopAndWait() would deliver what the comment promises.
The reason will be displayed to describe this comment to others. Learn more.
b66add1e0 makes Run wait for the feeds, and has them stop on ctx.Done() rather than stop, as the monitor's already do. 22526f27f makes the wait a deferred call, so it also holds when Run panics.
Wait() alone would hang Run whenever it exits other than by stop closing. Run also exits when its context is cancelled, which a bucket refresh failure does, and it accepts a nil stop, as TestSchedulerStopsIfBucketFailure passes. The context is cancelled in every case.
TestSchedulerWaitsForChangefeedsBeforeReturning holds a feed inside Next and checks that Run does not report done until the feed is released, whether Run exits through stop, its context or a panic. Without the wait every case fails. With the feeds still on stop, the context case hangs. With the wait at the end of Run rather than deferred, the panic case fails.
The reason will be displayed to describe this comment to others. Learn more.
The comment explains that the Etag is absent “when there was no response — a transport error, for instance.” There is a second case: when _do returns (nil, err) for a wrong Content-Type on an otherwise successful HTTP response, the response object is also nil, so do() skips the header copy and the Etag is equally absent. The guard handles both correctly; the docstring just only names one of them.
Two additional feeds are not covered by the resilient iterator introduced in this PR:
pkg/database/openshiftversions.go line 47 — This still returns the raw generated iterator. The frontend's changefeedOcpVersions goroutine reads from this via a hand-rolled loop that has no staleness metrics and no recovery path — a poison page here stalls the OCP version cache indefinitely. The three database wrappers covered by this PR (gateway, subscriptions, openshiftclusters) show the pattern: wrap the call in cosmosdb.NewResilient*. A NewResilientOpenShiftVersionDocumentChangeFeed along the same lines would close this gap.
pkg/database/platformworkloadidentityrolesets.go line 47 — Same situation: this returns the raw generated iterator, and changefeedRoleSets in pkg/frontend/changefeed.go reads it through a hand-rolled loop with no recovery and no metrics. The PWIRS cache gates cluster-creation requests, so a stalled feed here has user-visible impact. Wrapping this the same way as the other three wrappers would complete the coverage.
The generated change feed iterators advance their continuation only after a
successful read. A page which fails deterministically is therefore requested
again from the same position on every poll, for as long as the process runs,
and nothing downstream of the feed is updated again. A document which cannot
be decoded is enough to do this: pkg/database/extensions.go panics when a
secure field cannot be decrypted, and the codec turns that into a decode
error.
Add a hand-written resilient iterator in pkg/database/cosmosdb which
re-implements Next with that error path corrected, and substitute it at the
three pkg/database change feed wrappers. All five consumers are covered
without a change to any call site or interface.
The position to advance to is already in hand at the point of failure: do
copies the response headers into the caller's map whenever there was a
response, and _do returns the response alongside a decode error, so the Etag
naming the next page is present. It is absent when there was no response at
all, and the advance is conditional on it: an empty If-None-Match asks Cosmos
DB to read from the beginning of the feed, so advancing to "" would re-read
the whole collection and arrive back at the same page. Combined with a
threshold of thirty consecutive failures, reset on any success, a page is
only ever skipped when it cannot be read at all, never during a passing
fault.
The iterators are generated by an external tool, so the generated files are
left alone; if the change is taken upstream the new file can be deleted.
Add the visibility this needed and did not have. RunChangefeed now takes a
metrics emitter and a feed name, emits the seconds since the last successful
poll and the count of consecutive failures on every poll, and carries the
feed name as a log field. A stalled feed previously produced nothing but
undistinguished error lines.
TestProcessLoop started both changefeeds and left them running while it
asserted, so the gauges added to RunChangefeed were emitted concurrently
with the assertion. The fake emitter faults both an assertion with nothing
emitted and a metric emitted but not asserted, so the test failed either
way depending on timing.
Track the goroutines startChangefeeds launches, and close the stop channel
and wait for them before the assertions.
The consecutive-failure count resets on the next success, and staleness
resets with it, so once the iterator advances past a page it cannot read
the only surviving evidence is a log line. A page whose contents were
never delivered is a fault worth alerting on, and turning logs into
alerts is the wrong direction.
Count skips on the iterator and emit changefeed.pages.skipped, which only
ever rises. Reported through an optional interface, so an iterator which
cannot skip emits nothing rather than a permanent zero that would read as
a feed known to have lost nothing.
The changefeeds WaitGroup was documented as letting a caller wait for
the scheduler's change feeds to exit, but Run returned without waiting
for them; only a test did. A caller which released what the feeds use
once done closed could race with two feeds still running.
Wait for them after the workers. That alone is not enough: the feeds
stopped on the stop channel, and Run also exits when its context is
cancelled, as a bucket refresh failure does, and may be given a nil
stop channel. Waiting on feeds which watch only stop would then hang
Run. Have them stop on the context instead, as the monitor's already
do. It is cancelled when stop closes, and whenever Run exits.
The new test holds a change feed inside Next and checks that Run
reports done only once the feed is released, for both ways of exiting.
Removing the wait fails both cases, and waiting while the feeds still
watch stop hangs the second.
The comment on onFailure gave a transport error as the case in which
there is no Etag to advance to. There is another: _do returns no
response when one arrives with a Content-Type other than JSON, even
with the status expected, so do has no headers to copy. The guard
handles both already; the comment now names both.
The type's comment said the file could go if the change were taken
upstream into the generator. The generator has since moved into this
repository, in cmd/gencosmosdb, so the correction could move into its
template instead.
Three of the seven change feeds in pkg/database used the resilient
iterator. The OpenShift versions and platform workload identity role
sets feeds, which the frontend reads, still used the generated one, so
a page in either which could not be decoded would stall that feed for
as long as the process ran. The frontend's readiness check asks only
whether each feed has ever succeeded, so the stall would not be
reported, and the role sets cache gates cluster creation.
Neither document type has an encrypted field, so a stale key cannot
cause this there, but any other decode failure can, such as a document
written in a shape this process does not read.
Give both the resilient iterator, and the maintenance manifests and
schedules feeds too. Those two have no consumer, since ChangeFeed is
not on either interface; they are covered so that the rule has no
exceptions. TestEveryChangeFeedIsResilient enforces it for all seven,
including the three converted earlier, which nothing tested.
The frontend's two loops and the gateway's are hand-written rather than
built on RunChangefeed, so they gain the recovery but not the staleness
and failure metrics. Converting them is left for a follow-up.
A page which fails only because this process's encryption keys are
stale becomes readable once they are refreshed, which #5058 allows at
most once every five minutes. The feed gave up on a page after thirty
consecutive failures. At ten-second polls the thirtieth falls 290
seconds after the first, so a page could be skipped up to ten seconds
before the refresh which would have read it. That needs a refresh for
some other reason in the ten seconds before the page first failed, with
a new key version written after it, but the page's documents are then
never delivered.
Raise the threshold to thirty-six: six minutes at ten-second polls,
with room for a slow poll. A page which no key can read is skipped a
minute later than before.
Run waited for its change feeds at the end of its body. A panic after
the feeds had started skipped that: the deferred calls cancelled the
context and closed done, and recover.Panic then recovered, so done
reported Run finished while the feeds could still be running.
Make the wait a deferred call, registered once the feeds have started.
Deferred calls run last-in first-out, so it runs before done is closed,
on every path out of Run. It cancels the context first, since on a
panic nothing else has yet, and the feeds stop on it.
A third case in TestSchedulerWaitsForChangefeedsBeforeReturning makes
GetValid panic during the poll loop. It fails with the wait at the end
of Run, and times out if the deferred wait does not cancel first.
8ed5344b4: every change feed in pkg/database now returns the resilient iterator. That covers OpenShift versions and role sets, and also maintenance manifests and schedules, so the rule has no exceptions. ChangeFeed is not on either of those two interfaces and nothing reads them. TestEveryChangeFeedIsResilient checks all seven, including the three converted earlier, which nothing tested.
OpenShift version and role set documents have no encrypted fields, so stale keys cannot stall those feeds, but any other decode failure can. The frontend's readiness check asks only whether each feed has ever succeeded, so such a stall goes unreported, and the role sets cache gates cluster creation.
The frontend's two loops and pkg/gateway's are hand-written rather than built on RunChangefeed, so they get recovery but not metrics. Converting them is a larger change and is left for a follow-up.
363e6e8be: the skip threshold was 30 polls, 290 s from the first failure to the last, against the five-minute key refresh interval in [ARO-29320] Refresh encryption keys when a document cannot be opened #5058. A page failing only on stale keys could therefore be skipped up to ten seconds before the refresh that would have read it. The threshold is now 36.
b66add1e0 and 35b637ac4 address the two line comments, and 22526f27f extends b66add1e0 to a panic in Run, which Copilot raised. The branch is rebased onto master.
Under -race, pkg/util/changefeed, pkg/monitor and pkg/mimo/scheduler report data races in the generated fake Cosmos DB clients and some existing tests. They occur at the same rate on master, and CI does not run -race. I'll raise them separately.
Update documentation to reflect gateway feed metrics
pkg/otel/gatewayauth/gatewayauth.go:110
This change now routes the gateway feed through RunChangefeed with the metrics emitter, so the gateway emits changefeed.poll.staleness and changefeed.poll.failures (and pages.skipped when applicable). The PR description currently says the gateway uses a hand-written loop and gets recovery without metrics; please reconcile the description with this shipped behavior so operators know which feeds are instrumented.
The comment on the skip threshold claimed that every consumer polls
every ten seconds or less often. That is not so: gatewayauth takes its
interval from configuration, and accepts a shorter one.
The margin over the key refresh still holds. Only cluster documents
have encrypted fields, so only the cluster feed can stall on a stale
key. The scheduler and the monitor read that feed at a fixed ten
seconds, and gatewayauth reads gateway documents and holds no keys.
The comment now confines the margin to the cluster feed, and notes that
a faster reader of that feed would need the threshold expressed as a
time rather than a count.
RunChangefeed passed one dimensions map to every gauge it emitted, poll
after poll. The statsd emitter writes its extra dimensions into the map
it is given, on the caller's goroutine, and encodes it later on its
own. Each poll emits two or three gauges back to back, so the map can
be written while the previous emission is still being encoded. The Go
runtime treats that as fatal: a loop emitting back to back with one
shared map, against a running statsd emitter, ended with "concurrent
map iteration and map write" in ten runs of ten.
Build a fresh map for each emission. A test gives RunChangefeed an
emitter which, like statsd, writes into each map it receives, and fails
if any map is given to it twice.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
skippypull requests raised by member of Team Skippy
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cloudygreybeard
added
next-release
Which issue this PR addresses:
Fixes https://redhat.atlassian.net/browse/ARO-29321
What this PR does / why we need it:
A Cosmos DB change feed is read by passing the
Etagof each response back as the nextrequest's
If-None-Match. The generated iterators store that position only after asuccessful read, so a page which fails deterministically is requested again from the same
position on every poll, for the life of the process, and nothing downstream sees another
update.
One undecodable document is enough.
pkg/database/extensions.gopanics when a secure fieldcannot be decrypted, and the codec reports that as a decode error; a document written in a
shape the reader does not understand fails the same way, with no encryption involved.
Readiness checks expose the condition at best. The scheduler and the monitor check change
feed freshness, so a stalled instance reports itself unready, but it stays stalled until it
is restarted. The gateway and the frontend check only that each feed has ever succeeded, and
nothing checks the gateway auth cache. This PR makes the condition recoverable everywhere.
Recovery. A hand-written iterator in
pkg/database/cosmosdbre-implementsNextwiththe error path corrected. A failed read still leaves the next page's
Etagin hand:_doreturns the response alongside a decode error, and
docopies its headers into thecaller's map. After thirty-six consecutive failures at one position, the iterator advances
to that
Etag. Without one (after a transport error, or a response which was not JSON) itstays put, since a feed with no position starts again from the beginning, re-reads the
whole collection and arrives back at the same page.
Any success resets the count, so the threshold measures persistence, not frequency: a page
is skipped only when it cannot be read at all. Cluster documents are the only ones with
encrypted fields, and the scheduler and the monitor read them every ten seconds, so there
the thirty-sixth failure comes about six minutes after the first. That is longer than any
throttling or transient fault, and longer than the five minutes #5058 allows between
encryption key refreshes, so a page which fails only because the reader's keys are stale is
retried with fresh keys before it can be skipped. That depends on #5058, which should merge
first. Without it such a page is unreadable until the process restarts, as on master, but
the feed moves past it rather than stopping there.
Coverage. Every change feed wrapper in
pkg/databasenow returns the resilientiterator, with no change to any interface or call site. That covers each feed in use:
clusters and subscriptions (scheduler, monitor), gateway (gateway auth cache, gateway), and
OpenShift versions and platform workload identity role sets (frontend). The maintenance
manifest and schedule feeds have no reader but are converted too, so the rule has no
exceptions; a test enforces it.
Visibility.
RunChangefeednow takes a metrics emitter and a feed name, and on everypoll emits the seconds since the last successful poll (
changefeed.poll.staleness) and theconsecutive failures (
changefeed.poll.failures), plus the pages skipped(
changefeed.pages.skipped) where the iterator can skip. That covers the scheduler, themonitor and the gateway auth cache. The frontend and
pkg/gatewayread their feeds inhand-written loops rather than
RunChangefeed, so for now they get recovery without thesemetrics;
pkg/gatewayalready reports its last successful poll asgateway.lastchangefeed.Shutdown. The MIMO scheduler's
Runnow waits for its change feeds before it reportsdone, including when it panics, and they stop on its context rather than its stop channel,
so they exit however
Rundoes.The generated files are untouched. The generator has since moved into this repository
(
cmd/gencosmosdb), so the correction could later move into its template, and this filewould go.
Test plan for issue:
pkg/database/cosmosdb/changefeed_test.go, againsthttptest.NewTLSServer: normaliteration, recovery from an undecodable page, no advance without an
Etag, and thefailure threshold. Disabling the recovery path fails three of the four.
TestChangefeedEmitsFailureMetricschecks the exact sequence of the failure counteracross a scripted feed.
TestChangefeedGivesEachEmissionItsOwnDimensionsfails ifRunChangefeedhands themetrics emitter the same dimensions map twice. The statsd emitter writes into the map it
is given and encodes it on another goroutine, so a shared map can crash the process.
TestEveryChangeFeedIsResilientchecks that every feedpkg/databasehands out can skipa page. Reverting any one wrapper fails its case.
TestSchedulerWaitsForChangefeedsBeforeReturningholds a feed insideNextand checksthat
Rundoes not report done until the feed is released, whetherRunexits throughits stop channel, its context or a panic. Without the wait every case fails; with the
feeds still watching the stop channel the context case hangs; with the wait at the end of
Runrather than deferred, the panic case fails.The affected packages pass, lint is clean, and the new tests pass under
-race. Under-race,pkg/util/changefeed,pkg/monitorandpkg/mimo/scheduleralso report races inthe generated fake clients and existing tests, at the same rate as on master. There is no
e2e coverage: the e2e suite cannot plant an undecodable document in Cosmos DB.
Is there any documentation that needs to be updated for this PR?
No. Everything sits behind the existing
DocumentIteratorinterface. The metrics follow thechangefeed.*naming and the{"name": "<DocumentType>"}dimension already used bychangefeed.caches.size. Only iterators which can skip reportchangefeed.pages.skipped,so a feed which cannot emits nothing, rather than a zero which would claim nothing was lost.
How do you know this will function as expected in production?
A stalled feed now shows as rising
changefeed.poll.stalenesswith non-zerochangefeed.poll.failures, per feed, rather than as undistinguished log lines. Both resetwhen the feed recovers, so
changefeed.pages.skippedonly ever rises: a page whosedocuments were never delivered is a fault which should outlast the recovery, and something
to alert on.
The recovery is deliberately conservative: it skips only after thirty-six consecutive
failures, and only to a position known to move forward. The residual risk is that a skipped
page's updates never reach the consumer. Every consumer is a cache built from a full
initial read, and the alternative today is that it receives nothing further at all.