fix: fold timeout into scope query#1154
Conversation
Coverage Report for CI Build 28108508086Coverage increased (+0.05%) to 78.573%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
|
It's an interesting approach, but it might beat us in case we never call Not sure if it's worth the performance gain here (1ms or less) vs a bit of a hidden timeout logic |
that's fine and covered by a test |
cache serialization of headers/jwt payload per request Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
ferhatelmas
force-pushed
the
ferhat/timeout-fold-scope
branch
from
827b5b4 to
7e10952
Compare
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
| }) | ||
|
|
||
| if (isMultigres && transaction) { | ||
| // Multigres requires SET LOCAL for statement_timeout, and role scope setup resets it. |
There was a problem hiding this comment.
@GuptaManan100 set_config('statement_timeout', ..., true) does not change effective transaction timeout. Is this worked on? Workaround is making the hot code complex, it would be desirable if we wouldn't need
There was a problem hiding this comment.
It doesn't!? I'll file a bug and fix it on Multigres, we shouldn't need to change code here. I'll get this fixed and let you know when the fix is in place.
There was a problem hiding this comment.
Nope, I have tests which work with pg/oriole but fails with Multigres. Looking forward to your fix 🙏🏻
There was a problem hiding this comment.
Verified this is indeed a bug, and I've reproduced it locally. Working on a fix now.
What kind of change does this PR introduce?
Refactor for perf
What is the current behavior?
Right now, statement timeout is sent separately which introduces extra round trip.
What is the new behavior?
Delay until the first statement (set_config) and combine them together in the same query to reduce one round trip to db.
Additional context
Additionally, make query text for scope module level, and cache headers/jwt payload for multiple queries in the same connection.