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.

Sign up for GitHub

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: change network request sweep interval from 1s -> 10s #25209

Merged
merged 9 commits into from
Dec 21, 2022
Merged

fix: change network request sweep interval from 1s -> 10s #25209

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
106041f
fix: change sweep interval from 1s -> 10s
lmiller1990 Dec 19, 2022
6b78454
binaries
lmiller1990 Dec 19, 2022
f6c35cf
Merge branch 'develop' into fix/correct-sweep-interval
mjhenkes Dec 19, 2022
ba9339f
update variable name
lmiller1990 Dec 19, 2022
eb50d14
Merge remote-tracking branch 'origin/develop' into fix/correct-sweep-…
lmiller1990 Dec 19, 2022
4a9bc6f
use DI to make PreRequest class more testable
lmiller1990 Dec 20, 2022
371aa25
revery circle yml
lmiller1990 Dec 20, 2022
5567d30
revert code [skip ci]
lmiller1990 Dec 20, 2022
53cfa64
try tweaking test
lmiller1990 Dec 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .circleci/workflows.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ mainBuildFilters: &mainBuildFilters
only:
- develop
- /^release\/\d+\.\d+\.\d+$/
- 'feature/create-from-react-component'
- 'fix/correct-sweep-interval'

# usually we don't build Mac app - it takes a long time
# but sometimes we want to really confirm we are doing the right thing
Expand All @@ -37,15 +37,15 @@ macWorkflowFilters: &darwin-workflow-filters
when:
or:
- equal: [ develop, << pipeline.git.branch >> ]
- equal: [ 'feature/create-from-react-component', << pipeline.git.branch >> ]
- equal: [ 'fix/correct-sweep-interval', << pipeline.git.branch >> ]
- matches:
pattern: /^release\/\d+\.\d+\.\d+$/
value: << pipeline.git.branch >>
linuxArm64WorkflowFilters: &linux-arm64-workflow-filters
when:
or:
- equal: [ develop, << pipeline.git.branch >> ]
- equal: [ 'feature/create-from-react-component', << pipeline.git.branch >> ]
- equal: [ 'fix/correct-sweep-interval', << pipeline.git.branch >> ]
- matches:
pattern: /^release\/\d+\.\d+\.\d+$/
value: << pipeline.git.branch >>
Expand All @@ -63,7 +63,7 @@ windowsWorkflowFilters: &windows-workflow-filters
when:
or:
- equal: [ develop, << pipeline.git.branch >> ]
- equal: [ 'feature/create-from-react-component', << pipeline.git.branch >> ]
- equal: [ 'fix/correct-sweep-interval', << pipeline.git.branch >> ]
- matches:
pattern: /^release\/\d+\.\d+\.\d+$/
value: << pipeline.git.branch >>
Expand Down Expand Up @@ -129,7 +129,7 @@ commands:
- run:
name: Check current branch to persist artifacts
command: |
if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "feature/create-from-react-component" ]]; then
if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "fix/correct-sweep-interval" ]]; then
echo "Not uploading artifacts or posting install comment for this branch."
circleci-agent step halt
fi
Expand Down
9 changes: 5 additions & 4 deletions packages/proxy/lib/http/util/prerequests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ export class PreRequests {
requestTimeout: number
pendingPreRequests = new StackMap<PendingPreRequest>()
pendingRequests = new StackMap<PendingRequest>()
sweepInterval: ReturnType<typeof setInterval>
sweepIntervalId: NodeJS.Timeout
Copy link
Contributor

@flotwig flotwig Dec 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

NodeJS.Timeout is an object, not an ID

Suggested change
sweepIntervalId: NodeJS.Timeout
sweepIntervalTimer: NodeJS.Timeout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Woah, did not know this. Fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ba9339f

sweepInterval = 1000 * 10 // 10 seconds

constructor (requestTimeout = 500) {
// If a request comes in and we don't have a matching pre-request after this timeout,
Expand All @@ -94,11 +95,11 @@ export class PreRequests {
// make sure we don't discard any pre-requests prematurely but that we don't leak memory over time
// if a large number of pre-requests don't match up
// fixes: https://github.com/cypress-io/cypress/issues/17853
this.sweepInterval = setInterval(() => {
this.sweepIntervalId = setInterval(() => {
const now = Date.now()

this.pendingPreRequests.removeMatching(({ timestamp, browserPreRequest }) => {
if (timestamp + this.requestTimeout * 2 < now) {
if (timestamp + this.sweepInterval < now) {
debugVerbose('timed out unmatched pre-request: %o', browserPreRequest)
metrics.unmatchedPreRequests++

Expand All @@ -107,7 +108,7 @@ export class PreRequests {

return true
})
}, this.requestTimeout * 2)
}, this.sweepInterval)
}

addPending (browserPreRequest: BrowserPreRequest) {
Expand Down