[jwrosewell]

A Cloud Request Engine with a cache now fetches its evidence keys and properties before its first process, whatever else is in the pipeline, and fetches them once rather than for every process, so its cache key is built from the Evidence the cloud lists and a repeated request is served from the cache with no further call to the cloud.

Problem

Two faults, the second found in review.

1. The cache key took every Evidence value. The engine's cache key is evidenceKeyFilter.filterEvidence(flowData.evidence.getAll()) (engine.js lines 90 to 92). The Cloud Request Engine's filter is the one that accepts every key until getEvidenceKeys replaces it with the cloud's list, and that only happens through ready(). Only a cloud engine calls ready() (cloudEngine.js line 93), so a pipeline with a Cloud Request Engine and no cloud engine keyed its cache on every Evidence value. Since #53 put the Sequence Element first, every request carries a new query.session-id or a higher query.sequence, so such a pipeline missed the cache on every request.
2. The keys and properties were fetched again for every process. keysAndPropertiesFetched tested this.evidenceKeyFilter.length > 0 (cloudRequestEngine.js lines 168 to 170 at aa6fccc). The only thing ever assigned to that field after construction is a BasicListEvidenceKeyFilter (line 531), which holds its keys in list and has no length, so the test was undefined > 0 and never true. ready() fetches whenever the test is false, so every call to ready() made a fresh get of evidencekeys and of accessibleproperties. On rc that already happens on every process of a pipeline with a cloud engine, because the cloud engine calls ready() in processInternal (cloudEngine.js line 126 at 56d91a1). Before the second fix, this draft's own process took the same path on every process, so a repeated request was served from the cache and still made two calls to the cloud.

What the specification says

The Start-up activity section of cloud-request-engine.md at d72eb074 says

The result from evidencekeys populates the accepted Evidence for the Engine.

and

The Engine MUST attempt both discovery requests when it is built (in its builder or constructor), and MAY make them in parallel, so that in the normal case a built Engine is fully initialised and immediately ready to process Flow Data.

and, for a failure,

The Engine MUST log a warning, complete the build, and retry discovery on first use. Failures of those first-use retries are process-time failures

The section asks for discovery once, at build, with a retry on first use only after a failure. Nothing in it asks for discovery on every process.

What .NET does

.NET fetches the evidence keys the first time anything reads EvidenceKeyFilter, through a Lazy set up in the constructor (CloudRequestEngine.cs lines 292 to 304 and the getter at lines 361 to 375, at rc 2b5c9eda). The cache key reads that filter, so the first process fetches the keys whatever else is in the pipeline, and a Lazy holds its value once made, so the keys are fetched once. CloudRequestEngineBuilder also calls WarmUp (CloudRequestEngineBuilder.cs line 376), which makes both discovery requests at build (CloudRequestEngine.cs line 460).

What changed

Line numbers are at the head, 56d91a1.

• CloudRequestEngine gains a process (lines 244 to 264) that, until the evidence keys and properties are fetched, waits for ready() before the base engine's process, which is where the cache key is built. Requests that arrive while the fetch runs share it. A fetch that fails rejects the process, which the pipeline handles as any other process error, and the next process tries again, as the section above asks of a first-use retry. This is the first-use half of .NET's behaviour.
• getEvidenceKeys sets a new evidenceKeysFetched when the keys arrive (line 546), the constructor starts it false (lines 163 and 164), and keysAndPropertiesFetched tests it in place of the filter's length (lines 179 to 182). Once both have arrived, ready() resolves with no request and process goes straight to the base engine, for this engine's own process and for every cloud engine's ready(). A fetch of either that fails leaves the test false, so the next process fetches both again, as before.
• The types are updated in types/cloudRequestEngine.d.ts, and the comment above the cache tests' pipeline no longer gives the missing evidence keys as a reason.
• c87777a gives the two updateOnStartMissingFile.test.js files, for Device Detection and IP Intelligence, 60 seconds a test as the named constant DownloadTestTimeoutMs. Each test serves, downloads and unpacks the whole Lite data file, and on Windows with Node 22 "asks for the data file with no If-Modified-Since header" went past the 10 second timeout twice, in run 42907092247 for Give each web integration element its own switch, put Sequence first, and give it element data #53 and in this draft's first full build below, each time in a build that changed nothing near it.

#48, stacked on #46, makes the same evidenceKeysFetched change to the same three places, line for line, so a merge of the two meets the same text on both sides. #48's own test, which counts the requests made through a cloud engine, still adds cover this draft does not have.

Tests

• serves a repeated request from the cache with no cloud engine in pipeline/fiftyone.pipeline.cloudrequestengine/tests/cloudCache.test.js builds a default pipeline, so the Sequence Element is first, holding a Cloud Request Engine with a cache and no cloud engine, and does not call ready(). It processes the same request twice, checks the two session ids differ, and checks the cloud was asked once. The existing serves a repeated request from the cache in a default pipeline calls ready() first, as a cloud engine does, so it passed before.
• fetches the evidence keys and properties once for many requests, in the same file (lines 122 to 140 at 56d91a1), processes three requests through the same pipeline and then calls ready(), as a cloud engine would, and checks the client made two gets, one for the evidence keys and one for the accessible properties, and one post. CountingClient now counts gets as well as posts, because under a resource key both discovery requests are gets.

Earlier runs, on earlier heads. Every temporary run in this list sits in one workflow file shared by the temporary runs, so its page can carry the name of another branch in its title. Each run's own branch is named here.

• Red, on branch tmp/node-cachekeys-red, run 43016664427, the test commit 7e99681. The first new test fails and the other 7 in the file pass.
• Green, on branch tmp/node-cachekeys-green, run 43016687993, 579253c with every Cloud Request Engine and core test. 480 of 483 pass, and the 3 that fail are the live tests in integration.test.js, which need a resource key that temporary run does not carry.
• First full build, on 579253c, run 43016967762. Windows with Node 22 failed 1 of 1076 tests, the download timeout above, and the other jobs were cancelled.
• Full build on c87777a, success, run 43019134914.
• a584579 merges rc at 5228a9f9, among them #61 and #72, which both change cloudRequestEngine.js. The merge was clean. 9246dfb gives processInternal its own comment back and puts process after it. Neither changes behaviour.
• Full build on 9246dfb, run 43055883741, failed on Ubuntu ARM with Node 24, 1 of 1199. The failing test, evidence never supplies resource, license or values, came with Never send resource, license, licence or values from Evidence to the cloud #61 and records every post the engine makes. Under a license key the engine now asks for its accessible properties by post before its first process, so the test saw two posts where it expected one. aa6fccc has the test answer that post with the mock's properties and record only the request it reads. Green, on branch tmp/node-cachekeys-green2, run 43057644264.
• Full build on aa6fccc, success, run 43057973583.

Runs for the second fix, each a temporary workflow on Ubuntu with Node 22 running cloudCache.test.js, cloudRequestEngine.test.js and cloudEngine.test.js, in a workflow file named for its own branch.

• 7bef061 merges rc at f76d8b05, which had taken #55 and #45. The merge was clean.
• Red, on branch tmp/node-73-red6, run 43111925905, the test commit 70749c0 with the temporary workflow. fetches the evidence keys and properties once for many requests fails with "Expected: 2, Received: 8", being two gets for each of the three processes and two more for ready(), and the other 32 of 33 pass.
• Green, on branch tmp/node-73-green6, run 43111942036, the fix 56d91a1 with the temporary workflow. All 33 pass.
• An earlier red and green, on tmp/node-73-red5 and tmp/node-73-green5, and a full build on tmp/pr-matrix-73-f, were created during the GitHub Actions outage of 11 November and never started, so they are not cited.
• Full build on the head 56d91a1, run 43111851218, failed on Ubuntu ARM with Node 22, 1 of 1225, and the other jobs were cancelled. The failing test is shareUsageBackOff.test.js "share usage back-off - full queue drops new evidence", which sent four of the six entries it expects to be dropped. This draft changes no file in usage sharing, the core or the engines package, the same test passed on both Ubuntu ARM jobs of #75's full build on the same rc, and #77's full build failed the same way on the same rc. The test expects ten processes to finish inside a two second pause, so a slow runner can end the pause early. The test needs a change of its own, which this draft does not make.
• Full build on the head 56d91a1, success, run 43113879702. Configure and the Ubuntu, Ubuntu ARM and Windows jobs on Node 22 and 24 pass. Each build and test job ran eslint . --ext .js and gave 1,223 passed and 2 skipped in the unit tests and 132 passed and 4 skipped in the integration tests, with cloudCache.test.js and shareUsageBackOff.test.js passing. Configure printed "Left out 2 macOS entries", and all 7 Setting 'PullRequestSha' lines are 56d91a1f9fac89ee6b5bad9569eacf5cb899741d.

macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.

What remains

Two MUSTs in the Start-up activity section are still not met, and each waits for a reason this draft cannot remove.

1. Discovery when the Engine is built. Nothing in Node starts discovery when the engine is built. A cloud engine calls the Cloud Request Engine's ready() when it processes (processInternal in cloudEngine.js), and with this draft the Cloud Request Engine calls it before its own first process, so discovery happens at the first process whatever is in the pipeline. Starting the two requests from the Cloud Request Engine's own constructor needs fetchEvidenceKeysAndProperties to settle on failure whether or not a pipeline holds the engine, because it rejects only when the engine is attached to one, so a request started before then would never settle. It also changes every test that counts the requests an engine makes or feeds it failures in order, as each engine would make two more on construction. That is a change of its own.
2. Failing the build on a 4xx. The engine is built by its constructor and PipelineBuilder.build(), and both are synchronous, so neither can wait for the answer to a discovery request. A 4xx, such as an invalid resource key, therefore surfaces as the error of the first process, or of ready(), and not as a failed build. Meeting this needs an asynchronous build step that callers await, which changes the public API. .NET does both through CloudRequestEngineBuilder, which calls WarmUp before it returns the engine (CloudRequestEngineBuilder.cs line 376 at 2b5c9eda).

Both are carried by #76.

A key whose accessible properties list no product leaves flowElementProperties empty, so keysAndPropertiesFetched stays false and the engine fetches again on the next process. That is unchanged here and matches #48. Such a key gives a cloud engine nothing to read, so it is not worth a change of its own tonight.

[jwrosewell]

Closing and reopening at once to start the checks again, because the runs queued during GitHub's Actions outage this evening never started.