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

feat: wallet optimize output manager db operations #4663

Merged

Conversation

hansieodendaal
Copy link
Contributor

@hansieodendaal hansieodendaal commented Sep 12, 2022

Description

Optimized output manager and contacts liveness database operations to do as much work as possible inside the SQL database instead of inside Rust code. These changes are most noticeable where a query would return a number of outputs and followed by updating each output one by one.

Motivation and Context

See #4621

How Has This Been Tested?

  • Passed unit tests
  • Passed cucumber tests
  • System-level tests

@hansieodendaal hansieodendaal changed the title [wip] feat: optimize output manager db operations [wip] feat: wallet optimize output manager db operations Sep 12, 2022
@hansieodendaal hansieodendaal force-pushed the ho_optimize_sql_queries branch 3 times, most recently from 9abc858 to c53a61f Compare September 19, 2022 11:08
Optimized output manager database operations to do as much work
as possible inside the SQL database instead of inside Rust code.
@hansieodendaal hansieodendaal changed the title [wip] feat: wallet optimize output manager db operations feat: wallet optimize output manager db operations Sep 20, 2022
Copy link
Collaborator

@stringhandler stringhandler left a comment

Choose a reason for hiding this comment

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

Nice, maybe add a transaction in a new PR

&conn,
)?;
}
update_outputs_with_tx_id_and_status_to_new_status(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
update_outputs_with_tx_id_and_status_to_new_status(
// use a transaction here
conn.transaction(|| {
update_outputs_with_tx_id_and_status_to_new_status(

&conn,
)?;
}
diesel::update(outputs::table.filter(outputs::status.eq(OutputStatus::ShortTermEncumberedToBeReceived as i32)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

also add a transaction over these two statements

@stringhandler stringhandler merged commit 25c4d99 into tari-project:development Sep 20, 2022
@hansieodendaal hansieodendaal deleted the ho_optimize_sql_queries branch September 20, 2022 07:09
stringhandler pushed a commit that referenced this pull request Sep 26, 2022
Description
---
Added  SQL transactions to the encumbering queries to make the entire operation atomic.
_(This is a further safeguard as a follow-on to #4663.)_

Motivation and Context
---
It is possible that an independent thread can change the same SQL data (outputs) that were selected and verified to be encumbered just before it is encumbered.

Fixes #4701.

How Has This Been Tested?
---
Passed unit tests
Passed cucumber tests
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