Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

perf(client) reduce amount of timers on init_worker #130

Merged
merged 6 commits into from Jun 10, 2021
Merged

Conversation

ghost
Copy link

@ghost ghost commented May 27, 2021

We end up scheduling 1 timer for each hostname on the init_worker phase. To fix it we are "deduplicating" timers created to resolve the same hostname for the asynchronous case. That is, we are reusing timers for DNS queries of the same hostname. If we have 6k targets, and all of the targets are "httpbin.org" there's no reason to create 6k timers per worker. Instead, we should just create a maximum of 3 timers for 3 DNS queries (default: 1 for SRV, 1 for A and 1 for CNAME) and don't schedule more timers than that to resolve the same hostname. We should only schedule more timers if we try to resolve different hostnames. And the approach we're using in the patch is: if the current nginx phase can't be blocked by a semaphore (which is the case of init_worker context, but not of timer context, for example), then we should not schedule more timers for the same hostname, we should just return the previous query so that the previous timers can be reutilized (in the init_worker phase, for example). Otherwise, if the current nginx phase can blocked by a semaphore, then we will just block and wait for the DNS query to complete (in the timer context, for example, and this hasn't changed). In a nutshell: we are reutilizing timers scheduled to resolve the same hostname.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@ghost ghost changed the title Perf/timers usage perf(client) reduce amount of timers on init_worker May 27, 2021
@ghost ghost force-pushed the perf/timers-usage branch 4 times, most recently from a5214dc to 482e734 Compare May 28, 2021 02:31
@ghost ghost force-pushed the perf/timers-usage branch from 482e734 to 4df9b89 Compare May 28, 2021 12:17
@ghost ghost marked this pull request as ready for review May 28, 2021 12:22
@ghost ghost requested review from Tieske and locao and removed request for Tieske May 28, 2021 12:22
@locao locao merged commit 826f445 into master Jun 10, 2021
@locao locao deleted the perf/timers-usage branch June 10, 2021 21:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants