Skip to content

Commit 24c314f

Browse files
committed
Fix based on feedback
1 parent 6a0b9c4 commit 24c314f

File tree

4 files changed

+38
-35
lines changed

4 files changed

+38
-35
lines changed

core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandler.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,9 @@ boolean canOnePhaseCommit(Snapshot snapshot) throws CommitException {
222222
}
223223

224224
try {
225-
// If the mutations can be grouped together, the mutations can be done in a single mutate API
226-
// call, so we can one-phase commit the transaction
227-
return mutationsGrouper.canBeGroupedTogether(
225+
// If the mutations can be grouped altogether, the mutations can be done in a single mutate
226+
// API call, so we can one-phase commit the transaction
227+
return mutationsGrouper.canBeGroupedAltogether(
228228
Stream.concat(snapshot.getPutsInWriteSet().stream(), deletesInDeleteSet.stream())
229229
.collect(Collectors.toList()));
230230
} catch (ExecutionException e) {

core/src/main/java/com/scalar/db/transaction/consensuscommit/MutationsGrouper.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public List<List<Mutation>> groupMutations(Collection<Mutation> mutations)
5151
return groupToBatches.values().stream().flatMap(List::stream).collect(Collectors.toList());
5252
}
5353

54-
public boolean canBeGroupedTogether(Collection<Mutation> mutations) throws ExecutionException {
54+
public boolean canBeGroupedAltogether(Collection<Mutation> mutations) throws ExecutionException {
5555
if (mutations.size() <= 1) {
5656
return true;
5757
}

core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitHandlerTest.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -974,7 +974,7 @@ public void canOnePhaseCommit_WhenOnePhaseCommitDisabled_ShouldReturnFalse() thr
974974

975975
// Assert
976976
assertThat(actual).isFalse();
977-
verify(mutationsGrouper, never()).canBeGroupedTogether(anyList());
977+
verify(mutationsGrouper, never()).canBeGroupedAltogether(anyList());
978978
}
979979

980980
@Test
@@ -988,7 +988,7 @@ public void canOnePhaseCommit_WhenValidationRequired_ShouldReturnFalse() throws
988988

989989
// Assert
990990
assertThat(actual).isFalse();
991-
verify(mutationsGrouper, never()).canBeGroupedTogether(anyList());
991+
verify(mutationsGrouper, never()).canBeGroupedAltogether(anyList());
992992
}
993993

994994
@Test
@@ -1002,7 +1002,7 @@ public void canOnePhaseCommit_WhenNoWritesAndDeletes_ShouldReturnFalse() throws
10021002

10031003
// Assert
10041004
assertThat(actual).isFalse();
1005-
verify(mutationsGrouper, never()).canBeGroupedTogether(anyList());
1005+
verify(mutationsGrouper, never()).canBeGroupedAltogether(anyList());
10061006
}
10071007

10081008
@Test
@@ -1022,7 +1022,7 @@ public void canOnePhaseCommit_WhenDeleteWithoutExistingRecord_ShouldReturnFalse(
10221022

10231023
// Assert
10241024
assertThat(actual).isFalse();
1025-
verify(mutationsGrouper, never()).canBeGroupedTogether(anyList());
1025+
verify(mutationsGrouper, never()).canBeGroupedAltogether(anyList());
10261026
}
10271027

10281028
@Test
@@ -1036,14 +1036,14 @@ public void canOnePhaseCommit_WhenMutationsCanBeGrouped_ShouldReturnTrue() throw
10361036
TransactionResult result = mock(TransactionResult.class);
10371037
snapshot.putIntoReadSet(new Snapshot.Key(delete), Optional.of(result));
10381038

1039-
doReturn(true).when(mutationsGrouper).canBeGroupedTogether(anyList());
1039+
doReturn(true).when(mutationsGrouper).canBeGroupedAltogether(anyList());
10401040

10411041
// Act
10421042
boolean actual = handler.canOnePhaseCommit(snapshot);
10431043

10441044
// Assert
10451045
assertThat(actual).isTrue();
1046-
verify(mutationsGrouper).canBeGroupedTogether(anyList());
1046+
verify(mutationsGrouper).canBeGroupedAltogether(anyList());
10471047
}
10481048

10491049
@Test
@@ -1057,14 +1057,14 @@ public void canOnePhaseCommit_WhenMutationsCannotBeGrouped_ShouldReturnFalse() t
10571057
TransactionResult result = mock(TransactionResult.class);
10581058
snapshot.putIntoReadSet(new Snapshot.Key(delete), Optional.of(result));
10591059

1060-
doReturn(false).when(mutationsGrouper).canBeGroupedTogether(anyList());
1060+
doReturn(false).when(mutationsGrouper).canBeGroupedAltogether(anyList());
10611061

10621062
// Act
10631063
boolean actual = handler.canOnePhaseCommit(snapshot);
10641064

10651065
// Assert
10661066
assertThat(actual).isFalse();
1067-
verify(mutationsGrouper).canBeGroupedTogether(anyList());
1067+
verify(mutationsGrouper).canBeGroupedAltogether(anyList());
10681068
}
10691069

10701070
@Test
@@ -1079,7 +1079,7 @@ public void canOnePhaseCommit_WhenMutationsGrouperThrowsException_ShouldThrowCom
10791079
TransactionResult result = mock(TransactionResult.class);
10801080
snapshot.putIntoReadSet(new Snapshot.Key(delete), Optional.of(result));
10811081

1082-
doThrow(ExecutionException.class).when(mutationsGrouper).canBeGroupedTogether(anyList());
1082+
doThrow(ExecutionException.class).when(mutationsGrouper).canBeGroupedAltogether(anyList());
10831083

10841084
// Act Assert
10851085
assertThatThrownBy(() -> handler.canOnePhaseCommit(snapshot))

core/src/test/java/com/scalar/db/transaction/consensuscommit/MutationsGrouperTest.java

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -337,18 +337,19 @@ public void groupMutations_WithStorageAtomicityAndBatchSizeLimit_ShouldGroupCorr
337337
}
338338

339339
@Test
340-
public void canBeGroupedTogether_WithEmptyCollection_ShouldReturnTrue()
340+
public void canBeGroupedAltogether_WithEmptyCollection_ShouldReturnTrue()
341341
throws ExecutionException {
342342
// Act
343-
boolean result = grouper.canBeGroupedTogether(Collections.emptyList());
343+
boolean result = grouper.canBeGroupedAltogether(Collections.emptyList());
344344

345345
// Assert
346346
assertThat(result).isTrue();
347347
}
348348

349349
@Test
350-
public void canBeGroupedTogether_WithAllMutationsInSameGroupForRecordAtomicity_ShouldReturnTrue()
351-
throws ExecutionException {
350+
public void
351+
canBeGroupedAltogether_WithAllMutationsInSameGroupForRecordAtomicity_ShouldReturnTrue()
352+
throws ExecutionException {
352353
// Arrange
353354
String namespace = "ns";
354355
String table = "table";
@@ -368,15 +369,15 @@ public void canBeGroupedTogether_WithAllMutationsInSameGroupForRecordAtomicity_S
368369
createMutation(namespace, table, partitionKey1, Optional.of(clusteringKey1));
369370

370371
// Act
371-
boolean result = grouper.canBeGroupedTogether(Arrays.asList(mutation1, mutation2));
372+
boolean result = grouper.canBeGroupedAltogether(Arrays.asList(mutation1, mutation2));
372373

373374
// Assert
374375
assertThat(result).isTrue();
375376
}
376377

377378
@Test
378379
public void
379-
canBeGroupedTogether_WithAllMutationsInSameGroupForPartitionAtomicity_ShouldReturnTrue()
380+
canBeGroupedAltogether_WithAllMutationsInSameGroupForPartitionAtomicity_ShouldReturnTrue()
380381
throws ExecutionException {
381382
// Arrange
382383
String namespace = "ns";
@@ -398,14 +399,14 @@ public void canBeGroupedTogether_WithAllMutationsInSameGroupForRecordAtomicity_S
398399
createMutation(namespace, table, partitionKey1, Optional.of(clusteringKey2));
399400

400401
// Act
401-
boolean result = grouper.canBeGroupedTogether(Arrays.asList(mutation1, mutation2));
402+
boolean result = grouper.canBeGroupedAltogether(Arrays.asList(mutation1, mutation2));
402403

403404
// Assert
404405
assertThat(result).isTrue();
405406
}
406407

407408
@Test
408-
public void canBeGroupedTogether_WithAllMutationsInSameGroupForTableAtomicity_ShouldReturnTrue()
409+
public void canBeGroupedAltogether_WithAllMutationsInSameGroupForTableAtomicity_ShouldReturnTrue()
409410
throws ExecutionException {
410411
// Arrange
411412
String namespace = "ns";
@@ -424,15 +425,15 @@ public void canBeGroupedTogether_WithAllMutationsInSameGroupForTableAtomicity_Sh
424425
Mutation mutation2 = createMutation(namespace, table, partitionKey2, Optional.empty());
425426

426427
// Act
427-
boolean result = grouper.canBeGroupedTogether(Arrays.asList(mutation1, mutation2));
428+
boolean result = grouper.canBeGroupedAltogether(Arrays.asList(mutation1, mutation2));
428429

429430
// Assert
430431
assertThat(result).isTrue();
431432
}
432433

433434
@Test
434435
public void
435-
canBeGroupedTogether_WithAllMutationsInSameGroupForNamespaceAtomicity_ShouldReturnTrue()
436+
canBeGroupedAltogether_WithAllMutationsInSameGroupForNamespaceAtomicity_ShouldReturnTrue()
436437
throws ExecutionException {
437438
// Arrange
438439
String namespace = "ns";
@@ -452,15 +453,16 @@ public void canBeGroupedTogether_WithAllMutationsInSameGroupForTableAtomicity_Sh
452453
Mutation mutation2 = createMutation(namespace, table2, partitionKey2, Optional.empty());
453454

454455
// Act
455-
boolean result = grouper.canBeGroupedTogether(Arrays.asList(mutation1, mutation2));
456+
boolean result = grouper.canBeGroupedAltogether(Arrays.asList(mutation1, mutation2));
456457

457458
// Assert
458459
assertThat(result).isTrue();
459460
}
460461

461462
@Test
462-
public void canBeGroupedTogether_WithAllMutationsInSameGroupForStorageAtomicity_ShouldReturnTrue()
463-
throws ExecutionException {
463+
public void
464+
canBeGroupedAltogether_WithAllMutationsInSameGroupForStorageAtomicity_ShouldReturnTrue()
465+
throws ExecutionException {
464466
// Arrange
465467
String namespace1 = "ns1";
466468
String namespace2 = "ns2";
@@ -482,14 +484,14 @@ public void canBeGroupedTogether_WithAllMutationsInSameGroupForStorageAtomicity_
482484
Mutation mutation2 = createMutation(namespace2, table2, partitionKey2, Optional.empty());
483485

484486
// Act
485-
boolean result = grouper.canBeGroupedTogether(Arrays.asList(mutation1, mutation2));
487+
boolean result = grouper.canBeGroupedAltogether(Arrays.asList(mutation1, mutation2));
486488

487489
// Assert
488490
assertThat(result).isTrue();
489491
}
490492

491493
@Test
492-
public void canBeGroupedTogether_WithMutationsInDifferentGroups_ShouldReturnFalse()
494+
public void canBeGroupedAltogether_WithMutationsInDifferentGroups_ShouldReturnFalse()
493495
throws ExecutionException {
494496
// Arrange
495497
String namespace1 = "ns1";
@@ -518,15 +520,15 @@ public void canBeGroupedTogether_WithMutationsInDifferentGroups_ShouldReturnFals
518520
Mutation mutation2 = createMutation(namespace2, table2, partitionKey2, Optional.empty());
519521

520522
// Act
521-
boolean result = grouper.canBeGroupedTogether(Arrays.asList(mutation1, mutation2));
523+
boolean result = grouper.canBeGroupedAltogether(Arrays.asList(mutation1, mutation2));
522524

523525
// Assert
524526
assertThat(result).isFalse();
525527
}
526528

527529
@Test
528530
public void
529-
canBeGroupedTogether_WithMutationsInDifferentTables_ShouldReturnFalseForTableAtomicity()
531+
canBeGroupedAltogether_WithMutationsInDifferentTables_ShouldReturnFalseForTableAtomicity()
530532
throws ExecutionException {
531533
// Arrange
532534
String namespace = "ns";
@@ -547,15 +549,15 @@ public void canBeGroupedTogether_WithMutationsInDifferentGroups_ShouldReturnFals
547549
Mutation mutation2 = createMutation(namespace, table2, partitionKey2, Optional.empty());
548550

549551
// Act
550-
boolean result = grouper.canBeGroupedTogether(Arrays.asList(mutation1, mutation2));
552+
boolean result = grouper.canBeGroupedAltogether(Arrays.asList(mutation1, mutation2));
551553

552554
// Assert
553555
assertThat(result).isFalse();
554556
}
555557

556558
@Test
557559
public void
558-
canBeGroupedTogether_WithMutationsInDifferentPartitions_ShouldReturnFalseForPartitionAtomicity()
560+
canBeGroupedAltogether_WithMutationsInDifferentPartitions_ShouldReturnFalseForPartitionAtomicity()
559561
throws ExecutionException {
560562
// Arrange
561563
String namespace = "ns";
@@ -575,14 +577,15 @@ public void canBeGroupedTogether_WithMutationsInDifferentGroups_ShouldReturnFals
575577
Mutation mutation2 = createMutation(namespace, table, partitionKey2, Optional.empty());
576578

577579
// Act
578-
boolean result = grouper.canBeGroupedTogether(Arrays.asList(mutation1, mutation2));
580+
boolean result = grouper.canBeGroupedAltogether(Arrays.asList(mutation1, mutation2));
579581

580582
// Assert
581583
assertThat(result).isFalse();
582584
}
583585

584586
@Test
585-
public void canBeGroupedTogether_WithOverMaxCount_ShouldReturnFalse() throws ExecutionException {
587+
public void canBeGroupedAltogether_WithOverMaxCount_ShouldReturnFalse()
588+
throws ExecutionException {
586589
// Arrange
587590
String namespace = "ns";
588591
String table = "table";
@@ -601,7 +604,7 @@ public void canBeGroupedTogether_WithOverMaxCount_ShouldReturnFalse() throws Exe
601604
}
602605

603606
// Act
604-
boolean result = grouper.canBeGroupedTogether(mutations);
607+
boolean result = grouper.canBeGroupedAltogether(mutations);
605608

606609
// Assert
607610
assertThat(result).isFalse();

0 commit comments

Comments
 (0)