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

Improve pruning #4125

Merged
merged 7 commits into from
Nov 7, 2022
Merged

Improve pruning #4125

merged 7 commits into from
Nov 7, 2022

Conversation

lutter
Copy link
Collaborator

@lutter lutter commented Nov 3, 2022

The pruning code could cause long running transactions which are problematic because they cause bloat in frequently updated tables, most notably subgraph_deployment. This PR changes the pruning code to perform all operations in transactions that are time-limited (no more than 5 minutes)

Copy link
Contributor

@evaporei evaporei left a comment

Choose a reason for hiding this comment

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

It looks correct to me!

table.switch(conn, self)?;
cancel.check_cancel()?;
conn.transaction(|| table.switch(conn, self))?;
cancel.check_cancel().map_err(CancelableError::from)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why wasn't this needed before?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it was because before the closure was declared as returning an error CancelableError<StoreError> of and after the change the typechecker needed this hint to find the conversions from Canceled -> CancelableError -> StoreError

Copying of nonfinal entity versions happened in one very long transaction,
which can have terrible consequences for the rest of the system. We now
break that transaction up into multiple transactions.
No functional change, just switch to using raw strings
This makes it possible to handle tables that do not live in the same
namespace as the overall layout
That makes it possible to use the exact same structure, index names
etc. for the temporary table as for the main table, so that we don't have
to do complex renaming of indexes when swapping tables.
If we wait for the lock inside a transaction, we might cause a long-running
transaction which we need to avoid. Instead, poll for the deployment lock
before starting the actual transaction for writing changes or reverting.
The counting itself can take considerable amounts of time, and doesn't seem
to help copying of final rows all that much.
@lutter lutter merged commit 6033965 into master Nov 7, 2022
@lutter lutter deleted the lutter/prune branch November 7, 2022 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants