Repository navigation
Conversation
| newSize := currentSize + uint64(len(newLeafHashes)) | ||
| var rootHash []byte | ||
|
|
||
| _, err := w.seq.dbPool.ReadWriteTransactionWithOptions(ctx, func(ctx context.Context, txn *spanner.ReadWriteTransaction) error { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Yeah, cool - that sounds like a good approach.
| } | ||
| } | ||
|
|
||
| if err := w.logStore.setEntryBundle(ctx, currBundleIdx, partial, bundleRaw); err != nil { |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
Done. Added an errgroup with a concurrency limit (i.e. 32).
There was a problem hiding this comment.
Let me know if 32 is too large. I think it's more than enough.
610306e to
82878c6
Compare
Towards #945.