googleapis / googleapis/google-cloud-java

[java-bigtable] RowMutationEntry exposes no serialized size, so bulk-mutation callers must rebuild the proto

Open
#14,018 0 comments 0 reactions 1 assignee Claimed by @mutianf View on GitHub
Dominant language
Java
Stars
2.1k
Forks
1.2k
Avg merge
1d 23h
Merged PRs (30d)
154

Description

Version: `google-cloud-bigtable` 2.80.0. `RowMutationEntry.java` and `Mutation.java` are
byte-identical at `main` today, so the line numbers below apply there too.

### Summary

`RowMutationEntry` exposes no accessor for its serialized size, its row key or its mutation count. A
caller that needs any of those — to bound its own in-flight memory, to report a bytes-written
metric, or to put a rejected mutation on a dead-letter queue — has to call the `@InternalApi`
`toProto()` and build a whole `MutateRowsRequest.Entry` to read one number.

### Detail

The whole public instance surface of `RowMutationEntry` is the mutation builders plus:

```java
// RowMutationEntry.java:203-204
@InternalApi
public MutateRowsRequest.Entry toProto() {
```

`Mutation` already accumulates a size on every cell added, though not the framed size a caller
would need, and it is private with no accessor:

```java
// Mutation.java:329-340
private void addMutation(com.google.bigtable.v2.Mutation mutation) {
Preconditions.checkState(numMutations + 1 <= MAX_MUTATIONS, "Too many mutations per row");
Preconditions.checkState(
byteSize + mutation.getSerializedSize() <= MAX_BYTE_SIZE,
"Byte size of mutations is too large");

numMutations++;
byteSize += mutation.getSerializedSize();

mutations.add(mutation);
}
```

The client's own batching path needs exactly this number and gets it the expensive way —
`MutateRowsBatchingDescriptor.countBytes()` calls `entry.toProto().getSerializedSize()`.

### Why a caller needs it

Bulk mutations are unbounded in size: a single row mutation may legitimately be megabytes
(`Mutation.MAX_BYTE_SIZE` is 200 MB), so an application that bounds its own in-flight work by
counting entries bounds nothing useful. Bounding by bytes requires each entry's size, and
`toProto()` is the only route to it — an `@InternalApi` method, which callers are asked not to
depend on, for a value the object could know.

The same gap makes the row key unreachable. Reporting which mutation was rejected, or writing it to
a dead-letter destination, means building the proto purely to call `getRowKey()`.

### Cost of the workaround

Measured on OpenJDK 21.0.5 (aarch64, `-Xmx4g`, default collector) against `google-cloud-bigtable`
2.80.0; five JVM forks, 4 s warmup and 7 timed iterations of >=2 s each, allocation sampled over
the same loop:

| Entry shape | serialized size | `toProto().getSerializedSize()` | allocated |
|---|---|---|---|
| 1 cell, 64 B value | 88 B | 33.0 ns | 176 B |
| 8 cells, 128 B values | 1191 B | 154.1 ns | 336 B |
| 64 cells, 128 B values | 9533 B | 912.0 ns | 1232 B |

Small next to an RPC, and that is not really the argument — the argument is that it is an
`@InternalApi` dependency taken to read a value the object holds, on every record of a streaming
pipeline.

### Proposed API

```java
public ByteString getRowKey();
public int getMutationCount();
public long getSerializedSize(); // == toProto().getSerializedSize(), without building it
```

I have this implemented and tested, and will open a PR referencing this issue — happy to reshape it
if you would rather have different names, a subset, or an explicitly approximate size. Two things
that came out of implementing it, in case they affect what you want:

- **`Mutation.byteSize` is not the number**, so this cannot just be exposed. It sums each child's
own serialized size and omits the row-key field and the per-mutation tag and length prefixes. My
PR accumulates the framed size alongside it instead.
- **Three factories bypass `addMutation`** — `fromProtoUnsafe(List)`, `fromProtoUnsafe(Iterable)`
and `fromProto(List)` all do `mutation.mutations.addAll(protos)` directly, so `numMutations` and
`byteSize` stay at zero for entries built through `createFromMutationUnsafe`. That is pre-existing
and I have not changed it (the `MAX_MUTATIONS` and `MAX_BYTE_SIZE` preconditions therefore do not
account for those mutations today), but any accessor added here has to be maintained on those
paths or it would be silently wrong for them.

### Related

The client's own duplicate proto construction in `MutateRowsBatchingDescriptor.createResource()` is
filed separately, and is fixable without any API change.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.