Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: batched and chunked insertion and deletion of relation tuples and UUID mappings #1631

Merged
merged 1 commit into from
Dec 5, 2024
Merged

feat: batched and chunked insertion and deletion of relation tuples and UUID mappings #1631

merged 1 commit into from
Dec 5, 2024

Conversation

alnr
Copy link
Collaborator

@alnr alnr commented Nov 18, 2024

No description provided.

@alnr alnr self-assigned this Nov 18, 2024
@alnr alnr requested review from zepatrik and hperl as code owners November 18, 2024 17:28
@alnr alnr force-pushed the alnr/batch-db-operations branch 4 times, most recently from e7ea7e0 to b95a6cf Compare November 18, 2024 18:29
@alnr alnr changed the title feat: batch deletion of relation tuples feat: batch insertion and deletion of relation tuples Nov 18, 2024
aeneasr
aeneasr requested changes Nov 26, 2024
View reviewed changes
Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good but I suggest we add tests that delete 500 tuples and create 500 tuples because my gut feeling says that we'll run into issues.

internal/persistence/sql/relationtuples.go Show resolved Hide resolved
internal/persistence/sql/relationtuples.go Outdated Show resolved Hide resolved
internal/persistence/sql/relationtuples.go Show resolved Hide resolved
internal/persistence/sql/relationtuples.go Show resolved Hide resolved
internal/persistence/sql/query_test.go Show resolved Hide resolved
@alnr alnr force-pushed the alnr/batch-db-operations branch from 584439d to facb8a5 Compare November 29, 2024 16:24
@alnr alnr changed the title feat: batch insertion and deletion of relation tuples feat: batch and chunked insertion and deletion of relation tuples Nov 29, 2024
@alnr alnr requested a review from aeneasr November 29, 2024 16:29
@alnr alnr changed the title feat: batch and chunked insertion and deletion of relation tuples feat: batches and chunked insertion and deletion of relation tuples Nov 29, 2024
@alnr alnr changed the title feat: batches and chunked insertion and deletion of relation tuples feat: batched and chunked insertion and deletion of relation tuples Nov 29, 2024
zepatrik
zepatrik requested changes Dec 2, 2024
View reviewed changes
internal/persistence/sql/relationtuples.go Outdated Show resolved Hide resolved
internal/persistence/sql/relationtuples.go Show resolved Hide resolved
internal/persistence/sql/relationtuples.go Show resolved Hide resolved
internal/e2e/transaction_cases_test.go Outdated Show resolved Hide resolved
@alnr alnr requested a review from zepatrik December 2, 2024 12:26
zepatrik
zepatrik approved these changes Dec 2, 2024
View reviewed changes
Copy link
Member

@zepatrik zepatrik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great 🎉

@alnr alnr force-pushed the alnr/batch-db-operations branch from a5fd8cd to 0949de1 Compare December 4, 2024 21:37
@alnr
Copy link
Collaborator Author

alnr commented Dec 4, 2024

Alright after being led down some rabbit holes following red herrings (tired of animals yet?) I fixed the test and then marked it as slow.

But while investigating I also cleaned up some deprecated and arguably incorrect uses of the Go gRPC API and fixed a race condition. Didn't touch the non-test code.

Should be good to merge.

@alnr alnr changed the title feat: batched and chunked insertion and deletion of relation tuples feat: batched and chunked insertion and deletion of relation tuples and UUID mappings Dec 4, 2024
@alnr alnr enabled auto-merge December 5, 2024 14:14
@alnr alnr added this pull request to the merge queue Dec 5, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 5, 2024
@aeneasr aeneasr added this pull request to the merge queue Dec 5, 2024
Merged via the queue into master with commit c01b9c3 Dec 5, 2024
24 checks passed
@aeneasr aeneasr deleted the alnr/batch-db-operations branch December 5, 2024 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonas-jonas jonas-jonas jonas-jonas approved these changes

@zepatrik zepatrik zepatrik approved these changes

@aeneasr aeneasr aeneasr approved these changes

@hperl hperl Awaiting requested review from hperl hperl is a code owner

Assignees

@alnr alnr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alnr @aeneasr @zepatrik @jonas-jonas