google-cloud-go icon indicating copy to clipboard operation
google-cloud-go copied to clipboard

bigtable: Expose mutation protobufs via mut.Ops()

Open coxley opened this issue 1 year ago • 4 comments

Summary

Context: #9034

Recording or analyzing database operations can be valuable. Think recording historical requests and replaying them locally — or in a shadow tier — and comparing what actions were taken.

This isn't possible when using the current BigTable SDK.

We add two functions in this PR to address that: mut.Ops() and mut.OpsValues().

Why two?

To avoid unexpected behavior from the client mutating results from mut.Ops() before table.Apply(), we need to clone the underlying protobufs. The amount of extra allocations will grow with the number of staged operations.

Except for mut.Set() operations.

At least in my use cases, we write fairly large blobs to cells. For basic replay data, I don't need a copy of the bytes I've already written — I already have access to them. I'd rather save the extra alloc space.

Assuming this is a sensible default, mut.Ops() will zero out btpb.Mutation_SetCell.Value. The unmodified version can be retrieved from mut.OpsValues()

Sensible default?

I was going to flip the behavior around but mut.OpsNoValues or mut.OpsWithoutValues feels clunky.

I'm not tied to it — happy to make the change if someone has better names. :)

(speaking of names, mut.OpsData() was my instinct but decided to match the SetCell field name)

Test Plan

➜ go test ./...
?       cloud.google.com/go/bigtable/cmd/emulator       [no test files]
?       cloud.google.com/go/bigtable/internal   [no test files]
?       cloud.google.com/go/bigtable/internal/conformance       [no test files]
?       cloud.google.com/go/bigtable/internal/mockserver        [no test files]
?       cloud.google.com/go/bigtable/internal/option    [no test files]
?       cloud.google.com/go/bigtable/internal/stat      [no test files]
ok      cloud.google.com/go/bigtable    9.709s
ok      cloud.google.com/go/bigtable/bttest     0.133s
ok      cloud.google.com/go/bigtable/internal/testproxy 0.028s

Benchmark, assuming writing a single cell value of 4K:

BenchmarkMutationOps/ops-8                877124              1356 ns/op             904 B/op         19 allocs/op
BenchmarkMutationOps/opsvalues-8          484908              2467 ns/op            5008 B/op         21 allocs/op

coxley avatar Dec 09 '23 16:12 coxley

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Dec 09 '23 16:12 google-cla[bot]

I was asked to request review from @bhshkh in #9034 — I don't have permission to edit reviewers on the PR so mentioning here. :)

coxley avatar Dec 09 '23 16:12 coxley

This is on my radar. Will review it soon.

bhshkh avatar Dec 12 '23 01:12 bhshkh

LGTM but would be great to have a review from @mutianf or @igorbernstein2 as well

bhshkh avatar Dec 13 '23 04:12 bhshkh