Skip to content

[PureGo] Remove UC descriptor cache - #699

Merged
zlata-stefanovic-db merged 2 commits into
mainfrom
purego-remove-descriptor-cache
Aug 7, 2026
Merged

[PureGo] Remove UC descriptor cache#699
zlata-stefanovic-db merged 2 commits into
mainfrom
purego-remove-descriptor-cache

Conversation

@zlata-stefanovic-db

Copy link
Copy Markdown
Contributor

What changes are proposed in this pull request?

Removes the Unity Catalog descriptor cache from the PureGo SDK and documents the fetch-once-and-reuse contract in its place.

WHAT:

  • Delete the cache from purego/zerobus/sdk.go: the dynamicSchemaCache field, cachedDescriptor struct, the 5-minute TTL and 128-entry constants, getDynamicDescriptorFromCache, storeDynamicDescriptor, the SHA-256 credential-scoped dynamicSchemaCacheKey, and the cache reset in Close. fetchProtoDescriptor now goes straight to UC, and SDK.mu guards only the open-stream set and the closed flag.
  • Document the contract where a reader will actually hit it: the FetchProtoDescriptorFromUC godoc (with a reuse-across-streams snippet and an explicit warning that per-stream calls put a UC round-trip on the ingestion path), a new "Proto descriptors from Unity Catalog" section in the package godoc, and a callout in the README's dynamic-proto section.
  • Move the descriptor fetch in examples/dynamic/single out of its openStream helper and into main, since bundling fetch into stream creation is exactly the pattern that now costs a round-trip per stream. The batch and proto examples already fetched once and only gain a comment.
  • Correct the NEXT_CHANGELOG.md entry to the current API name and state the fetch-once semantics.

WHY:

Per @andrijast-db's review feedback: fetching the descriptor is now an explicit standalone call rather than a step inside stream creation, so a caller can fetch once and reuse the bytes for every stream on a table. That makes the cache's cost/benefit negative — it bought a TTL, an entry limit, a credential-scoped key, and a proposed RefreshProtoDescriptorFromUC escape hatch, in exchange for saving a round-trip that correct callers do not make in the first place.

Deleting it also resolves #676 by construction rather than by implementing the lifecycle machinery that issue proposed. There is no stale descriptor to invalidate because every call is fresh, no cache key to coalesce misses on, and no shared in-flight work whose cancellation needs isolating. OAuth token minting is still shared across streams via auth.SharedTokenCache, which is unaffected by this change.

This supersedes #678, which is now closed.

Note for reviewers: FetchProtoDescriptor remains as a Deprecated: alias for FetchProtoDescriptorFromUC. Both names landed in #666 and v0.1.0 has not shipped, so it is deprecating a name no user has seen. I left it alone to keep this PR scoped to the cache removal, but I am happy to delete it here if you would rather not carry it into the first release.

How is this tested?

  • cd purego && go test -race ./... — all packages pass.
  • cd purego && go vet ./... and gofmt -l . — clean.
  • cd purego/examples && go build ./... && go vet ./... — clean.

Test changes:

  • TestSDKFetchProtoDescriptor_CacheHit became TestSDKFetchProtoDescriptor_FetchesOnEveryCall. It now asserts two UC schema requests for two fetches, pinning the no-caching behavior instead of the old "1 request" expectation, and still checks that repeated fetches return equivalent descriptors.
  • Added TestSDKFetchProtoDescriptorFromUC_RejectsClosedSDK. The closed-SDK guard was previously only exercised through the deleted storeDynamicDescriptor test, so this keeps it covered against the public API.
  • Deleted the four tests whose subject no longer exists: cache-hit counting, credential-scoped cache keys, expired-entry pruning, and the entry limit.

Closes #676

Fetching a UC descriptor is now an explicit standalone call rather than a
step inside stream creation, so callers can fetch once and reuse the bytes
for every stream on a table. The TTL, entry limit, and credential-scoped
cache key bought nothing in exchange for that complexity.

Document the fetch-once-and-reuse contract in the README, package godoc, and
FetchProtoDescriptorFromUC godoc so the cost of repeated calls is explicit.

Signed-off-by: Zlata Stefanovic <zlata.stefanovic@databricks.com>
The end-to-end public-flow test exercised the deprecated
FetchProtoDescriptor, so the recommended entry point had no coverage and
the test would break whenever the alias is removed. Point it at
FetchProtoDescriptorFromUC and cover the alias with a delegation test.

Signed-off-by: Zlata Stefanovic <zlata.stefanovic@databricks.com>

@danilonajkov-db danilonajkov-db left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I like this approach its better than us internally managing it

@zlata-stefanovic-db
zlata-stefanovic-db added this pull request to the merge queue Aug 7, 2026
Merged via the queue into main with commit b318b72 Aug 7, 2026
18 checks passed
@zlata-stefanovic-db
zlata-stefanovic-db deleted the purego-remove-descriptor-cache branch August 7, 2026 15:07
@zlata-stefanovic-db zlata-stefanovic-db mentioned this pull request Aug 7, 2026
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[PureGo] Improve UC descriptor cache lifecycle

2 participants