From 044c8746b9d5751667ad5b3596d13a8a4ac56e1b Mon Sep 17 00:00:00 2001 From: Leena V <shadowkitten32@gmail.com> Date: Mon, 18 Nov 2024 23:35:58 +0000 Subject: [PATCH 1/9] fix: hotswap for functions uses exponential backoff --- .../lib/api/hotswap/appsync-mapping-templates.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts b/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts index d05aa56063339..aec6b81e388e2 100644 --- a/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts +++ b/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts @@ -118,7 +118,7 @@ export async function isHotswappableAppSyncChange( const functions = await sdk.appsync().listFunctions({ apiId: sdkRequestObject.apiId }); const { functionId } = functions.find((fn) => fn.name === physicalName) ?? {}; // Updating multiple functions at the same time or along with graphql schema results in `ConcurrentModificationException` - await simpleRetry( + await exponentialBackoffRetry( () => sdk.appsync().updateFunction({ ...sdkRequestObject, @@ -169,13 +169,13 @@ async function fetchFileFromS3(s3Url: string, sdk: SDK) { return (await sdk.s3().getObject({ Bucket: s3Bucket, Key: s3Key })).Body?.transformToString(); } -async function simpleRetry(fn: () => Promise<any>, numOfRetries: number, errorCodeToRetry: string) { +async function exponentialBackoffRetry(fn: () => Promise<any>, backoff: number, errorCodeToRetry: string) { try { await fn(); } catch (error: any) { - if (error && error.name === errorCodeToRetry && numOfRetries > 0) { - await sleep(1000); // wait a whole second - await simpleRetry(fn, numOfRetries - 1, errorCodeToRetry); + if (error && error.name === errorCodeToRetry) { + await sleep(backoff); // time to wait doubles everytime function fails, starts at 1 second + await exponentialBackoffRetry(fn, backoff * 2, errorCodeToRetry); } else { throw error; } From 2d869f27a8baa0bc776aa9ec3bdc66425fad5f58 Mon Sep 17 00:00:00 2001 From: Leena V <shadowkitten32@gmail.com> Date: Mon, 18 Nov 2024 23:36:47 +0000 Subject: [PATCH 2/9] fix: implement base backoff as 1 second --- packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts b/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts index aec6b81e388e2..9b002fdfcaba4 100644 --- a/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts +++ b/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts @@ -124,7 +124,7 @@ export async function isHotswappableAppSyncChange( ...sdkRequestObject, functionId: functionId, }), - 5, + 1000, 'ConcurrentModificationException', ); } else if (isGraphQLSchema) { From 633c9ea104c03b08065e7273d403015436ac201e Mon Sep 17 00:00:00 2001 From: Leena V <shadowkitten32@gmail.com> Date: Tue, 19 Nov 2024 19:06:23 +0000 Subject: [PATCH 3/9] fix: added retry limit --- .../lib/api/hotswap/appsync-mapping-templates.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts b/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts index 9b002fdfcaba4..d5b520d6a13e5 100644 --- a/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts +++ b/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts @@ -118,12 +118,13 @@ export async function isHotswappableAppSyncChange( const functions = await sdk.appsync().listFunctions({ apiId: sdkRequestObject.apiId }); const { functionId } = functions.find((fn) => fn.name === physicalName) ?? {}; // Updating multiple functions at the same time or along with graphql schema results in `ConcurrentModificationException` - await exponentialBackoffRetry( + await exponentialBackOffRetry( () => sdk.appsync().updateFunction({ ...sdkRequestObject, functionId: functionId, }), + 6, 1000, 'ConcurrentModificationException', ); @@ -169,13 +170,13 @@ async function fetchFileFromS3(s3Url: string, sdk: SDK) { return (await sdk.s3().getObject({ Bucket: s3Bucket, Key: s3Key })).Body?.transformToString(); } -async function exponentialBackoffRetry(fn: () => Promise<any>, backoff: number, errorCodeToRetry: string) { +async function exponentialBackOffRetry(fn: () => Promise<any>, numOfRetries: number, backOff: number, errorCodeToRetry: string) { try { await fn(); } catch (error: any) { - if (error && error.name === errorCodeToRetry) { - await sleep(backoff); // time to wait doubles everytime function fails, starts at 1 second - await exponentialBackoffRetry(fn, backoff * 2, errorCodeToRetry); + if (error && error.name === errorCodeToRetry && numOfRetries > 0) { + await sleep(backOff); // time to wait doubles everytime function fails, starts at 1 second + await exponentialBackOffRetry(fn, numOfRetries - 1, backOff * 2, errorCodeToRetry); } else { throw error; } From b782d075548fbc8a11789ddd81e454c021b1fdaf Mon Sep 17 00:00:00 2001 From: Leena V <shadowkitten32@gmail.com> Date: Mon, 9 Dec 2024 19:36:46 +0000 Subject: [PATCH 4/9] update test coverage --- ...ping-templates-hotswap-deployments.test.ts | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts index 909345ec2f729..b8df22ac91b8c 100644 --- a/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts @@ -969,6 +969,62 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot }, ); + silentTest( + 'calls the startSchemaCreation() API and ensures that it resolves via hotswap on failure with ConcurrentModificationException', + async () => { + // GIVEN + mockAppSyncClient.on(GetSchemaCreationStatusCommand).resolvesOnce({ status: 'FAILED', details: 'ConcurrentModificationException' }); + + setup.setCurrentCfnStackTemplate({ + Resources: { + AppSyncGraphQLSchema: { + Type: 'AWS::AppSync::GraphQLSchema', + Properties: { + ApiId: 'apiId', + Definition: 'original graphqlSchema', + }, + Metadata: { + 'aws:asset:path': 'old-path', + }, + }, + }, + }); + setup.pushStackResourceSummaries( + setup.stackSummaryOf( + 'AppSyncGraphQLSchema', + 'AWS::AppSync::GraphQLSchema', + 'arn:aws:appsync:us-east-1:111111111111:apis/apiId/schema/my-schema', + ), + ); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + AppSyncGraphQLSchema: { + Type: 'AWS::AppSync::GraphQLSchema', + Properties: { + ApiId: 'apiId', + Definition: 'new graphqlSchema', + }, + Metadata: { + 'aws:asset:path': 'new-path', + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockAppSyncClient).toHaveReceivedCommandWith(StartSchemaCreationCommand, { + apiId: 'apiId', + definition: 'new graphqlSchema', + }); + }, + ); + silentTest('calls the updateFunction() API with functionId when function is listed on second page', async () => { // GIVEN mockAppSyncClient From dfb2680c65658a6246742f172096b1f897c04193 Mon Sep 17 00:00:00 2001 From: Leena V <shadowkitten32@gmail.com> Date: Mon, 9 Dec 2024 21:29:26 +0000 Subject: [PATCH 5/9] update to unit test --- ...ping-templates-hotswap-deployments.test.ts | 48 ++++++++++++------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts index b8df22ac91b8c..ef87a0da22fde 100644 --- a/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts @@ -970,18 +970,28 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot ); silentTest( - 'calls the startSchemaCreation() API and ensures that it resolves via hotswap on failure with ConcurrentModificationException', + 'updateFunction() API recovers from failed update attempt through retry logic', async () => { - // GIVEN - mockAppSyncClient.on(GetSchemaCreationStatusCommand).resolvesOnce({ status: 'FAILED', details: 'ConcurrentModificationException' }); + const ConcurrentModError = new Error('ConcurrentModificationException: Schema is currently being altered, please wait until that is complete.'); + ConcurrentModError.name = 'ConcurrentModificationException'; + mockAppSyncClient + .on(ListFunctionsCommand) + .resolvesOnce({ functions: [{ name: 'my-function', functionId: 'functionId' }] }).rejects(ConcurrentModError); + mockAppSyncClient + .on(ListFunctionsCommand) + .resolvesOnce({ functions: [{ name: 'my-function', functionId: 'functionId' }] }); setup.setCurrentCfnStackTemplate({ Resources: { - AppSyncGraphQLSchema: { - Type: 'AWS::AppSync::GraphQLSchema', + AppSyncFunction: { + Type: 'AWS::AppSync::FunctionConfiguration', Properties: { + Name: 'my-function', ApiId: 'apiId', - Definition: 'original graphqlSchema', + DataSourceName: 'my-datasource', + FunctionVersion: '2018-05-29', + RequestMappingTemplate: '## original request template', + ResponseMappingTemplate: '## original response template', }, Metadata: { 'aws:asset:path': 'old-path', @@ -989,21 +999,18 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot }, }, }); - setup.pushStackResourceSummaries( - setup.stackSummaryOf( - 'AppSyncGraphQLSchema', - 'AWS::AppSync::GraphQLSchema', - 'arn:aws:appsync:us-east-1:111111111111:apis/apiId/schema/my-schema', - ), - ); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { Resources: { - AppSyncGraphQLSchema: { - Type: 'AWS::AppSync::GraphQLSchema', + AppSyncFunction: { + Type: 'AWS::AppSync::FunctionConfiguration', Properties: { + Name: 'my-function', ApiId: 'apiId', - Definition: 'new graphqlSchema', + DataSourceName: 'my-datasource', + FunctionVersion: '2018-05-29', + RequestMappingTemplate: '## original request template', + ResponseMappingTemplate: '## new response template', }, Metadata: { 'aws:asset:path': 'new-path', @@ -1018,9 +1025,14 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).not.toBeUndefined(); - expect(mockAppSyncClient).toHaveReceivedCommandWith(StartSchemaCreationCommand, { + expect(mockAppSyncClient).toHaveReceivedCommandWith(UpdateFunctionCommand, { apiId: 'apiId', - definition: 'new graphqlSchema', + dataSourceName: 'my-datasource', + functionId: 'functionId', + functionVersion: '2018-05-29', + name: 'my-function', + requestMappingTemplate: '## original request template', + responseMappingTemplate: '## new response template', }); }, ); From 63b1bfd66bd6815fafaf546eea312f7e1b3cafc5 Mon Sep 17 00:00:00 2001 From: Leena V <shadowkitten32@gmail.com> Date: Mon, 9 Dec 2024 22:12:09 +0000 Subject: [PATCH 6/9] update to error rejection --- .../appsync-mapping-templates-hotswap-deployments.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts index ef87a0da22fde..d235250ab786d 100644 --- a/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts @@ -975,8 +975,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot const ConcurrentModError = new Error('ConcurrentModificationException: Schema is currently being altered, please wait until that is complete.'); ConcurrentModError.name = 'ConcurrentModificationException'; mockAppSyncClient - .on(ListFunctionsCommand) - .resolvesOnce({ functions: [{ name: 'my-function', functionId: 'functionId' }] }).rejects(ConcurrentModError); + .on(ListFunctionsCommand).rejects(ConcurrentModError); mockAppSyncClient .on(ListFunctionsCommand) .resolvesOnce({ functions: [{ name: 'my-function', functionId: 'functionId' }] }); From 7de6463eae62e74d113c3ade0dfa9bd199fc64f0 Mon Sep 17 00:00:00 2001 From: Leena V <shadowkitten32@gmail.com> Date: Tue, 10 Dec 2024 00:22:02 +0000 Subject: [PATCH 7/9] back off retry test --- ...ping-templates-hotswap-deployments.test.ts | 81 ++++++++++++++++++- 1 file changed, 77 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts index d235250ab786d..93023c318f046 100644 --- a/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts @@ -975,10 +975,9 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot const ConcurrentModError = new Error('ConcurrentModificationException: Schema is currently being altered, please wait until that is complete.'); ConcurrentModError.name = 'ConcurrentModificationException'; mockAppSyncClient - .on(ListFunctionsCommand).rejects(ConcurrentModError); - mockAppSyncClient - .on(ListFunctionsCommand) - .resolvesOnce({ functions: [{ name: 'my-function', functionId: 'functionId' }] }); + .on(UpdateFunctionCommand) + .rejectsOnce(ConcurrentModError) + .resolvesOnce({ functionConfiguration: { name: 'my-function', dataSourceName: 'my-datasource', functionId: 'functionId' } }); setup.setCurrentCfnStackTemplate({ Resources: { @@ -1036,6 +1035,80 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot }, ); + silentTest( + 'updateFunction() API fails if it recieves 6 failed attempts in a row - this is a long running test', + async () => { + const ConcurrentModError = new Error('ConcurrentModificationException: Schema is currently being altered, please wait until that is complete.'); + ConcurrentModError.name = 'ConcurrentModificationException'; + mockAppSyncClient + .on(UpdateFunctionCommand) + .rejectsOnce(ConcurrentModError) + .rejectsOnce(ConcurrentModError) + .rejectsOnce(ConcurrentModError) + .rejectsOnce(ConcurrentModError) + .rejectsOnce(ConcurrentModError) + .rejectsOnce(ConcurrentModError) + .rejectsOnce(ConcurrentModError) + .resolvesOnce({ functionConfiguration: { name: 'my-function', dataSourceName: 'my-datasource', functionId: 'functionId' } }); + + setup.setCurrentCfnStackTemplate({ + Resources: { + AppSyncFunction: { + Type: 'AWS::AppSync::FunctionConfiguration', + Properties: { + Name: 'my-function', + ApiId: 'apiId', + DataSourceName: 'my-datasource', + FunctionVersion: '2018-05-29', + RequestMappingTemplate: '## original request template', + ResponseMappingTemplate: '## original response template', + }, + Metadata: { + 'aws:asset:path': 'old-path', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + AppSyncFunction: { + Type: 'AWS::AppSync::FunctionConfiguration', + Properties: { + Name: 'my-function', + ApiId: 'apiId', + DataSourceName: 'my-datasource', + FunctionVersion: '2018-05-29', + RequestMappingTemplate: '## original request template', + ResponseMappingTemplate: '## new response template', + }, + Metadata: { + 'aws:asset:path': 'new-path', + }, + }, + }, + }, + }); + + // WHEN + await expect(() => hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact)).rejects.toThrow( + 'ConcurrentModificationException', + ); + + // THEN + expect(mockAppSyncClient).not.toHaveReceivedCommandWith(UpdateFunctionCommand, { + apiId: 'apiId', + dataSourceName: 'my-datasource', + functionId: 'functionId', + functionVersion: '2018-05-29', + name: 'my-function', + requestMappingTemplate: '## original request template', + responseMappingTemplate: '## new response template', + }); + }, + 320000, + ); + silentTest('calls the updateFunction() API with functionId when function is listed on second page', async () => { // GIVEN mockAppSyncClient From 0b657ba0a7c8beca18f9b4456033ee77dc76258f Mon Sep 17 00:00:00 2001 From: Leena V <shadowkitten32@gmail.com> Date: Tue, 10 Dec 2024 00:27:04 +0000 Subject: [PATCH 8/9] update failed attempt count --- .../appsync-mapping-templates-hotswap-deployments.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts index 93023c318f046..f70914bb65e65 100644 --- a/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts @@ -1036,7 +1036,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot ); silentTest( - 'updateFunction() API fails if it recieves 6 failed attempts in a row - this is a long running test', + 'updateFunction() API fails if it recieves 7 failed attempts in a row - this is a long running test', async () => { const ConcurrentModError = new Error('ConcurrentModificationException: Schema is currently being altered, please wait until that is complete.'); ConcurrentModError.name = 'ConcurrentModificationException'; From 07008e2315a5cd2345b7704f7430e7252758fe6d Mon Sep 17 00:00:00 2001 From: Praveen Gupta <pravgupt@amazon.com> Date: Tue, 10 Dec 2024 16:54:24 +0100 Subject: [PATCH 9/9] fix test --- ...ping-templates-hotswap-deployments.test.ts | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts index f70914bb65e65..46e03cc88aba2 100644 --- a/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts @@ -972,6 +972,14 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot silentTest( 'updateFunction() API recovers from failed update attempt through retry logic', async () => { + + // GIVEN + mockAppSyncClient + .on(ListFunctionsCommand) + .resolvesOnce({ + functions: [{ name: 'my-function', functionId: 'functionId' }], + }); + const ConcurrentModError = new Error('ConcurrentModificationException: Schema is currently being altered, please wait until that is complete.'); ConcurrentModError.name = 'ConcurrentModificationException'; mockAppSyncClient @@ -1023,6 +1031,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).not.toBeUndefined(); + expect(mockAppSyncClient).toHaveReceivedCommandTimes(UpdateFunctionCommand, 2); // 1st failure then success on retry expect(mockAppSyncClient).toHaveReceivedCommandWith(UpdateFunctionCommand, { apiId: 'apiId', dataSourceName: 'my-datasource', @@ -1038,6 +1047,14 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot silentTest( 'updateFunction() API fails if it recieves 7 failed attempts in a row - this is a long running test', async () => { + + // GIVEN + mockAppSyncClient + .on(ListFunctionsCommand) + .resolvesOnce({ + functions: [{ name: 'my-function', functionId: 'functionId' }], + }); + const ConcurrentModError = new Error('ConcurrentModificationException: Schema is currently being altered, please wait until that is complete.'); ConcurrentModError.name = 'ConcurrentModificationException'; mockAppSyncClient @@ -1096,7 +1113,8 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot ); // THEN - expect(mockAppSyncClient).not.toHaveReceivedCommandWith(UpdateFunctionCommand, { + expect(mockAppSyncClient).toHaveReceivedCommandTimes(UpdateFunctionCommand, 7); // 1st attempt and then 6 retries before bailing + expect(mockAppSyncClient).toHaveReceivedCommandWith(UpdateFunctionCommand, { apiId: 'apiId', dataSourceName: 'my-datasource', functionId: 'functionId',