From 589540475b0b2a055018a1cb6e475800fdd46a37 Mon Sep 17 00:00:00 2001 From: Kevin Kim Date: Wed, 15 Jan 2025 10:48:34 -0500 Subject: [PATCH] chore: Common pitfalls unit test (#1564) * clean up server deprecated code * test passes now with changes to test proxy * remove passing test * remove await * fix sync repo settings * grab app profileId from client * remove passing test * changes to timeout * fix deadline error * remove package.json line * fix prettier issues * remove old functions for deadline * Update bulk-mutate-rows.js * Update read-rows.js * Update read-modify-write-row.js * Update read-row.js * add unit test that confirms a common pitfall * fix error syntax * remove describe block * modify code to have server send deadline exceeded error * add modified unit test with hook * have hook handle requests * add test that confirms a pitfall * fix typo * fix prettier * remove unused import --- test/readrows.ts | 40 +++++++++++++++++++++++++ test/utils/readRowsImpl.ts | 23 +++++++++++++- test/utils/readRowsServiceParameters.ts | 4 ++- 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/test/readrows.ts b/test/readrows.ts index 83b24a15c..60385aac2 100644 --- a/test/readrows.ts +++ b/test/readrows.ts @@ -461,6 +461,46 @@ describe('Bigtable/ReadRows', () => { })(); }); + it.skip('pitfall: should not request full table scan during a retry on a transient error', async () => { + const requests = []; + + const TRANSIENT_ERROR_SERVICE: ReadRowsServiceParameters = { + chunkSize: CHUNK_SIZE, + valueSize: VALUE_SIZE, + chunksPerResponse: CHUNKS_PER_RESPONSE, + keyFrom: STANDARD_KEY_FROM, + keyTo: STANDARD_KEY_TO, + deadlineExceededError: true, + hook: request => { + requests.push(request); + }, + debugLog, + }; + + async function readRowsWithDeadline() { + service.setService({ + ReadRows: ReadRowsImpl.createService( + TRANSIENT_ERROR_SERVICE + ) as ServerImplementationInterface, + }); + + const rows = await table.getRows(); + return rows; + } + + try { + await readRowsWithDeadline(); + assert.fail('Should have thrown error'); + } catch (err) { + if (err instanceof GoogleError) { + assert.equal(err.code, 'DEADLINE_EXCEEDED'); + } + + // Assert that no retry attempted. + assert.strictEqual(requests.length, 1); + } + }); + after(async () => { server.shutdown(() => {}); }); diff --git a/test/utils/readRowsImpl.ts b/test/utils/readRowsImpl.ts index 51c22f0bd..77d0a613a 100644 --- a/test/utils/readRowsImpl.ts +++ b/test/utils/readRowsImpl.ts @@ -349,6 +349,11 @@ export class ReadRowsImpl { */ private async handleRequest(stream: ReadRowsWritableStream) { const debugLog = this.serviceParameters.debugLog; + const hook = this.serviceParameters.hook; + if (hook) { + hook(stream.request); + } + prettyPrintRequest(stream.request, debugLog); const readRowsRequestHandler = new ReadRowsRequestHandler(stream, debugLog); stream.on('cancelled', () => { @@ -374,12 +379,15 @@ export class ReadRowsImpl { ) { const stream = readRowsRequestHandler.stream; const debugLog = readRowsRequestHandler.debugLog; + const deadlineExceededError = this.serviceParameters.deadlineExceededError; + let chunksSent = 0; let lastScannedRowKey: string | undefined; let currentResponseChunks: protos.google.bigtable.v2.ReadRowsResponse.ICellChunk[] = []; let chunkIdx = 0; let skipThisRow = false; + for (const chunk of chunks) { if (readRowsRequestHandler.cancelled) { break; @@ -409,12 +417,14 @@ export class ReadRowsImpl { currentResponseChunks.push(chunk); ++chunkIdx; } + if ( currentResponseChunks.length === this.serviceParameters.chunksPerResponse || chunkIdx === this.errorAfterChunkNo || // if we skipped a row and set lastScannedRowKey, dump everything and send a separate message with lastScannedRowKey - lastScannedRowKey + lastScannedRowKey || + deadlineExceededError ) { const response: protos.google.bigtable.v2.IReadRowsResponse = { chunks: currentResponseChunks, @@ -422,6 +432,7 @@ export class ReadRowsImpl { chunksSent += currentResponseChunks.length; await readRowsRequestHandler.sendResponse(response); currentResponseChunks = []; + if (chunkIdx === this.errorAfterChunkNo) { debugLog(`sending error after chunk #${chunkIdx}`); this.errorAfterChunkNo = undefined; // do not send error for the second time @@ -431,7 +442,17 @@ export class ReadRowsImpl { readRowsRequestHandler.cancelled = true; break; } + + if (deadlineExceededError) { + debugLog('sending deadline exceeded error'); + const error = new GoogleError('Deadline exceeded'); + error.code = Status.DEADLINE_EXCEEDED; + stream.emit('error', error); + readRowsRequestHandler.cancelled = true; + break; + } } + if (lastScannedRowKey) { const response: protos.google.bigtable.v2.IReadRowsResponse = { lastScannedRowKey, diff --git a/test/utils/readRowsServiceParameters.ts b/test/utils/readRowsServiceParameters.ts index bf9b2c076..53955ce65 100644 --- a/test/utils/readRowsServiceParameters.ts +++ b/test/utils/readRowsServiceParameters.ts @@ -27,11 +27,13 @@ interface SharedReadRowsParameters { export type DebugLog = (message: string) => void; export interface ReadRowsServiceParameters extends SharedReadRowsParameters { + deadlineExceededError?: boolean; // Send deadline exceeded transient error + errorAfterChunkNo?: number; // The chunk that the error should come after keyFrom?: number; // The key the data coming from the service will start from keyTo?: number; // The key the data coming from the service will end at - errorAfterChunkNo?: number; // The chunk that the error should come after chunksPerResponse: number; // The total number of chunks the server should send debugLog: DebugLog; + hook?: (request: protos.google.bigtable.v2.IReadRowsRequest) => void; } export interface ChunkGeneratorParameters extends SharedReadRowsParameters {