Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

datastore.it.ITDatastoreTest: testTransactionWithRead and testTransactionWithQuery fail if the test DB type is Firestore in Datastore mode #497

Closed
aristide-n opened this issue Aug 12, 2021 · 1 comment · Fixed by #1079
Assignees
Labels
api: datastore Issues related to the googleapis/java-datastore API. priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@aristide-n
Copy link
Contributor

aristide-n commented Aug 12, 2021

These tests always timeout when testing with a Firestore in Datastore Mode DB. I think the reason they pass in continuous tests is because the type of DB they use (in project gcloud-devel) is "Cloud Datastore".
I believe the root cause of the timeouts is the transaction isolation and consistency behavior differences between the 2 Datastore DB types, documented here and here.
TL;DR
With Firestore in datastore mode when 2 concurrent transactions read or write the same data, the lock held by one transaction can delay the other transaction (hence the timeouts), but with Cloud Datastore the contention is reported as an error at commit.

Environment details

All environments

Steps to reproduce

  1. Create a Firestore in Datastore Mode DB (or use an existing DB, if its type is known to be "Firestore in Datastore Mode")
  2. Set the GOOGLE_CLOUD_PROJECT env var to the project ID that owns the DB
  3. Set the GOOGLE_APPLICATION_CREDENTIALS env var to the absolute path of the key file for a service account that has permission to write to the DB
  4. run the tests, e.g.
GOOGLE_CLOUD_PROJECT=some-project GOOGLE_APPLICATION_CREDENTIALS=/tmp/sa-key.json mvn -P enable-integration-tests verify

Stack trace

it.ITDatastoreTest.testTransactionWithQuery

org.junit.runners.model.TestTimedOutException: test timed out after 100 seconds
	at java.net.SocketInputStream.socketRead0(Native Method)
	at java.net.SocketInputStream.socketRead(SocketInputStream.java:116)
	at java.net.SocketInputStream.read(SocketInputStream.java:171)
	at java.net.SocketInputStream.read(SocketInputStream.java:141)
	at sun.security.ssl.SSLSocketInputRecord.read(SSLSocketInputRecord.java:457)
	at sun.security.ssl.SSLSocketInputRecord.bytesInCompletePacket(SSLSocketInputRecord.java:68)
	at sun.security.ssl.SSLSocketImpl.readApplicationRecord(SSLSocketImpl.java:1332)
	at sun.security.ssl.SSLSocketImpl.access$300(SSLSocketImpl.java:73)
	at sun.security.ssl.SSLSocketImpl$AppInputStream.read(SSLSocketImpl.java:948)
	at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
	at java.io.BufferedInputStream.read1(BufferedInputStream.java:286)
	at java.io.BufferedInputStream.read(BufferedInputStream.java:345)
	at sun.net.www.http.HttpClient.parseHTTPHeader(HttpClient.java:735)
	at sun.net.www.http.HttpClient.parseHTTP(HttpClient.java:678)
	at sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1593)
	at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1498)
	at java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:480)
	at sun.net.www.protocol.https.HttpsURLConnectionImpl.getResponseCode(HttpsURLConnectionImpl.java:352)
	at com.google.api.client.http.javanet.NetHttpResponse.<init>(NetHttpResponse.java:36)
	at com.google.api.client.http.javanet.NetHttpRequest.execute(NetHttpRequest.java:149)
	at com.google.api.client.http.javanet.NetHttpRequest.execute(NetHttpRequest.java:84)
	at com.google.api.client.http.HttpRequest.execute(HttpRequest.java:1012)
	at com.google.datastore.v1.client.RemoteRpc.call(RemoteRpc.java:97)
	at com.google.datastore.v1.client.Datastore.commit(Datastore.java:85)
	at com.google.cloud.datastore.spi.v1.HttpDatastoreRpc.commit(HttpDatastoreRpc.java:162)
	at com.google.cloud.datastore.DatastoreImpl$5.call(DatastoreImpl.java:564)
	at com.google.cloud.datastore.DatastoreImpl$5.call(DatastoreImpl.java:561)
	at com.google.api.gax.retrying.DirectRetryingExecutor.submit(DirectRetryingExecutor.java:105)
	at com.google.cloud.RetryHelper.run(RetryHelper.java:76)
	at com.google.cloud.RetryHelper.runWithRetries(RetryHelper.java:50)
	at com.google.cloud.datastore.DatastoreImpl.commit(DatastoreImpl.java:560)
	at com.google.cloud.datastore.DatastoreImpl.commitMutation(DatastoreImpl.java:553)
	at com.google.cloud.datastore.DatastoreImpl.put(DatastoreImpl.java:513)
	at com.google.cloud.datastore.DatastoreHelper.put(DatastoreHelper.java:53)
	at com.google.cloud.datastore.DatastoreImpl.put(DatastoreImpl.java:488)
	at com.google.cloud.datastore.it.ITDatastoreTest.testTransactionWithQuery(ITDatastoreTest.java:284)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:299)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:293)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.lang.Thread.run(Thread.java:748)

it.ITDatastoreTest.testTransactionWithRead
this test would also timeout if this code was updating ENTITY3 as stated in the comment.

java.lang.AssertionError: Expecting a failure
	at org.junit.Assert.fail(Assert.java:89)
	at com.google.cloud.datastore.it.ITDatastoreTest.testTransactionWithRead(ITDatastoreTest.java:254)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:299)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:293)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.lang.Thread.run(Thread.java:748)

External references such as API reference guides

@product-auto-label product-auto-label bot added the api: datastore Issues related to the googleapis/java-datastore API. label Aug 12, 2021
@meredithslota meredithslota added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Oct 13, 2021
@yoshi-automation yoshi-automation added 🚨 This issue needs some love. and removed 🚨 This issue needs some love. labels Jan 11, 2022
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Feb 8, 2022
@gforgurups gforgurups assigned kolea2 and unassigned gforgurups Feb 16, 2023
@kolea2 kolea2 assigned jainsahab and unassigned kolea2 Feb 21, 2023
@jainsahab
Copy link
Member

jainsahab commented Mar 8, 2023

These tests are verifying the implementation details, and relies on the underlying concurrency control mechanism (optimistic vs pessimistic) that is use to achieve serializability. Instead of testing the implementation details, these tests should verify the behaviour instead i.e. if two transactions conflict with each other, the database guarantees that only one can commit successfully at a time.

gcf-merge-on-green bot pushed a commit that referenced this issue May 19, 2023
…he underlying concurrency (#1079)

fix #497 

Since we are testing the behaviour _(i.e. if two transactions conflict with each other, the database guarantees that only one can commit successfully at a time)_ now, both of the tests are now agnostic of underlying concurrency mechanism. This makes sure that both of the tests continue to pass in any gcloud project .
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the googleapis/java-datastore API. priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
6 participants