Skip to content

Implement IntegrateBundles for GCP MirrorWriter - #1216

Merged
roger2hk merged 6 commits into
transparency-dev:mainfrom
roger2hk:gcp-mirror-writer-integrate-bundles
Oct 15, 2026
Merged

roger2hk merged 6 commits into
transparency-dev:mainfrom
roger2hk:gcp-mirror-writer-integrate-bundles

Conversation

@roger2hk

@roger2hk roger2hk commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Towards #945.

@roger2hk
roger2hk requested a review from AlCutter October 8, 2026 18:49
@roger2hk
roger2hk requested a review from a team as a code owner October 8, 2026 18:49
Comment thread storage/gcp/gcp.go Outdated
Comment thread storage/gcp/gcp.go
newSize := currentSize + uint64(len(newLeafHashes))
var rootHash []byte

_, err := w.seq.dbPool.ReadWriteTransactionWithOptions(ctx, func(ctx context.Context, txn *spanner.ReadWriteTransaction) error {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This transaction could potentially be very large, if the user passed in an iterator that covered a huge range it could be that it can never commit because it's larger than the Spanner max.

We could start by just having some per-implementation (or Tessera-wide on the storage API contract?) limit for the number of entries this call will accept, and then figure out what we should do when that's exceeded (integrate to the limit, but return an error? - I think there's space in the tlog-mirror for that behaviour, right?)
Alternatively we could try to "chunk it up" in here, maybe that's too much complexity.

Any thoughts?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To cover the MTC Mirror use case, I think having MirrorTarget.AddEntries to split each request into IntegrateBundles calls of at most N bundles could be the way to go. Every driver's IntegrateBundles then stays in a single bounded transaction with no chucking logic.

For MTC use case, MirrorTarget.AddEntries also knows how far it got to return 202 if the streaming request is interrupted. I guess we can take MTC use case as the default value. Other personality can override that in an option.

How does that sound?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, cool - that sounds like a good approach.

Comment thread storage/gcp/gcp.go Outdated
Comment thread storage/gcp/gcp.go Outdated
}
}

if err := w.logStore.setEntryBundle(ctx, currBundleIdx, partial, bundleRaw); err != nil {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You probably don't want to do this serially - GCS can be quite slow for writes, would be better to spin these off in parallel (could just go for now, or, better, do a worker-channel pattern thing with some amount of concurrency)

@roger2hk roger2hk Oct 14, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Added an errgroup with a concurrency limit (i.e. 32).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let me know if 32 is too large. I think it's more than enough.

Comment thread storage/gcp/gcp.go Outdated
@roger2hk
roger2hk force-pushed the gcp-mirror-writer-integrate-bundles branch from 610306e to 82878c6 Compare October 14, 2026 17:38
@roger2hk
roger2hk merged commit e9f911d into transparency-dev:main Oct 15, 2026
21 of 23 checks passed
@roger2hk
roger2hk deleted the gcp-mirror-writer-integrate-bundles branch October 15, 2026 10:42
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.

2 participants