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

OJ-2748 lambda warmer stage added to performance tests #827

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

sandragds
Copy link

@sandragds sandragds commented Sep 2, 2024

What?

Set up stage for imposter stub to warm up lambdas before performance testing is ran.

Changes:

  • function added to run requests to imposter stub in nino-check
  • nino-check.ts script modified to include the request before the performance test

Why?

  • Due to lambda cold start issue, performance tests are slow in imposter stubs. To mediate the issue, lambdas can be warmed up by sending requests to the imposter stubs before the performance tests.

Related:

@sandragds sandragds requested review from a team as code owners September 2, 2024 15:29
@sandragds sandragds marked this pull request as draft September 2, 2024 15:34
@sandragds sandragds force-pushed the OJ-2748-lambda-warmer branch from 5e2f520 to 2264d25 Compare September 18, 2024 08:06
@abhinaysrivastavaperf
Copy link
Collaborator

Just to highlight, the latest changes mean that the lambda warmer call will be executed each iteration.

Comment on lines 90 to 96
let imposterstarted = false
if (!imposterstarted) {
const response = imposter.handler()
console.log('Response status: ', response.status)
console.log('Response body: ', response.body)
imposterstarted = true
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let imposterstarted = false
if (!imposterstarted) {
const response = imposter.handler()
console.log('Response status: ', response.status)
console.log('Response body: ', response.body)
imposterstarted = true
}
const response = imposter.handler()
console.log('Response status: ', response.status)
console.log('Response body: ', response.body)

This code is functionally the same as removing the imposterstarted variable and the if statement, as it will always run.

If you are trying to run this once at the beginning of the test I'd move this call to the setup() function, or if you want to run it once per VU the imposterstarted will need to be declared outside of the function scope.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I included this in the setup() function initially but I had a chat with Sandra and was advised that this needs to run periodically instead of only once.
To avoid running this as many time as the actual test iterations, we may have a separate function for this which will be called like 10% or 20% of the total number of executed iterations

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, well that approach will need to be implemented and the code smell fixed first before merging

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the approach to make 10 concurrent request (as a batch request) in the setUp function and fixed the code smell.

Copy link

sonarqubecloud bot commented Oct 4, 2024

Copy link
Contributor

@Tom-Dann Tom-Dann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the PR title, but otherwise okay

@sandragds sandragds changed the title OJ-2748 one request set in nino-check script to warm lambda OJ-2748 lambda warmer stage added to performance tests Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants