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: implement keepalive socket context #1635

Closed
wants to merge 1 commit into from

Conversation

bjne
Copy link

@bjne bjne commented Jan 29, 2020

A new option is added to the tcp.connect options_table:

ctx = boolean, where a "true" value would change the value returned
from connect from current ok, err to:

local socket_ctx, reused = sock:connect('127.0.0.1', 5432, {ctx = true})
if not socket_ctx then
    return nil, reused
end

if reused == 0 then
    setmetatable(socket_ctx, some_mt)
end

socket_ctx.sock = sock

socket_ctx:execute("select $1, $2", 1, 2)

if reused < 100 then
    sock:setkeepalive()
end

I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.

A new option is added to the tcp.connect options_table:

ctx = boolean, where a "true" value would change the value returned
from connect from current ok, err to:

local socket_ctx, reused = sock:connect('127.0.0.1', 5432, {ctx = true})
if not socket_ctx then
    return nil, reused
end

if reused == 0 then
    setmetatable(socket_ctx, some_mt)
end

socket_ctx.sock = sock

socket_ctx:execute("select $1, $2", 1, 2)

if reused < 100 then
    sock:setkeepalive()
end
@bjne
Copy link
Author

bjne commented Jan 29, 2020

this is the basic idea for usage:

local pg = {
        login = function() return true end,
	prepare = function(self, query)
		local portal = (self.nportals or 0) + 1

		local ok, err = ngx.say("prepare ", portal, " ", query)
		if not ok then
			return nil, err
		end

		self.nportals, self[query] = portal, portal

		return portal
	end,
	execute = function(self, query, ...)
		if not query then
			return nil, "no query defined"
		end

		local portal, err = self[query]

		if not portal then
			portal, err = self:prepare(query)
			if not portal then
				return nil, err
			end
		end

		ngx.say("execute ", portal, " ", table.concat({...}, ", "))
	end
}

local pg_mt = { __index = pg }

local conn_opts = { ctx = true }

return {
	go = function()
		local query = ngx.unescape_uri(ngx.var.arg_query)
		if not query then
			return nil, "query must be spesified"
		end

		local sock = ngx.socket.tcp()
		local sock_ctx, reused = sock:connect('127.0.0.1', 1234, conn_opts)
		if not sock_ctx then
			return nil, reused
		end

		if reused == 0 then
			setmetatable(sock_ctx, pg_mt)
                        if not sock_ctx:login() then
                            return nil, "login failed"
                        end
		end

		sock_ctx.sock = sock

		sock_ctx:execute(query,  reused, sock_ctx.nportals)

		sock:setkeepalive()
	end
}

@spacewander
Copy link
Member

Three questions:

  1. I am not sure if it's a good idea to change the return values from ok, err to ctx, reused|err.
  2. Maybe we need a new API to create ctx on demand, just like ngx.ctx.
  3. More test cases, please.

@thibaultcha, what is your opinion?

@mergify
Copy link

mergify bot commented Oct 10, 2020

This pull request is now in conflict :(

@mergify
Copy link

mergify bot commented Mar 20, 2023

This pull request is now in conflict :(

@mergify mergify bot added the conflict label Mar 20, 2023
@mergify mergify bot removed the conflict label May 10, 2023
@mergify
Copy link

mergify bot commented May 10, 2023

This pull request is now in conflict :(

@mergify mergify bot added the conflict label May 10, 2023
@mergify mergify bot removed the conflict label Sep 23, 2023
@mergify
Copy link

mergify bot commented Sep 23, 2023

This pull request is now in conflict :(

@mergify mergify bot added the conflict label Sep 23, 2023
@mergify mergify bot removed the conflict label Mar 6, 2024
Copy link

mergify bot commented Mar 6, 2024

This pull request is now in conflict :(

@mergify mergify bot added the conflict label Mar 6, 2024
@mergify mergify bot removed the conflict label May 27, 2024
Copy link

mergify bot commented May 27, 2024

This pull request is now in conflict :(

@mergify mergify bot added the conflict label May 27, 2024
@zhuizhuhaomeng
Copy link
Contributor

You should use the sock:getreusedtimes() API instead of changing the return values of the API.

@mergify mergify bot removed the conflict label Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants