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

perf(db) improve kong startup time by reusing luasocket on kong.init #4178

Merged
merged 1 commit into from
Jan 9, 2019

Conversation

bungle
Copy link
Member

@bungle bungle commented Jan 9, 2019

Summary

This is the seond PR that comes from the discussion of #4171.

Without this patch the Postgres strategy creates 7 non-pooled luasocket connections to database on Kong.init(). This patch reduces that to 3.

Without this patch the Cassandra strategy creates 9 non-pooled luasocket connections to database on Kong.init(). This patch reduces that to 1.

The difference is because Postgres needs to run initialization SQL to setup the connection timezone and database schema on new connections. And for Postgres connect_migrations is the same
as connect while with Cassandra it is not. It is still a tiny improvement for Postgres as well.

@bungle bungle force-pushed the perf/db-init-kong branch from 7831717 to b963b1c Compare January 9, 2019 12:07
@bungle bungle changed the title perf(db) improve kong startup time by using same luasocket on kong.init perf(db) improve kong startup time by reusing luasocket on kong.init Jan 9, 2019
Without this patch the Postgres strategy creates 7 non-pooled
luasocket connections to database on Kong.init(). This patch
reduces that to 3.

Without this patch the Cassandra strategy creates 9 non-pooled
luasocket connections to database on Kong.init(). This patch
reduces that to 1.

The difference is because Postgres needs to run initialization
SQL to setup the connection timezone and database schema on new
connections. And for Postgres `connect_migrations` is the same
as `connect` while with Cassandra it is not. It is still a tiny
improvement for Postgres as well.
@bungle bungle force-pushed the perf/db-init-kong branch from b963b1c to 5b6bbd7 Compare January 9, 2019 12:09
if not plugins_map_semaphore then
error("failed to create plugins map semaphore: " .. err)
end

plugins_map_semaphore:post(1) -- one resource, treat this as a mutex
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious, why this change? Seems unrelated?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@p0pr0ck5 nothing, just a small cleanup as I was touching code close to it. Not a big deal to include or remove the change.

@thibaultcha thibaultcha merged commit 191c07b into master Jan 9, 2019
@thibaultcha thibaultcha deleted the perf/db-init-kong branch January 9, 2019 18:18
@hishamhm hishamhm added this to the 1.0.1 milestone Jan 10, 2019
@jeremyjpj0916
Copy link
Contributor

jeremyjpj0916 commented Jan 13, 2019

Hey folks I gave this PR a go on 1.0 GA and saw this error during startup in debug mode:

2019/01/13 01:39:13 [debug] 22#0: [lua] plugins.lua:259: load_plugin_schemas(): Loading plugin: acl
2019/01/13 01:39:13 [debug] 22#0: [lua] plugins.lua:273: load_plugin_schemas(): Loading custom plugin entity: 'acl.acls'
2019/01/13 01:39:13 [debug] 22#0: [lua] plugins.lua:259: load_plugin_schemas(): Loading plugin: kong-service-virtualization
2019/01/13 01:39:13 [debug] 22#0: [lua] plugins.lua:259: load_plugin_schemas(): Loading plugin: request-transformer
2019/01/13 01:39:13 [debug] 22#0: [lua] plugins.lua:259: load_plugin_schemas(): Loading plugin: rate-limiting
2019/01/13 01:39:13 [debug] 22#0: [lua] plugins.lua:259: load_plugin_schemas(): Loading plugin: statsd
2019/01/13 01:39:13 [debug] 22#0: [lua] plugins.lua:259: load_plugin_schemas(): Loading plugin: oauth2
2019/01/13 01:39:13 [debug] 22#0: [lua] plugins.lua:273: load_plugin_schemas(): Loading custom plugin entity: 'oauth2.oauth2_credentials'
2019/01/13 01:39:13 [debug] 22#0: [lua] plugins.lua:273: load_plugin_schemas(): Loading custom plugin entity: 'oauth2.oauth2_authorization_codes'
2019/01/13 01:39:13 [debug] 22#0: [lua] plugins.lua:273: load_plugin_schemas(): Loading custom plugin entity: 'oauth2.oauth2_tokens'
2019/01/13 01:39:15 [error] 22#0: init_by_lua error: /usr/local/share/lua/5.1/kong/db/init.lua:125: attempt to concatenate local 'err' (a nil value)
stack traceback:
	/usr/local/share/lua/5.1/kong/db/init.lua:125: in function 'prefix_err'
	/usr/local/share/lua/5.1/kong/db/init.lua:190: in function 'close'
	/usr/local/share/lua/5.1/kong/init.lua:360: in function 'init'
	init_by_lua:3: in main chunk
nginx: [error] init_by_lua error: /usr/local/share/lua/5.1/kong/db/init.lua:125: attempt to concatenate local 'err' (a nil value)
stack traceback:
	/usr/local/share/lua/5.1/kong/db/init.lua:125: in function 'prefix_err'
	/usr/local/share/lua/5.1/kong/db/init.lua:190: in function 'close'
	/usr/local/share/lua/5.1/kong/init.lua:360: in function 'init'
	init_by_lua:3: in main chunk

And init.lua line 360 was the assert db close in the newly modified snippet:

local err
  plugins_map_semaphore, err = semaphore.new(1) -- 1 = treat this as a mutex
  if not plugins_map_semaphore then
    error("failed to create plugins map semaphore: " .. err)
  end

  local _, err = build_plugins_map(db, "init")
  if err then
    error("error building initial plugins map: ", err)
  end

  assert(runloop.build_router(db, "init"))
  
  assert(db:close())
end

Just thought I would bring it to attention since I noticed this was indeed already merged if it raises any concerns.

EDIT - commenting out the assert(db:close()) seems to have "dirty" fixed it, maybe thats not the right approach for the official pr.
But HOLY COW this improves Kong 1.0 startup time for me from 2 minutes with 1k resources down to literally 3-4 seconds... What black magic did you apply here :O ???

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