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

Two phase commit for mobile packet verifier #700

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

maplant
Copy link
Contributor

@maplant maplant commented Jan 4, 2024

I need to figure out how to get the migration for this to work

mobile_packet_verifier/src/burner.rs Outdated Show resolved Hide resolved
mobile_packet_verifier/src/burner.rs Outdated Show resolved Hide resolved
mobile_packet_verifier/src/burner.rs Outdated Show resolved Hide resolved
mobile_packet_verifier/src/burner.rs Outdated Show resolved Hide resolved
mobile_packet_verifier/src/burner.rs Outdated Show resolved Hide resolved

ALTER TABLE data_transfer_sessions RENAME TO old_data_transfer_sessions;
ALTER TABLE data_transfer_sessions_by_row to data_transfer_sessions;
DROP TABLE data_transfer_sessions;
Copy link
Contributor

Choose a reason for hiding this comment

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

don't you mean DROP TABLE old_data_transfer_sessions?

Comment on lines +111 to +112
.bind(&session.payer)
.bind(session.uploaded_bytes)
Copy link
Contributor

Choose a reason for hiding this comment

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

you're missing the bind on num_dcs

.execute(pool)
for pending_txn in pending_txns {
let txn: Signature = pending_txn.signature.parse()?;
let mut transaction = self.pool.begin().await?;
Copy link
Contributor

Choose a reason for hiding this comment

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

i don't think you ever commit this transaction. does it need to be inside this for loop or can it be a single transaction for all of the pending_txns?

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