CesiumGS / CesiumGS/cesium

RequestScheduler doesn't honor throttle settings

Open
#6,482 0 comments 0 reactions 0 assignees View on GitHub
priority - high
Dominant language
JavaScript
Stars
15.7k
Forks
3.9k
Avg merge
4d 6h
Merged PRs (30d)
34

Description

RequestScheduler doesn't honor throttle settings and makes it easy for one server to starve all others. Take this code for example:

```js
for (var i = 0; i < RequestScheduler.priorityHeapLength; i++) {
var promise = RequestScheduler.request(new Request({
url: 'http://test.invalid/1',
throttle: true,
throttleByServer: true,
requestFunction: function() { return when.defer(); }
}));
}
```

I would expect `promise` to be undefined for all calls after `i = 6`, since 6 is the limit per-server. Unfortunately, all of them succeed. This is because the per-server numbers used by the `request` function to ensure throttling are only updated once per frame and we're instead running into the overall `priorityHeapLength` limit. The end result here is that a 3D Tiles or other provider that schedules lots of requests in one frame (not uncommon) will then starve every other provider making requests in that same frame. The only reason our unit tests pass is because they cheat by manually calling `RequestScheduler.update` whenever they need stuff to work. This obviously doesn't fly during actual use.

The "simple" fix is to update the per-server throttle value immediately on insert, but @lilleyse is worried that doing that will affect prioritization because and more thought is needed to figure out the solution.

I know I sound like a broken record, but our `RequestScheduler` continues to do more harm than good and we should seriously consider disabling it completely until we have the time to properly review and refactor it.

CC @pjcozzi @lilleyse

Contributor guide

Open the contributing guide

Research direction

Start with RequestScheduler.request and RequestScheduler.update, then inspect the existing RequestScheduler unit tests and reproduce the provided loop. Determine how per-server throttle counts are updated during insertion and prioritization; done means the per-server limit is enforced during a frame without breaking request prioritization or existing tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
networking, performance
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.