Remove aggressive check in RegistryJpaIO.Write (#1889)

This commit is contained in:
Weimin Yu 2022-12-22 17:12:09 -05:00 committed by GitHub
parent 6a1fd3491f
commit 61389a66bb
2 changed files with 4 additions and 15 deletions

View file

@ -14,7 +14,6 @@
package google.registry.beam.common;
import static com.google.common.base.Preconditions.checkArgument;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static org.apache.beam.sdk.values.TypeDescriptors.integers;
@ -345,8 +344,7 @@ public final class RegistryJpaIO {
try {
tm().transact(
() -> {
// Don't modify existing objects as it could lead to race conditions
entities.forEach(this::verifyObjectNonexistence);
// TODO(b/263502442): properly handle creations and blind-writes.
tm().putAll(entities);
});
counter.inc(entities.size());
@ -364,8 +362,7 @@ public final class RegistryJpaIO {
try {
tm().transact(
() -> {
// Don't modify existing objects as it could lead to race conditions
verifyObjectNonexistence(entity);
// TODO(b/263502442): properly handle creations and blind-writes.
tm().put(entity);
});
counter.inc();
@ -391,15 +388,5 @@ public final class RegistryJpaIO {
return "Non-SqlEntity: " + entity;
}
}
/** SqlBatchWriter should not re-write existing entities due to potential race conditions. */
private void verifyObjectNonexistence(Object obj) {
// We cannot rely on calling "insert" on the objects because the underlying JPA persist call
// adds the input object to the persistence context, meaning that any modifications (e.g.
// updateTimestamp) are reflected in the input object. Beam doesn't allow modification of
// input objects, so this throws an exception.
// TODO(go/non-datastore-allocateid): also check that all the objects have IDs
checkArgument(!tm().exists(obj), "Entities created in SqlBatchWriter must not already exist");
}
}
}

View file

@ -33,6 +33,7 @@ import java.io.Serializable;
import org.apache.beam.sdk.Pipeline.PipelineExecutionException;
import org.apache.beam.sdk.transforms.Create;
import org.joda.time.DateTime;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
@ -67,6 +68,7 @@ class RegistryJpaWriteTest implements Serializable {
.containsExactlyElementsIn(contacts);
}
@Disabled("b/263502442")
@Test
void testFailure_writeExistingEntity() {
// RegistryJpaIO.Write actions should not write existing objects to the database because the