Skip to content

Commit 90e92a2

Browse files
fix(bigtable): count mutations wrapped from protos towards the row limits
Mutation.MAX_MUTATIONS and MAX_BYTE_SIZE are enforced in addMutation, against counters only addMutation maintains. fromProtoUnsafe(List), fromProtoUnsafe(Iterable) and fromProto(List) add to the mutation list directly and leave both counters at zero, so mutations wrapped from existing protos count towards neither limit. The mutation count is backstopped by RowMutationEntry.toProto() and BulkMutation.add, which re-check the real list size, so it surfaces late and as a different exception type. The byte size is not backstopped anywhere, so a Mutation seeded from protos can exceed 200 MB with nothing client-side objecting. Count wrapped protos in the three factories, so the counters describe the whole row. Deliberately not a checkState in the factories themselves: that would make wrapping an already-over-limit proto throw where it currently does not.
1 parent 9337a93 commit 90e92a2

2 files changed

Lines changed: 67 additions & 1 deletion

File tree

  • java-bigtable/google-cloud-bigtable/src

java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Mutation.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ public static Mutation createUnsafe() {
8686
public static Mutation fromProtoUnsafe(List<com.google.bigtable.v2.Mutation> protos) {
8787
Mutation mutation = new Mutation(true);
8888
mutation.mutations.addAll(protos);
89+
mutation.countAllTowardsLimits(protos);
8990
return mutation;
9091
}
9192

@@ -99,6 +100,7 @@ public static Mutation fromProtoUnsafe(List<com.google.bigtable.v2.Mutation> pro
99100
public static Mutation fromProtoUnsafe(Iterable<com.google.bigtable.v2.Mutation> protos) {
100101
Mutation mutation = new Mutation(true);
101102
mutation.mutations.addAll(protos);
103+
mutation.countAllTowardsLimits(protos);
102104
return mutation;
103105
}
104106

@@ -115,6 +117,7 @@ public static Mutation fromProtoUnsafe(Iterable<com.google.bigtable.v2.Mutation>
115117
static Mutation fromProto(List<com.google.bigtable.v2.Mutation> protos) {
116118
Mutation mutation = new Mutation(false);
117119
mutation.mutations.addAll(protos);
120+
mutation.countAllTowardsLimits(protos);
118121
return mutation;
119122
}
120123

@@ -333,10 +336,20 @@ private void addMutation(com.google.bigtable.v2.Mutation mutation) {
333336
byteSize + mutation.getSerializedSize() <= MAX_BYTE_SIZE,
334337
"Byte size of mutations is too large");
335338

339+
countTowardsLimits(mutation);
340+
341+
mutations.add(mutation);
342+
}
343+
344+
private void countTowardsLimits(com.google.bigtable.v2.Mutation mutation) {
336345
numMutations++;
337346
byteSize += mutation.getSerializedSize();
347+
}
338348

339-
mutations.add(mutation);
349+
private void countAllTowardsLimits(Iterable<com.google.bigtable.v2.Mutation> protos) {
350+
for (com.google.bigtable.v2.Mutation proto : protos) {
351+
countTowardsLimits(proto);
352+
}
340353
}
341354

342355
private static ByteString wrapByteString(String str) {

java-bigtable/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/MutationTest.java

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.google.bigtable.v2.Mutation.DeleteFromRow;
2424
import com.google.bigtable.v2.Mutation.MergeToCell;
2525
import com.google.cloud.bigtable.data.v2.models.Range.TimestampRange;
26+
import com.google.common.collect.ImmutableList;
2627
import com.google.common.primitives.Longs;
2728
import com.google.protobuf.ByteString;
2829
import java.io.ByteArrayInputStream;
@@ -261,6 +262,58 @@ public void tooLargeRequest() {
261262
assertThat(actualError).isInstanceOf(IllegalStateException.class);
262263
}
263264

265+
@Test
266+
public void tooManyMutationsCountsWrappedProtosTest() {
267+
Mutation mutation =
268+
Mutation.fromProtoUnsafe(
269+
ImmutableList.of(
270+
com.google.bigtable.v2.Mutation.newBuilder()
271+
.setDeleteFromRow(com.google.bigtable.v2.Mutation.DeleteFromRow.newBuilder())
272+
.build()));
273+
274+
for (int i = 0; i < Mutation.MAX_MUTATIONS - 1; i++) {
275+
mutation.setCell("f", "", "");
276+
}
277+
278+
Exception actualError = null;
279+
try {
280+
mutation.setCell("f", "", "");
281+
} catch (Exception e) {
282+
actualError = e;
283+
}
284+
285+
assertThat(actualError).isInstanceOf(IllegalStateException.class);
286+
assertThat(mutation.getMutations()).hasSize(Mutation.MAX_MUTATIONS);
287+
}
288+
289+
@Test
290+
public void tooLargeRequestCountsWrappedProtosTest() {
291+
Mutation mutation =
292+
Mutation.fromProtoUnsafe(
293+
ImmutableList.of(
294+
com.google.bigtable.v2.Mutation.newBuilder()
295+
.setSetCell(
296+
com.google.bigtable.v2.Mutation.SetCell.newBuilder()
297+
.setFamilyName("f")
298+
.setValue(ByteString.copyFrom(new byte[Mutation.MAX_BYTE_SIZE / 2])))
299+
.build(),
300+
com.google.bigtable.v2.Mutation.newBuilder()
301+
.setSetCell(
302+
com.google.bigtable.v2.Mutation.SetCell.newBuilder()
303+
.setFamilyName("f")
304+
.setValue(ByteString.copyFrom(new byte[Mutation.MAX_BYTE_SIZE / 2])))
305+
.build()));
306+
307+
Exception actualError = null;
308+
try {
309+
mutation.setCell("f", "", "");
310+
} catch (Exception e) {
311+
actualError = e;
312+
}
313+
314+
assertThat(actualError).isInstanceOf(IllegalStateException.class);
315+
}
316+
264317
@Test
265318
public void testWithLongValue() {
266319
Mutation mutation =

0 commit comments

Comments
 (0)