Skip to content

Commit 70380c6

Browse files
committed
Fix Lapis initialization not taking place during ngx.timer.at callbacks
Exposed by running `bundle exec minitest test/apis/v0/test_analytics.rb -n test_expected_response` by itself, the basic issue is that the database initialization needs to happen for async nginx timers, since otherwise it only happens in the pre-request callback. This cropped up after the Lapis 1.7 upgrade and the changes in 7562163. But it seems like it was related to the 1.7 upgrade, since even without the new database init approach, this problem would occur. But I think this new approach actually makes more sense, because it seems logical that the async callbacks would also need something to setup their database pool connections.
1 parent 7562163 commit 70380c6

File tree

5 files changed

+47
-18
lines changed

5 files changed

+47
-18
lines changed

src/api-umbrella/web-app/actions/v0/analytics.lua

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ local respond_to = require "api-umbrella.web-app.utils.respond_to"
1414
local stable_object_hash = require "api-umbrella.utils.stable_object_hash"
1515
local t = require("api-umbrella.web-app.utils.gettext").gettext
1616
local time = require "api-umbrella.utils.time"
17+
local timer_at = require "api-umbrella.web-app.utils.timer_at"
1718

1819
local db_statement_timeout_ms = config["web"]["analytics_v0_summary_db_timeout"] * 1000
1920

@@ -325,7 +326,7 @@ function _M.summary(self)
325326
-- users don't get a super slow response and we don't overwhelm the server
326327
-- when it's uncached).
327328
if cache:updated_at_timestamp() < ngx.now() - 60 * 60 * 6 then
328-
ngx.timer.at(0, function()
329+
timer_at(0, function()
329330
-- Ensure only one pre-seed is happening at a time (at least per
330331
-- server).
331332
interval_lock.mutex_exec("preseed_analytics_summary_cache", generate_summary)
@@ -337,7 +338,7 @@ function _M.summary(self)
337338

338339
-- Trigger analytics generation in background so if it takes longer than
339340
-- this request, it can still populate.
340-
ngx.timer.at(0, function()
341+
timer_at(0, function()
341342
-- Ensure only one pre-seed is happening at a time (at least per
342343
-- server).
343344
interval_lock.mutex_exec("initial_analytics_summary_cache", generate_summary)

src/api-umbrella/web-app/app.lua

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
local Admin = require "api-umbrella.web-app.models.admin"
22
local config = require("api-umbrella.utils.load_config")()
33
local csrf = require "api-umbrella.web-app.utils.csrf"
4-
local db = require "lapis.db"
4+
local db_init = require "api-umbrella.web-app.utils.db_init"
55
local error_messages_by_field = require "api-umbrella.web-app.utils.error_messages_by_field"
66
local escape_html = require("lapis.html").escape
77
local flash = require "api-umbrella.web-app.utils.flash"
@@ -10,7 +10,6 @@ local http_headers = require "api-umbrella.utils.http_headers"
1010
local is_empty = require "api-umbrella.utils.is_empty"
1111
local lapis = require "lapis"
1212
local lapis_config = require("lapis.config").get()
13-
local pg_utils = require "api-umbrella.utils.pg_utils"
1413
local refresh_local_active_config_cache = require("api-umbrella.web-app.stores.active_config_store").refresh_local_cache
1514
local resty_session = require "resty.session"
1615
local t = require("api-umbrella.web-app.utils.gettext").gettext
@@ -160,20 +159,7 @@ local function current_admin_from_session(self)
160159
end
161160

162161
local function before_filter(self)
163-
-- Set session variables for the database connection (always use UTC and set
164-
-- an app name for auditing).
165-
local pg = db.connect()
166-
-- I don't think this should really be possible, but if somehow lapis has
167-
-- already established a connection, then `db.connect()` will return `nil`.
168-
-- So in that case, fetch the cached connection Lapis uses internally, and
169-
-- force the full setup (even if it's a reused socket), since we don't know
170-
-- if this socket was fully setup by Lapis or not.
171-
local force_first_time_setup = false
172-
if not pg then
173-
pg = ngx.ctx.pgmoon_default
174-
force_first_time_setup = true
175-
end
176-
pg_utils.setup_connection(pg, "api-umbrella-web-app", force_first_time_setup)
162+
db_init()
177163

178164
-- Refresh cache per request if background polling is disabled.
179165
if config["router"]["active_config"]["refresh_local_cache_interval"] == 0 then

src/api-umbrella/web-app/hooks/init_preload_modules.lua

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ require "api-umbrella.web-app.utils.csrf"
119119
require "api-umbrella.web-app.utils.csv"
120120
require "api-umbrella.web-app.utils.datatables"
121121
require "api-umbrella.web-app.utils.db_escape_patches"
122+
require "api-umbrella.web-app.utils.db_init"
122123
require "api-umbrella.web-app.utils.dbify_json_nulls"
123124
require "api-umbrella.web-app.utils.error_messages_by_field"
124125
require "api-umbrella.web-app.utils.flash"
@@ -136,6 +137,7 @@ require "api-umbrella.web-app.utils.pretty_yaml_dump"
136137
require "api-umbrella.web-app.utils.require_admin"
137138
require "api-umbrella.web-app.utils.respond_to"
138139
require "api-umbrella.web-app.utils.test_env_mock_userinfo"
140+
require "api-umbrella.web-app.utils.timer_at"
139141
require "api-umbrella.web-app.utils.username_label"
140142
require "api-umbrella.web-app.utils.validation_ext"
141143
require "api-umbrella.web-app.utils.wrapped_json_params"
@@ -163,6 +165,7 @@ require "lapis.db.model"
163165
require "lapis.db.model.relations"
164166
require "lapis.features.etlua"
165167
require "lapis.html"
168+
require "lapis.nginx"
166169
require "lapis.util"
167170
require "libcidr-ffi"
168171
require "lualdap"
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
local db = require "lapis.db"
2+
local pg_utils = require "api-umbrella.utils.pg_utils"
3+
4+
-- Set session variables for the database connection that Lapis uses (always
5+
-- use UTC and set an app name for auditing). This needs to be called before
6+
-- Lapis `db.query` is called (by either Lapis requests, or also before Lapis
7+
-- timers).
8+
return function()
9+
local pg = db.connect()
10+
11+
-- I don't think this should really be possible, but if somehow lapis has
12+
-- already established a connection, then `db.connect()` will return `nil`.
13+
-- So in that case, fetch the cached connection Lapis uses internally, and
14+
-- force the full setup (even if it's a reused socket), since we don't know
15+
-- if this socket was fully setup by Lapis or not.
16+
local force_first_time_setup = true
17+
if not pg then
18+
pg = ngx.ctx.pgmoon_default
19+
force_first_time_setup = true
20+
end
21+
22+
pg_utils.setup_connection(pg, "api-umbrella-web-app", force_first_time_setup)
23+
end
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
local db_init = require "api-umbrella.web-app.utils.db_init"
2+
local timer_at = require("lapis.nginx").timer_at
3+
4+
-- For Lapis web app requests instead of using `ngx.timer.at`, use this wrapper which accomplishes 2 things:
5+
--
6+
-- 1. Uses Lapis' existing wrapper to ensure db disconnect or other after tasks
7+
-- occur:
8+
-- https://github.com/leafo/lapis/blob/v1.17.0/lapis/nginx.moon#L166-L177
9+
-- 2. Integrate our own `db_init` handler to ensure database connections inside
10+
-- the timer have the proper search_path and other settings configured.
11+
return function(delay, callback, ...)
12+
timer_at(delay, function(_premature, ...)
13+
db_init()
14+
return callback(...)
15+
end, ...)
16+
end

0 commit comments

Comments
 (0)