Sticky Sessions: Tolerate ClickHouse Session ID Mechanism #117
Conversation
| @@ -99,6 +98,11 @@ func (rp *reverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) { | |||
| ReadCloser: req.Body, | |||
| } | |||
|
|
|||
| // publish session_id if needed | |||
| if s.sessionId != "" { | |||
| rw.Header().Set("X-ClickHouse-Server-Session-Id", s.sessionId) | |||
There was a problem hiding this comment.
Quick check didn't show any results for "X-ClickHouse-Server-Session-Id" header in CH docs. Could you pls add a comment what exactly this header is used for?
There was a problem hiding this comment.
I added this header just to make sure ChProxy acknowledged and processed correctly the value was set to session_id
There was a problem hiding this comment.
I was confused while reading it. I think, it requires a proper comment to explain your intention.
There was a problem hiding this comment.
@hagen1778 treat this header as ECHO Service, you pass the value of session_id and you'll want to assure that that value was accepted by server-side correctly.
There was a problem hiding this comment.
@hagen1778 let's merge it already and let's enable GitHub actions to release versions properly for multiple platforms, what do u think?
main_test.go
Outdated
| "http POST request with session id", | ||
| "testdata/http-session-id.yml", | ||
| func(t *testing.T) { | ||
| req, err := http.NewRequest("POST", "http://127.0.0.1:9090/?query_id=45395792-a432-4b92-8cc9-536c14e1e1a9&extremes=0&session_id=default-session-id233", bytes.NewBufferString("SELECT * FROM system.numbers LIMIT 10")) |
There was a problem hiding this comment.
nit: would be nice to keep the line length consistent with the rest of the code.
|
|
||
| h := c.getHost() |
There was a problem hiding this comment.
should we use if-else here to avoid unnecessary counter's increase on line 58 when session_id isn't empty?
| @@ -99,6 +98,11 @@ func (rp *reverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) { | |||
| ReadCloser: req.Body, | |||
| } | |||
|
|
|||
| // publish session_id if needed | |||
| if s.sessionId != "" { | |||
| rw.Header().Set("X-ClickHouse-Server-Session-Id", s.sessionId) | |||
There was a problem hiding this comment.
I was confused while reading it. I think, it requires a proper comment to explain your intention.
…ue to Datadog lib)
|
Thanks for contribution! |
A proposed naive approach to be able to stick to user session across multiple queries:
Use Case the PR Should be solved :
Query #1:
CREATE TEMPORARY TABLE if not exists analyse AS SELECT * from system.numbers limit 100Query #2:
SELECT count(*) as total from analyseFor each query same session-id should be passed.
Please review