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

Fix flaky lambda concurrency test #9180

Merged
merged 2 commits into from Sep 19, 2023
Merged

Conversation

joe4dev
Copy link
Member

@joe4dev joe4dev commented Sep 19, 2023

Motivation

A flaky Lambda test occasionally blocks the pipeline.

Thank you @bentsku for raising this https://localstack-cloud.slack.com/archives/C02U7DN1B4M/p1695076802171769

Changes

  • Increase reserved concurrency in test case
  • Document race condition
  • Add extra debug logging for concurrency exceptions

Discussion

a) The safest fix is to increase ReservedConcurrentExecutions=3 but then we only occasionally test the border case.
b) We could slow down the test and use sleeps to have a high likelyhood of succeeding
c) We separate the mixed async/sync invoke into a separate test case (or after asserting the logs).

@joe4dev joe4dev added the semver: patch Non-breaking changes which can be included in patch releases label Sep 19, 2023
@joe4dev joe4dev added this to the 2.3 milestone Sep 19, 2023
@joe4dev joe4dev self-assigned this Sep 19, 2023
@coveralls
Copy link

Coverage Status

coverage: 80.004% (-3.0%) from 82.998% when pulling 1c5fc7a on fix-flaky-lambda-concurrency-test into 1b16827 on master.

@github-actions
Copy link

LocalStack Community integration with Pro

       2 files         2 suites   1h 10m 29s ⏱️
2 214 tests 1 717 ✔️ 497 💤 0
2 215 runs  1 717 ✔️ 498 💤 0

Results for commit 1c5fc7a.

Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

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

a) The safest fix is to increase ReservedConcurrentExecutions=3 but then we only occasionally test the border case.
b) We could slow down the test and use sleeps to have a high likelyhood of succeeding
c) We separate the mixed async/sync invoke into a separate test case (or after asserting the logs).

Let's do a) for now with c) in the long run 👍

@joe4dev joe4dev merged commit a4594a7 into master Sep 19, 2023
28 of 29 checks passed
@joe4dev joe4dev deleted the fix-flaky-lambda-concurrency-test branch September 19, 2023 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants