[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [LOW] Performance - E2E warmup results are included in the final measurements

[mountiny]

Problem

The warmup results are included in the final measurements. The warmup stage is needed to prepare a network cache for subsequent runs.

Typically, the warmup stage takes much more time than subsequent runs (because it sends real requests and waits for real results). Keeping such records in final measurements leads to unpredictable results (because network request depends on many variables: server responsiveness, speed of the network, etc.). From my observations, I've seen anomaly variables with values ~6-8s (while the average run with cache takes 400ms) -> obviously 8s will significantly increase the average and thus will bias real measurements.

Solution

We don't need to include warmup measurements in the final report and we need to add a mechanism to exclude it.

Issue Owner

Current Issue Owner: @laurenreidexpensify

[melvin-bot]

Triggered auto assignment to @laurenreidexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[kirillzyusko]

Problem

The warmup results are included in final measurements. The warmup stage is needed to prepare a network cache for subsequent runs.
Typically warmup stage takes much more time than subsequent runs (because it sends real request and wait for real results). Keeping such records in final measurements leads to unpredictable results (because network request depends on many variables: server responsiveness, speed of the network, etc.). From my observations I've seen anomaly variables with values ~6-8s (while average run with cache takes 400ms) -> obviously that 8s will significantly increase the average and thus will bias a real measurements.

Solution

We don't need to include warmup measurements in the final report and we need to add a mechanism to exclude it.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.81-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [NoQA] e2e: exclude warmup results from final measurements #43023

If no regressions arise, payment will be issued on 2024-06-18. 🎊

For reference, here are some details about the assignees on this issue:

• @kirillzyusko does not require payment (Contractor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@mountiny] The PR that introduced the bug has been identified. Link to the PR:
• [@mountiny] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@mountiny] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@kirillzyusko] Determine if we should create a regression test for this bug.
• [@kirillzyusko] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@laurenreidexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.82-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [NoQA] e2e: exclude warmup results from final measurements #43023

If no regressions arise, payment will be issued on 2024-06-20. 🎊

For reference, here are some details about the assignees on this issue:

• @kirillzyusko does not require payment (Contractor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@mountiny] The PR that introduced the bug has been identified. Link to the PR:
• [@mountiny] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@mountiny] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@kirillzyusko] Determine if we should create a regression test for this bug.
• [@kirillzyusko] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@laurenreidexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[laurenreidexpensify]

Payment Summary:

• no payment required - @kirillzyusko is agency worker

[laurenreidexpensify]

I don't think we need a regression test here - @mountiny pls reopen if you disagree