Repository navigation
dialog: keep the lifetime timer on a timing wheel - #4459
Open
subhanyousafcwit wants to merge 1 commit into
Open
subhanyousafcwit wants to merge 1 commit into
subhanyousafcwit wants to merge 1 commit into
Conversation
The lifetime timer kept all dialogs in one list sorted by expiry, and insert_gen_timer_unsafe() found the place of a new timeout by walking back from the tail with d_timer->lock held. That is one step when all dialogs share the same lifetime, but a timeout shorter than the ones already queued (a short $DLG_timeout set at the 200 OK while waiting for the ACK, per-call limits set from the script) walks past every active dialog, under the lock every worker needs. The cost of an insert grows with the number of active dialogs. The lifetime timer now hangs its dialogs on a hashed timing wheel of 16384 lists, one per second: a dialog due at second T goes to list T & mask. Insert, update and removal are O(1). Each run of the timer routine looks only at the lists of the seconds since its previous run (all of them once if it is a whole turn behind) and takes the entries that are due; an entry more than a turn away stays for a later turn. Entries are linked and marked exactly as in the sorted list, so the code around the timer is unchanged. An entry found on a list that is not its own, because its timeout was written directly instead of via update_dlg_timer() (the DB sync of an already known dialog does that), is moved to its own list when found, so a raised timeout is honoured. The delete-delay timer, whose entries all have the same delay, keeps the sorted list. Unit tests check the wheel against a model with random inserts, updates and removals, a late timer routine and timeouts written in place.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Keep the dialog lifetime timer on a hashed timing wheel instead of one sorted list, so inserting, updating and removing a dialog timeout no longer depends on the number of active dialogs.
Details
insert_gen_timer_unsafe()places a new timeout by walking the sorted lifetime list back from the tail, withd_timer->lockheld. When the new timeout is shorter than the ones already queued, the walk visits every active dialog. That is the normal case for scripts that set a short$DLG_timeoutat the 200 OK (waiting for the ACK) and restore the call limit at the ACK, or that set per-call limits. Under load the walk becomes the bottleneck: at 1,000 calls/s with ~3,000 active dialogs it holds the lock for ~0.6 CPU and the other workers spin behind it (details and profile in #4458).This is a generic problem (any script that sets dialog timeouts of different lengths), not tied to particular UACs.
Solution
dlg_timer_wheel.h: a wheel of 16384 lists (one per second, about 4.5 hours per turn). A dialog due at second T is appended to listT & mask, so insert/update/remove are a few pointer writes._get_gen_expired_dlgs()) looks only at the lists of the seconds since its previous run, all of them once if it is a whole turn or more behind, and takes the entries that are due. An entry more than a turn away stays in its list for a later turn.prev/next,FAKE_DIALOG_TL,prev = 0andtimeout = 0when expired), so the callers ofinsert_dlg_timer(),update_dlg_timer(),remove_dlg_timer()and the timer handler are unchanged.dlg_db_handler.cwritestl.timeoutof an already known dialog directly during a DB sync, withoutupdate_dlg_timer(). An entry found on a list that is not its own is moved to its own list when that list is looked at, so a raised timeout is honoured. (In the sorted list such an entry stays where it was and, while it is at the head, holds back the expiry of everything behind it.)d_timer) uses the wheel. The delete-delay timer, whose entries all share the same delay, keeps the list.Unit tests (
modules/dialog/test/test.c, run withmake test-dialog):All 20 dialog unit tests pass. Under load (4.0.3 with this change, same setup as in #4458): 6.4 CPUs at 1,000 calls/s instead of 14.2, normal setup times, and dialogs end at the same moments (60 s limit, 32 s ACK wait, normal calls).
Compatibility
No change to the script interface, the MI output or the DB/replication data: timeouts are stored and replicated as before. The wheel costs 16384 list heads in shared memory (384 KB on 64-bit). The expiry granularity stays one second, as before.
Closing issues
closes #4458