Skip to content

Commit

Permalink
perf(router) use request Host header in hosts reducer
Browse files Browse the repository at this point in the history
This patch is considered a performance improvements, but could also be
considered a fix (although we chose to not consider it as such for
reasons described below).

Perf
----

When no wildcard host was matched in the indexing step
(`ctx.hits.hosts`), the behavior of the reducers is to return the
complete category of Routes (categories being sets of Routes with the
same number of matching attributes).

The purpose of the reducer is to try to find a subset of Routes from
this category, so that the matchers evaluate a smaller (sometimes, much
smaller) set of Routes.

Not having the reducer fallback on the request's Host header meant that
matchers always had to evaluate all Routes in each category. This is
fine for plain hosts whose lookup cost is `O(1)`, but other matching
attributes have to be evaluated, and skipping Routes without a valid
Host header is beneficial to those scenarios.

Fix
---

This commit could re-enable the pending test in the Router's unit test
suite ("does not take precedence over a plain host").

Since a plain host was matched in the indexing step, `ctx.hits.host`
will be `nil` (wildcard host indexing is skipped). However, the reducer
would return the whole category (prior to this patch), and the category
has Routes that are **not sorted according to sub-matching rules**. Said
rules are described in the previous and subsequent commits. This would
result in Routes with wildcard hosts from this category to be evaluated
_before_ Routes with plain hosts, and matching.

Since this commit returns only a subset of the category (Routes with the
plain host header from the request), matchers will not evaluate wildcard
hosts Routes from the same category, resulting in the expected match.

**However**, this isn't an appropriate fix since we rely on indexers
(hints) instead of properly sorting categorized Routes as per the
sub-matching rules. Doing the latter could also fix other unexpected
matching priority issues, and is a step forward in being able to run the
router without indexers (for testing purposes only).

A subsequent commit will sort categorized Routes appropriately.
  • Loading branch information
thibaultcha committed Jul 3, 2019
1 parent 01b1cb8 commit e85257e
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion kong/router.lua
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ end
do
local reducers = {
[MATCH_RULES.HOST] = function(category, ctx)
return category.routes_by_hosts[ctx.hits.host]
return category.routes_by_hosts[ctx.hits.host or ctx.req_host]
end,

[MATCH_RULES.URI] = function(category, ctx)
Expand Down

0 comments on commit e85257e

Please sign in to comment.