THREESCALE-7941 - Review warning displayed in apicast logs about variables_hash_max_size & variables_hash_bucket_size #1395

ernaniaz · 2023-03-07T20:34:54Z

Increasing the limit of variables_hash_bucket_size make the warning message disapear.

This message is a warning that NGiNX display when the used variables
didn't fit the buffer. Increasing will also increase performance and
will not show this warning.

The NGiNX documenation about hashes says:

To quickly process static sets of data such as server names, map
directive’s values, MIME types, names of request header strings, nginx
uses hash tables. During the start and each re-configuration nginx
selects the minimum possible sizes of hash tables such that the bucket
size that stores keys with identical hash values does not exceed the
configured parameter (hash bucket size). The size of a table is
expressed in buckets. The adjustment is continued until the table size
exceeds the hash max size parameter. Most hashes have the corresponding
directives that allow changing these parameters, for example, for the
server names hash they are server_names_hash_max_size and
server_names_hash_bucket_size.

The hash bucket size parameter is aligned to the size that is a multiple
of the processor’s cache line size. This speeds up key search in a hash
on modern processors by reducing the number of memory accesses. If hash
bucket size is equal to one processor’s cache line size then the number
of memory accesses during the key search will be two in the worst case —
first to compute the bucket address, and second during the key search
inside the bucket. Therefore, if nginx emits the message requesting to
increase either hash max size or hash bucket size then the first
parameter should first be increased.

Reference: NGiNX: Setting up hashes

Also, to check the processor's cache line size, use:

cat /sys/devices/system/cpu/cpu*/cache/index*/coherency_line_size | head -n 1

Source: https://stackoverflow.com/questions/794632/programmatically-get-the-cache-line-size

This commit fix the Jira issue THREESCALE-7941

kevprice83

Try solving the issue with one change, if not possible then add the second change. Values printed in the logs are current values not suggested new values.

kevprice83 · 2023-03-08T01:11:11Z

gateway/bin/apicast

@@ -129,7 +129,7 @@ local function resty(code)

    if not cmd then return nil, 'could not find resty' end

-    local handle = io.popen(([[/usr/bin/env -i %q -e %q]]):format(cmd, code))
+    local handle = io.popen(([[/usr/bin/env -i %q -e %q 2>/dev/null]]):format(cmd, code))


This code change looks unrelated to this issue. I suspect it's unintended to be included in this commit and will cause a conflict with other PRs or with this one depending which gets merged first.

kevprice83 · 2023-03-08T01:39:40Z

gateway/conf/nginx.conf.liquid

@@ -43,7 +43,8 @@ http {
  tcp_nodelay        on;
  server_tokens off;

-
+  variables_hash_bucket_size 128;
+  variables_hash_max_size 1024;


There is no point in declaring variable_hash_max_size 1024; because that is already the default value. We can see that in the log output attached in the JIRA.

I think we need to just follow Nginx documentation here:

if nginx emits the message requesting to increase either hash max size or hash bucket size then the first parameter should first be increased.

variables_hash_max_size 2048;

If that fixes the issue we are good, if not then declare:

variables_hash_bucket_size 256;

At least this is how I understand what Nginx documentation is suggesting to do.

My mistake with the PR, it got the THREESCALE-7942 together, just ignore the first commit.
I added variables_hash_max_size with the default value because both configurations are used together, keep it clear what's configured, and there's no problem to specify some variable with their default value.

According to the doc:

The hash bucket size parameter is aligned to the size that is a multiple of the processor’s cache line size. This speeds up key search in a hash on modern processors by reducing the number of memory accesses

So, as I understand, the variables_hash_bucket_size is computed internally to be aligned with the processor’s cache line size, it does not have a fixed default size. We should not set it.

eguzki · 2023-03-09T11:08:48Z

I suggest that THREESCALE-7941 branch is based on master branch instead of THREESCALE-7942 branch. Not a big deal, but better to manage the changes.

I can help with the rebase stuff if you need.

eguzki · 2023-03-09T13:47:38Z

Verification

Setup env

make development
make dependencies

Minimal apicast config

cat <<EOF >config.json
{
   "services": [
      {
         "proxy": {
             "hosts": ["one"],
             "proxy_rules": [],
             "api_backend": "https://echo-api.3scale.net",
             "policy_chain": []
         }
      }
   ]
}
EOF

Run apicast

bash-4.4$ APICAST_LOG_LEVEL=warn APICAST_WORKER=1 THREESCALE_CONFIG_FILE=config.json BACKEND_ENDPOINT_OVERRIDE=http://localhost:8081 ./bin/apicast
loading production environment configuration: /opt/app-root/src/gateway/config/production.lua

The warning message is gone.

…to avoid startup warning.

eguzki · 2023-04-24T20:49:57Z

@kevprice83 waiting for your approval (since you requested changes)

kevprice83

Yes this looks better now and aligned with what the Nginx documentation recommends. Nice work!

ernaniaz requested a review from a team as a code owner March 7, 2023 20:34

kevprice83 requested changes Mar 8, 2023

View reviewed changes

ernaniaz added 2 commits April 19, 2023 12:28

THREESCALE-7941 - Set NGiNX variable variables_hash_max_size to 2048 …

236d473

…to avoid startup warning.

Updated CHANGELOG.md.

fc3794c

ernaniaz force-pushed the THREESCALE-7941 branch from b35fddc to fc3794c Compare April 19, 2023 15:31

eguzki approved these changes Apr 19, 2023

View reviewed changes

kevprice83 approved these changes May 2, 2023

View reviewed changes

kevprice83 merged commit fdf5b2e into master May 2, 2023

kevprice83 deleted the THREESCALE-7941 branch May 2, 2023 14:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

THREESCALE-7941 - Review warning displayed in apicast logs about variables_hash_max_size & variables_hash_bucket_size #1395

THREESCALE-7941 - Review warning displayed in apicast logs about variables_hash_max_size & variables_hash_bucket_size #1395

ernaniaz commented Mar 7, 2023 •

edited by eguzki

Loading

kevprice83 left a comment

kevprice83 Mar 8, 2023

kevprice83 Mar 8, 2023

ernaniaz Mar 8, 2023

eguzki Mar 9, 2023

eguzki commented Mar 9, 2023

eguzki commented Mar 9, 2023

eguzki commented Apr 24, 2023

kevprice83 left a comment

THREESCALE-7941 - Review warning displayed in apicast logs about variables_hash_max_size & variables_hash_bucket_size #1395

THREESCALE-7941 - Review warning displayed in apicast logs about variables_hash_max_size & variables_hash_bucket_size #1395

Conversation

ernaniaz commented Mar 7, 2023 • edited by eguzki Loading

kevprice83 left a comment

Choose a reason for hiding this comment

kevprice83 Mar 8, 2023

Choose a reason for hiding this comment

kevprice83 Mar 8, 2023

Choose a reason for hiding this comment

ernaniaz Mar 8, 2023

Choose a reason for hiding this comment

eguzki Mar 9, 2023

Choose a reason for hiding this comment

eguzki commented Mar 9, 2023

eguzki commented Mar 9, 2023

Verification

eguzki commented Apr 24, 2023

kevprice83 left a comment

Choose a reason for hiding this comment

ernaniaz commented Mar 7, 2023 •

edited by eguzki

Loading