Improve SQL query performance and metrics#668
Merged
Conversation
brandond
force-pushed
the
reorder-join
branch
4 times, most recently
from
May 15, 2026 09:32
d825e40 to
071b79d
Compare
brandond
force-pushed
the
reorder-join
branch
14 times, most recently
from
May 19, 2026 00:01
c270942 to
f8b5ed5
Compare
brandond
force-pushed
the
reorder-join
branch
6 times, most recently
from
May 21, 2026 18:24
2267155 to
4cc28ed
Compare
manuelbuil
reviewed
May 22, 2026
brandond
force-pushed
the
reorder-join
branch
3 times, most recently
from
May 26, 2026 21:48
7a1beff to
f41ddd5
Compare
* Reorder filter expressions to prevent use of temp tables. * Rework postgres queries to better align with other engines. There are still a few that are unique. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Adds per-query metrics buckets, which allows better determining what queries are slow, without leaking value bytes into logs. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
PrevKV should only be set on update, not after delete and recreate Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
The database already has the previous value in the row corresponding to prev_revision, we can just use that to select it in. Should cut down network throughput for updates and deletes by 2/3 since we were getting the old value AND sending it back, along with the new value. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
split debug+trace to stdout; add client IP to GRPC logs Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Caches successful lookups with a short TTL to avoid DNS-induced failures when dialing new connections. When kine runs with a too-small SQL connection pool, DNS-related delays were observed to be a significant cause of slow queries. Requires converting db.Open(driverName, dsn) calls to db.OpenDB(connector) so that we can explicitly configure the driver to use our dialer. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
List/Count now always return CurrentRev, we don't need to repeat the call Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
If the goroutine was preempted between when the row finished inserting and when the row id was stored into currentRev, currentRev may have already moved forward, and the unconditional store of the row id that was actually inserted some time ago actually rolled back currentRev until the next poll loop completed. This was the cause of bogus `required revision is a future revision` errors returned to clients. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Changes
required revision is a future revisionerrors.Unfortunately is is not possible to eliminate the filesort on name, although the query cost is still reduced by half due to elimination of the inner join.
Linked Issues:
Additional Notes
ORDER BY nameclause into the subquery (to eliminate the filesort) works some of the time, but the order of rows across an inner join is unspecified so it cannot be relied upon. In practice it does seem to work most of the time, but not when aLIMITis in effect as the engine will limit the number of rows produced by the subquery so that later revisions are not seen, producing incorrect results.SELECT ... FROM kine WHERE id IN (SELECT MAX(id) FROM kine WHERE name LIKE ? ESCAPE '^' AND name >= ? GROUP BY name)is less expensive when the list of IDs is large, but more expensive when the list of IDs is small - and TERRIBLE when only a single ID is matched.SELECT ... FROM kine INNER JOIN (SELECT MAX(id) AS id FROM kine WHERE name LIKE ? ESCAPE '^' AND name >= ? GROUP BY name) AS mkv USING (id)is slightly more expensive when the list of IDs is large, but less expensive when the list is short.This PR also enhances slow SQL warnings to show a redacted version of the query with args filled:
Performance Comparison
CI runs a very lightweight load test that just churns configmaps; this shows a roughly 20% improvement in average request latencies across all operation types. CPU and memory utilization are not measured.
These charts capture two runs - the first segment is Kine v0.15.0, the second is from this PR. Note in particular the drop in connection rate, query latency, and network throughput.
