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.

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

[no-release-notes] go: sqle/cluster: commithook: Create a sql session and make lifecycle callbacks when accessing srcDB. #8816

Merged
merged 3 commits into from
Feb 11, 2025

Conversation

reltuk
Copy link
Contributor

@reltuk reltuk commented Feb 5, 2025

As part of GC safepoint establishment, we need lifecycle callbacks around all the places we access the database. This PR adds those callbacks to the cluster replication commithook.

Lifecycle on registering the commit hooks on the database versus creating the SqlEngine instance is a little wonky. Concretely, we register the hooks before we create the SqlEngine, but we we change things so that we start running them after the SqlEngine is created and can supply the appropriate factory for the sql.Context.

reltuk and others added 2 commits February 4, 2025 16:59
… and make lifecycle callbacks when accessing srcDB.

As part of GC safepoint establishment, we need lifecycle callbacks
around all the places we access the database. This PR adds those
callbacks to the cluster replication commithook.

Lifecycle on registering the commit hooks on the database versus
creating the SqlEngine instance is a little wonky. Concretely,
we register the hooks before we create the SqlEngine, but we
we change things so that we start running them after the SqlEngine
is created and can supply the appropriate factory for the
sql.Context.
@coffeegoddd
Copy link
Contributor

@reltuk DOLT

comparing_percentages
100.000000 to 100.000000
version result total
dce03de ok 5937457
version total_tests
dce03de 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@coffeegoddd DOLT

comparing_percentages
100.000000 to 100.000000
version result total
7f3dc83 ok 5937457
version total_tests
7f3dc83 5937457
correctness_percentage
100.0

@reltuk reltuk requested a review from max-hoffman February 5, 2025 17:36
@coffeegoddd
Copy link
Contributor

@reltuk DOLT

comparing_percentages
100.000000 to 100.000000
version result total
b8a6bc7 ok 5937457
version total_tests
b8a6bc7 5937457
correctness_percentage
100.0

Copy link
Contributor

@max-hoffman max-hoffman left a comment

Choose a reason for hiding this comment

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

LGTM, testing whether the callbacks are functional in the way they are supposed to be would be nice

@reltuk reltuk merged commit ac6d4d4 into main Feb 11, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants