Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: new ngx_meta_lua_module (with multi-subsystems lua_shared_dict support) #76

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

thibaultcha
Copy link
Member

@thibaultcha thibaultcha commented Apr 1, 2020

This PR introduces a new NGINX core module: ngx_meta_lua_module.

This new module aims to:

  • Unify duplicated implementation between ngx_http_lua_module and ngx_stream_lua_module (I believe that many parts of both modules currently implemented via the tt2 templating system could eventually be rewritten more elegantly, simplifying lua-resty-core's codebase with regards to subsystems support, and also producing smaller binaries overall).
  • Support the long-pending work item of sharing lua_shared_dict between the http {} and stream {} subsystems (with some code cleanup along the way).

ngx_meta_lua_module offers a new nginx.conf configuration block: lua {}.

New lua {} conf block

In the below example, we specify lua_shared_dict in the new lua {} block to share the allocated shm zone between all subsystems:

lua {
    lua_shared_dict dogs 1m;
}

http {
    lua_shared_dict cats 1m;

    server {
        location /t {
            content_by_lua_block {
                ngx.shared.dogs:get("foo") -- works
                ngx.shared.cats:get("foo") -- works
             }
        }
    }
}

stream {
    server {
        content_by_lua_block {
            ngx.shared.dogs:get("foo") -- works too!
            ngx.shared.cats:get("foo") -- does not work (http-only)
        }
    }
}

Currently, lua_shared_dict is the only directive supported inside of the lua {} configuration block. Many more could follow when settings could be applied globally to all subsystems' Lua VMs (e.g. lua_package_path, lua_max_pending_timers, etc...). In some cases, it would also make sense to only allow some Lua directives to be specified from within the lua {} block, e.g. lua_sa_restart.

Current state

For the time being, this PR is opened for feedback and reviews without an estimated timeline for merging.

That said, all tests are passing for:

  • ✔️ ngx_http_lua_module
  • ✔️ ngx_stream_lua_module
  • ✔️ lua-resty-core
  • ✔️ ngx_meta_lua_module (including HUP and Valgrind modes)

See also

For this module to work, it must be compiled with updated versions of ngx_http_lua_module, ngx_stream_lua_module, and lua-resty-core. See the following branches:

The tt2 templates of this repositories have also been updated to support the ngx_meta_lua_module, and produce valid ngx_stream_lua_module/ngx_http_lua_module targets.

A list of topics requesting this feature:

TODOs

A non-exhaustive list:

  • Add CI for the new ngx_meta_lua_module (test without http and without stream subsystems too)
  • Debate the consequences of removing the public C API (which seems rather useless to me? unless it is used by someone out there, but if so what for?)
  • Open sibling PRs for other modules (as noted in the above section)

The new `util/build.sh` script and embedded test suite in this repo
warrant some updates to the `clean` target.
This replaces lua-nginx-module's ngx_http_lua_fake_shm_module for the
`t/148-fake-shm-zone.t` and `t/149-hup-fake-shm-zone.t` test suites.
@onlonely
Copy link

I need this function exactly,But not yet merged.

@stallion5632
Copy link

I need this function exactly,But not yet merged.
@onlonely
refer to https://github.com/stallion5632/apisix-nginx-module

this repository is a part of https://github.com/api7/apisix-nginx-module
To realize the #76

how to use
base openresty official 1.19.9.1 version
Follow the steps below:

cd apisix-nginx-module/patch
./patch.sh ThePathOfYourOpenRestySrcDirectory
/configure --add-module=../apisix-nginx-module/src/meta
make -j10

@geofflancaster
Copy link

Bump. Can this be merged?

@franciscopaniskaseker
Copy link

bump. same question. can be merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants