Skip to content
This repository was archived by the owner on Feb 20, 2026. It is now read-only.

Commit 73a0a39

Browse files
danieljbrucedanieljbruce
authored
fix: read time should be used for transaction reads (#1171)
* Allow datastore projectId to be fetched from clien Latency is caused by the call to getProjectId from Google auth. This change allows the project id to be retrieved if it is set in the client at creation time thereby reducing call latency. * Create a file for mocking out commits A test file is created where we mock out commit in the Gapic layer. The mock allows us to get the results passed to the commit endpoint in the Gapic layer. * Create a test to measure latency of call. To prove that the change works to reduce latency, a test is written. The test checks to see that the amount of time that passes between the time when the initial call is made in the user’s code and the time when the call reaches the gapic layer is sufficiently small. It will be a very small amount of time if the program does not need to do an auth lookup. * Run the linting fixes Run the linter so that spacing in the PR gets fixed for some of the lines of code. * Add license header to top of test file The license header needs to be added to the top of the new test file that is used for mocking out commit. * Add comment for test for now This is going to be a test for investigating the latency of the client. * Add a test for the mock server Measure the latency between the original call and the mock server. * Set current retry attempt to 0 * Add a log for call time Do check external to function after async call. Add log for call time. * Add another mock Other mock doesn’t require lazy client initialization. * Eliminate code from the mock file Eliminate the fake datastore client because we need to do assertion checks that are specific to each test. This means there is no point in defining runQuery once in a mock because each test will mock it out differently. * Start off by adding read time to read options Add the code change that will add read time to read options for transactions. # Conflicts: # test/transaction.ts * Update the test to use transactions The idea is to test that read time got passed along for transactions specifically. This will be necessary for snapshot reads to work. * Remove only Need the entire test suite to run * Remove the before hook The before hook is not necessary. Just mock out the data client at the start. * Remove unnecessary cherry picked files Files were cherry-picked that weren’t helpful for solving the problem. Remove them. * Clean up PR diff * clean up PR diff * Update the test so that it is run as a transaction Right now, providing a transaction id is necessary to run the request as a transaction. * Add an integration test The integration test looks at the data from the snapshot read time for transactions and ensures that the read has no data thereby exercising the read time parameter. * Linting fixing indents Fix the indents in the system test folder * Update the header * Fix unit test beginTransaction needs to be mocked out now that a transaction will begin if runQuery is called. * System test changes. Add a sleep. Instead of changing the current test, add a new test because it means the reader of the PR can be sure that test coverage wasn’t reduced which is better. * Modify test Modify the test so that sleeps are long enough to create predictable results and tests actually check for the right values. * Replace with less precise assert The test setup sometimes prepares before data with 0 entries and sometimes prepares before data with 1 entry so a less restrictive test is required in order for it to consistently pass.
1 parent 10f85fd commit 73a0a39

File tree

3 files changed

+167
-3
lines changed

3 files changed

+167
-3
lines changed

‎src/request.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,9 +1025,13 @@ class DatastoreRequest {
10251025
);
10261026
}
10271027

1028-
reqOpts.readOptions = {
1029-
transaction: this.id,
1030-
};
1028+
if (reqOpts.readOptions) {
1029+
Object.assign(reqOpts.readOptions, {transaction: this.id});
1030+
} else {
1031+
reqOpts.readOptions = {
1032+
transaction: this.id,
1033+
};
1034+
}
10311035
}
10321036

10331037
datastore.auth.getProjectId((err, projectId) => {

‎system-test/datastore.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1931,6 +1931,29 @@ async.each(
19311931
});
19321932
});
19331933
describe('transactions', () => {
1934+
before(async () => {
1935+
// This 'sleep' function is used to ensure that when data is saved to datastore,
1936+
// the time on the server is far enough ahead to be sure to be later than timeBeforeDataCreation
1937+
// so that when we read at timeBeforeDataCreation we get a snapshot of data before the save.
1938+
const key = datastore.key(['Company', 'Google']);
1939+
function sleep(ms: number) {
1940+
return new Promise(resolve => setTimeout(resolve, ms));
1941+
}
1942+
// Save for a key so that a read time can be accessed for snapshot reads.
1943+
const emptyData = {
1944+
key,
1945+
data: {},
1946+
};
1947+
await datastore.save(emptyData);
1948+
// Sleep for 10 seconds to ensure timeBeforeDataCreation includes the empty data
1949+
await sleep(10000);
1950+
timeBeforeDataCreation = await getReadTime([
1951+
{kind: 'Company', name: 'Google'},
1952+
]);
1953+
// Sleep for 10 seconds so that any future reads will be later than timeBeforeDataCreation.
1954+
await sleep(10000);
1955+
});
1956+
19341957
it('should run in a transaction', async () => {
19351958
const key = datastore.key(['Company', 'Google']);
19361959
const obj = {
@@ -2031,6 +2054,25 @@ async.each(
20312054
await transaction.commit();
20322055
});
20332056

2057+
it('should query within a transaction at a previous read time', async () => {
2058+
const transaction = datastore.transaction();
2059+
await transaction.run();
2060+
const query = transaction.createQuery('Company');
2061+
let entitiesBefore;
2062+
let entitiesNow;
2063+
try {
2064+
[entitiesBefore] = await query.run({
2065+
readTime: timeBeforeDataCreation,
2066+
});
2067+
[entitiesNow] = await query.run({});
2068+
} catch (e) {
2069+
await transaction.rollback();
2070+
return;
2071+
}
2072+
assert(entitiesBefore!.length < entitiesNow!.length);
2073+
await transaction.commit();
2074+
});
2075+
20342076
describe('aggregate query within a transaction', async () => {
20352077
it('should run a query and return the results', async () => {
20362078
// Add a test here to verify what the data is at this time.

‎test/gapic-mocks/runQuery.ts‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
// Copyright 2024 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
import * as assert from 'assert';
16+
import {describe} from 'mocha';
17+
import {DatastoreClient, Datastore} from '../../src';
18+
import * as protos from '../../protos/protos';
19+
import {Callback} from 'google-gax';
20+
21+
describe('Run Query', () => {
22+
const PROJECT_ID = 'project-id';
23+
const NAMESPACE = 'namespace';
24+
const clientName = 'DatastoreClient';
25+
const options = {
26+
projectId: PROJECT_ID,
27+
namespace: NAMESPACE,
28+
};
29+
const datastore = new Datastore(options);
30+
// By default, datastore.clients_ is an empty map.
31+
// To mock out commit we need the map to contain the Gapic data client.
32+
// Normally a call to the data client through the datastore object would initialize it.
33+
// We don't want to make this call because it would make a grpc request.
34+
// So we just add the data client to the map.
35+
const gapic = Object.freeze({
36+
v1: require('../../src/v1'),
37+
});
38+
datastore.clients_.set(clientName, new gapic.v1[clientName](options));
39+
40+
// This function is used for doing assertion checks.
41+
// The idea is to check that the right request gets passed to the commit function in the Gapic layer.
42+
function setRunQueryComparison(
43+
compareFn: (request: protos.google.datastore.v1.IRunQueryRequest) => void
44+
) {
45+
const dataClient = datastore.clients_.get(clientName);
46+
if (dataClient) {
47+
dataClient.runQuery = (
48+
request: any,
49+
options: any,
50+
callback: (
51+
err?: unknown,
52+
res?: protos.google.datastore.v1.IRunQueryResponse
53+
) => void
54+
) => {
55+
try {
56+
compareFn(request);
57+
} catch (e) {
58+
callback(e);
59+
}
60+
callback(null, {
61+
batch: {
62+
moreResults:
63+
protos.google.datastore.v1.QueryResultBatch.MoreResultsType
64+
.NO_MORE_RESULTS,
65+
},
66+
});
67+
};
68+
}
69+
}
70+
71+
it('should pass read time into runQuery for transactions', async () => {
72+
// First mock out beginTransaction
73+
const dataClient = datastore.clients_.get(clientName);
74+
const testId = Buffer.from(Array.from(Array(100).keys()));
75+
if (dataClient) {
76+
dataClient.beginTransaction = (
77+
request: protos.google.datastore.v1.IBeginTransactionRequest,
78+
options: protos.google.datastore.v1.IBeginTransactionResponse,
79+
callback: Callback<
80+
protos.google.datastore.v1.IBeginTransactionResponse,
81+
| protos.google.datastore.v1.IBeginTransactionRequest
82+
| null
83+
| undefined,
84+
{} | null | undefined
85+
>
86+
) => {
87+
callback(null, {
88+
transaction: testId,
89+
});
90+
};
91+
}
92+
setRunQueryComparison(
93+
(request: protos.google.datastore.v1.IRunQueryRequest) => {
94+
assert.deepStrictEqual(request, {
95+
readOptions: {
96+
transaction: testId,
97+
readTime: {
98+
seconds: 77,
99+
},
100+
},
101+
partitionId: {
102+
namespaceId: 'namespace',
103+
},
104+
query: {
105+
distinctOn: [],
106+
kind: [{name: 'Task'}],
107+
order: [],
108+
projection: [],
109+
},
110+
projectId: 'project-id',
111+
});
112+
}
113+
);
114+
const transaction = datastore.transaction();
115+
const query = datastore.createQuery('Task');
116+
await transaction.runQuery(query, {readTime: 77000});
117+
});
118+
});

0 commit comments

Comments
 (0)