Skip to content

Conversation

tzssangglass
Copy link
Member

@tzssangglass tzssangglass commented Feb 23, 2024

@tzssangglass tzssangglass marked this pull request as ready for review February 26, 2024 02:34
@tzssangglass
Copy link
Member Author

tzssangglass commented Feb 26, 2024

apply this feat in Kong maybe like:

diff --git a/kong/init.lua b/kong/init.lua
index d37a08325..667b99e30 100644
--- a/kong/init.lua
+++ b/kong/init.lua
@@ -95,6 +95,7 @@ local wasm = require "kong.runloop.wasm"
 local reports = require "kong.reports"
 local pl_file = require "pl.file"
 local req_dyn_hook = require "kong.dynamic_hook"
+local peer_conn = require("resty.kong.peer_conn")
 
 
 local kong             = kong
@@ -1278,6 +1279,7 @@ function Kong.balancer()
     -- record failure data
     local previous_try = tries[try_count - 1]
     previous_try.state, previous_try.code = get_last_failure()
+    previous_try.cached = peer_conn.get_last_peer_connection_cached()
 
     -- Report HTTP status for health checks
     local balancer_instance = balancer_data.balancer
@@ -1330,6 +1332,8 @@ function Kong.balancer()
     if retries > 0 then
       set_more_tries(retries)
     end
+
+    tries[try_count].cached = 0
   end

adding a "cached" field in the tries structure of the log to indicate that the connection used for this retry was obtained from the connection pool.

looks like

"tries": [
        {
            "ip": "10.42.8.161",
            "state": "failed",
            "port": 443,
            "code": 502,
            "balancer_start": 1701368939054,
			"cached": 1
            "balancer_latency": 0
        }

@chronolaw
Copy link
Contributor

�Does It only work in http subsystem? Should we support it in stream subsystem?

Another style suggestion: we should always have two blank lines to separate function and other code block.

@tzssangglass
Copy link
Member Author

Should we support it in stream subsystem?

I think we don't need to support in stream subsystem, because it is a patch of ngx_http_lua_balancer, not ngx_stream_lua_balancer.

@tzssangglass
Copy link
Member Author

ping @dndx , PTAL

Copy link
Member

@dndx dndx left a comment

Choose a reason for hiding this comment

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

Could you write a README doc for this new feature? It is hard to understand what it does without one.

@tzssangglass
Copy link
Member Author

Could you write a README doc for this new feature? It is hard to understand what it does without one.

add

@tzssangglass tzssangglass requested a review from dndx March 5, 2024 08:38
@tzssangglass tzssangglass requested a review from dndx March 14, 2024 06:15
Signed-off-by: tzssangglass <[email protected]>
Signed-off-by: tzssangglass <[email protected]>
Signed-off-by: tzssangglass <[email protected]>
Signed-off-by: tzssangglass <[email protected]>
Signed-off-by: tzssangglass <[email protected]>
Signed-off-by: tzssangglass <[email protected]>
Signed-off-by: tzssangglass <[email protected]>
Signed-off-by: tzssangglass <[email protected]>
Signed-off-by: tzssangglass <[email protected]>
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass tzssangglass requested a review from dndx March 15, 2024 07:50
Signed-off-by: tzssangglass <[email protected]>
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass tzssangglass requested a review from dndx March 18, 2024 03:39
@dndx dndx merged commit d1f8aaf into master Mar 18, 2024
@dndx dndx deleted the conn_cache branch March 18, 2024 06:28
@dndx
Copy link
Member

dndx commented Mar 18, 2024

@tzssangglass very nice documentation, I merged your PR

tzssangglass added a commit that referenced this pull request Mar 14, 2025
…cached` (#82)"

This reverts commit d1f8aaf.

Signed-off-by: tzssangglass <[email protected]>
samugi pushed a commit that referenced this pull request Mar 14, 2025
…cached` (#82)"

This reverts commit d1f8aaf.

Signed-off-by: tzssangglass <[email protected]>
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.

4 participants