-
Notifications
You must be signed in to change notification settings - Fork 7
Description
Summary
The lua-nginx-module fails to build with FreeNginx due to breaking changes in request time tracking introduced in commit 3329aa9d66a78d147a3a5186004f5122bfa9277a.
Root Cause Analysis
Specific Commit: 3329aa9
This commit introduced significant changes to time tracking in FreeNginx:
Key Changes:
-
Removed fields from
ngx_http_request_t:start_sec(time_t)start_msec(ngx_msec_t)
-
Added new field:
start_time(ngx_msec_t) - using monotonic time viangx_current_msec
-
Changed time calculation logic:
- Old method:
(tp->sec - r->start_sec) * 1000 + (tp->msec - r->start_msec) - New method:
ngx_current_msec - r->start_time
- Old method:
Impact on lua-nginx-module:
The lua-nginx-module likely contains code that directly accesses r->start_sec and r->start_msec fields, which no longer exist in FreeNginx. This causes compilation failures when building the module against FreeNginx.
Environment
- FreeNginx version: Post-commit 3329aa9 (latest)
- lua-nginx-module version: Latest from OpenResty
- Related failure: https://github.com/vozlt/nginx-module-vts/actions/runs/16381462851/job/46293696855?pr=326
Problem Description
When attempting to build lua-nginx-module with FreeNginx, the build fails because:
- The module tries to access
r->start_secandr->start_msec - These fields were removed in the monotonic time tracking changes
- No compatibility layer exists for modules using the old time tracking API
Expected vs Actual Behavior
- Expected: lua-nginx-module builds successfully with FreeNginx (as FreeNginx aims to be drop-in compatible)
- Actual: Build fails due to missing struct members
Suggested Solutions
Option 1: Compatibility Layer (Recommended)
Add compatibility macros or inline functions to maintain API compatibility:
#ifdef FREENGINX_VERSION
#define ngx_http_request_start_sec(r) ((r)->start_time / 1000)
#define ngx_http_request_start_msec(r) ((r)->start_time % 1000)
/* Or provide actual fields for backward compatibility */
#endifOption 2: Document Breaking Changes
- Clearly document this as a breaking change in FreeNginx release notes
- Provide migration guide for module developers
- List affected popular modules (lua-nginx-module, nginx-module-vts, etc.)
Option 3: Conditional Compilation Support
Ensure modules can detect FreeNginx version and adapt accordingly:
#ifdef FREENGINX_VERSION
// Use new start_time field
ms = ngx_current_msec - r->start_time;
#else
// Use legacy start_sec/start_msec
ms = (tp->sec - r->start_sec) * 1000 + (tp->msec - r->start_msec);
#endifImpact Assessment
This change affects any nginx module that:
- Accesses
r->start_secorr->start_msecdirectly - Performs custom request timing calculations
- Relies on the traditional nginx time tracking API
Known affected modules:
- lua-nginx-module (OpenResty)
- nginx-module-vts (and likely many others)
Request
Given that FreeNginx aims to be a drop-in replacement for nginx, could the maintainers consider:
- Adding a compatibility layer to maintain API compatibility for existing modules
- Providing clear migration documentation for module developers
- Testing against popular modules like lua-nginx-module in CI
- Coordinating with module maintainers on the transition
The goal would be to maintain FreeNginx's drop-in compatibility promise while improving the time tracking implementation.
Additional Context
- Commit purpose: "Ensures that $request_time is not affected by system time changes on platforms with monotonic time available"
- This is a valuable improvement but breaks existing module ecosystem
- Balance needed between technical improvements and ecosystem compatibility
Thank you for the excellent work on FreeNginx and for considering module compatibility!