Delete Query #175
Merged
Delete Query #175
Conversation
This file contains 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
Benchmark ResultCurrent status |
louiseschmidtgen
force-pushed
the
KU-1229/delete-query
branch
from
9e2fb9a
to
c077647
Compare
louiseschmidtgen
force-pushed
the
KU-1229/delete-query
branch
from
c077647
to
12db8b3
Compare
marco6
approved these changes
Sep 25, 2024
There was a problem hiding this comment.
From the performance POV, the analysis follows the very same pattern as #170 where we see an increase in write time, but we can keep the pace as shown in sec/op:
Delete/dqlite/1-workers-4 1.391m ± 5% 1.150m ± 5% -17.30% (p=0.001 n=7)
Delete/dqlite/2-workers-4 1.244m ± 2% 1.027m ± 3% -17.45% (p=0.001 n=7)
Delete/dqlite/4-workers-4 1.241m ± 4% 1.042m ± 7% -16.04% (p=0.001 n=7)
Delete/dqlite/8-workers-4 1.211m ± 16% 1.015m ± 11% -16.22% (p=0.001 n=7)
Delete/dqlite/16-workers-4 1.235m ± 5% 1.082m ± 7% -12.34% (p=0.001 n=7)
There is indeed some increase in pages read, but as it can be seen, it is very stable even with more workers, so I'm really not worried:
Delete/dqlite/1-workers-4 24.00 ± 1% 15.31 ± 1% -36.21% (p=0.001 n=7)
Delete/dqlite/2-workers-4 12.83 ± 4% 13.25 ± 0% +3.27% (p=0.001 n=7)
Delete/dqlite/4-workers-4 10.62 ± 1% 13.22 ± 1% +24.48% (p=0.001 n=7)
Delete/dqlite/8-workers-4 10.96 ± 1% 13.21 ± 1% +20.53% (p=0.001 n=7)
Delete/dqlite/16-workers-4 11.02 ± 1% 13.23 ± 1% +20.05% (p=0.001 n=7)
Also, this pattern is in common with what we had with the Update query.
Great work Louise!
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.
Description
This PR moves the logic for deletion into its own query in generic.
Query logic for deletion
Happy path:
Edge cases:
The specified revision does not exist in the database.
The specified revision is the latest and is already deleted.
The specified revision is not the latest revision.
Query
This is the delete query:
Optimization for deletion
EXPLAIN QUERY PLAN:
Potential optimization with an additional index on deleted.
Removing unconditional delete
As of Kubernetes v1.15 we do conditional deletes instead of unconditional deletes. A conditional delete requires the revision of the key to be deleted to match the revision provided in the delete request whereas an unconditional delete does not check for the revision and just deletes what it finds. K8s-dqlite is not used by deployments on v1.14 Kubernetes and can therefore be removed safely.