Skip to content

Store custom schema metas as proto binary in pogreb cache#1994

Merged
openshift-merge-bot[bot] merged 1 commit into
operator-framework:masterfrom
joelanford:pogreb-proto-meta-storage
May 21, 2026
Merged

Store custom schema metas as proto binary in pogreb cache#1994
openshift-merge-bot[bot] merged 1 commit into
operator-framework:masterfrom
joelanford:pogreb-proto-meta-storage

Conversation

@joelanford

Copy link
Copy Markdown
Member

Description of the change:

Changes the SendMetas backend method to send *structpb.Struct instead of raw []byte,
pushing deserialization responsibility into each backend implementation.

The pogreb backend now converts incoming JSON to protobuf binary wire format at write time
(PutMeta) and uses proto.Unmarshal at read time (SendMetas), which is significantly
cheaper than JSON parsing. The JSON backend continues to store JSON on disk and unmarshals
at read time.

This eliminates the JSON→structpb.Struct conversion that previously happened in the shared
ListPackageCustomSchemas method, since each backend now owns the full round-trip.

Motivation for the change:

Proto binary deserialization is 2-5x faster than JSON for structpb.Struct. Since the pogreb
cache is built infrequently but read on every query, shifting the JSON parse cost to build
time reduces per-request latency.

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Docs updated or added to /docs
  • Commit messages sensible and descriptive

Copilot AI review requested due to automatic review settings May 21, 2026 17:32
@openshift-ci openshift-ci Bot requested review from ankitathomas and oceanc80 May 21, 2026 17:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors custom schema meta handling so the cache backends stream *structpb.Struct directly (instead of raw []byte), and updates the pogreb cache backend to store metas in protobuf binary format to reduce per-query JSON parsing overhead.

Changes:

  • Updates the cache backend interface to SendMetas(..., func(*structpb.Struct) error) and simplifies ListPackageCustomSchemas to forward the sender directly.
  • Changes the pogreb backend to convert incoming meta JSON to protobuf wire format at write time and proto.Unmarshal it at read time.
  • Updates the JSON backend to continue storing JSON on disk but unmarshal into structpb.Struct at read time.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
pkg/cache/pogrebv1.go Switches pogreb meta storage from JSON bytes to protobuf binary and changes SendMetas to emit *structpb.Struct.
pkg/cache/json.go Changes SendMetas to unmarshal stored JSON into *structpb.Struct before sending.
pkg/cache/cache.go Updates backend interface and removes shared JSON→Struct conversion in ListPackageCustomSchemas.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/cache/pogrebv1.go
Comment thread pkg/cache/pogrebv1.go Outdated
@codecov

codecov Bot commented May 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 42.85714% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.75%. Comparing base (6b3a8f7) to head (a82fb7d).

Files with missing lines Patch % Lines
pkg/cache/pogrebv1.go 40.00% 4 Missing and 5 partials ⚠️
pkg/cache/json.go 40.00% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1994      +/-   ##
==========================================
- Coverage   58.77%   58.75%   -0.03%     
==========================================
  Files         141      141              
  Lines       13418    13426       +8     
==========================================
+ Hits         7887     7889       +2     
- Misses       4322     4325       +3     
- Partials     1209     1212       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tmshort

tmshort commented May 21, 2026

Copy link
Copy Markdown
Contributor

/approved
There are some copilot comments, however.

PutMeta receives JSON and each backend decides its internal format.
Pogreb now converts JSON to proto binary at build time and uses
proto.Unmarshal at read time, which is cheaper than JSON parsing.
The SendMetas sender signature changes to func(*structpb.Struct)
so backends own the deserialization. JSON backend still stores and
parses JSON.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@joelanford joelanford force-pushed the pogreb-proto-meta-storage branch from a45c0b6 to a82fb7d Compare May 21, 2026 17:56
@tmshort

tmshort commented May 21, 2026

Copy link
Copy Markdown
Contributor

/approve

@openshift-ci

openshift-ci Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tmshort

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 21, 2026
@fgiudici

Copy link
Copy Markdown
Member

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 21, 2026
@openshift-merge-bot openshift-merge-bot Bot merged commit 23c2915 into operator-framework:master May 21, 2026
11 of 13 checks passed
@joelanford joelanford deleted the pogreb-proto-meta-storage branch May 21, 2026 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants