KV: don't retrieve values for write ops #873

Merged
lx merged 3 commits from marvinj97/garage:perf/kv/insert-no-return into main 2024-09-10 09:06:29 +00:00
Contributor

As described in #851, return values in KV write ops are never used, so it is unnecessarily performing read ops (possibly with disk I/O) in the write hot path. But I haven't benchmarked the change in performance.

SQLite could probably use INSERT ... ON CONFLICT(REPLACE)

As described in https://git.deuxfleurs.fr/Deuxfleurs/garage/issues/851, return values in KV write ops are never used, so it is unnecessarily performing read ops (possibly with disk I/O) in the write hot path. But I haven't benchmarked the change in performance. SQLite could probably use INSERT ... ON CONFLICT(REPLACE)
marvinj97 added 1 commit 2024-09-04 16:50:45 +00:00
perf(kv): dont retrieve values for write ops
Some checks failed
ci/woodpecker/pr/debug Pipeline failed
74363c9060
see #851
marvinj97 added 1 commit 2024-09-04 16:52:14 +00:00
dont assert deletion count in sqlite KV adapter
Some checks failed
ci/woodpecker/pr/debug Pipeline failed
eb416a02fb
marvinj97 added 1 commit 2024-09-04 17:24:39 +00:00
test: fix db tests
All checks were successful
ci/woodpecker/pr/debug Pipeline was successful
8062ec7b4b
Contributor

Darn, you beat me to it ^^ That does feel like a good improvement!

For reference, here is a little PutObject benchmark for this change.

Before (as seen in #851) :

image

With this change :

image

There does appear to be a performance benefit: the second benchmark stays above 500#/s for longer.

I have got the diff for the INSERT OR REPLACE in SQLite but I don't really have benchmark data for it, so I can't say how that would perform.

JKR

Darn, you beat me to it ^^ That does feel like a good improvement! For reference, here is a little PutObject benchmark for this change. Before (as seen in #851) : ![image](/attachments/3d25cce4-4c9f-419a-a44f-b6fc68422432) With this change : ![image](/attachments/9b851d80-5188-45f6-ab07-6ce2da1c2894) There does appear to be a performance benefit: the second benchmark stays above 500#/s for longer. I have got the diff for the `INSERT OR REPLACE` in SQLite but I don't really have benchmark data for it, so I can't say how that would perform. JKR
Author
Contributor

@withings The benchmark images are broken unfortunately

@withings ~~The benchmark images are broken unfortunately~~
lx approved these changes 2024-09-10 09:06:23 +00:00
lx left a comment
Owner

LGTM

LGTM
lx merged commit 586957b4b7 into main 2024-09-10 09:06:29 +00:00
Sign in to join this conversation.
No reviewers
lx
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Deuxfleurs/garage#873
No description provided.