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(cat-gateway): Purge volatile data after immutable roll forward #1188

Draft
wants to merge 45 commits into
base: main
Choose a base branch
from
Draft

feat(cat-gateway): Purge volatile data after immutable roll forward #1188

wants to merge 45 commits into from

Conversation

saibatizoku
Copy link
Contributor

@saibatizoku saibatizoku commented Nov 11, 2024 •
edited
Loading

Description

Adds queries and logic for purgin volatile DB data.

Related Issue(s)

Closes #944

Description of Changes

Provide a clear and concise description of what the pull request changes.

Breaking Changes

Describe any breaking changes and the impact.

Screenshots

If applicable, add screenshots to help explain your changes.

Related Pull Requests

If applicable, list any related pull requests.

e.g., #1, #1

Please confirm the following checks

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

@saibatizoku saibatizoku added the draft Draft label Nov 11, 2024
@saibatizoku saibatizoku added this to the M4: Voting & Delegation milestone Nov 11, 2024
@saibatizoku saibatizoku self-assigned this Nov 11, 2024
@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 291/291}$ | ${\color{red}Fail: 0/291}$ |

@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 291/291}$ | ${\color{red}Fail: 0/291}$ |

@saibatizoku saibatizoku force-pushed the feat/purge-volatile-data-after-roll-forward branch from f5e7329 to a8bbb04 Compare November 12, 2024 15:06
@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 291/291}$ | ${\color{red}Fail: 0/291}$ |

@saibatizoku saibatizoku force-pushed the feat/purge-volatile-data-after-roll-forward branch from a8bbb04 to be5e37c Compare November 12, 2024 15:51
@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 320/320}$ | ${\color{red}Fail: 0/320}$ |

@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 291/291}$ | ${\color{red}Fail: 0/291}$ |

@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 291/291}$ | ${\color{red}Fail: 0/291}$ |

Mr-Leshiy
Mr-Leshiy reviewed Nov 13, 2024
View reviewed changes
catalyst-gateway/bin/src/db/index/queries/purge/mod.rs Outdated

/// All prepared DELETE query statements (purge DB table rows).
#[derive(strum_macros::Display)]
#[allow(dead_code)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it needed here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's temporary, I'm incorporating functionality, so this is WIP.

catalyst-gateway/bin/src/db/index/queries/purge/mod.rs Outdated

/// All prepared SELECT query statements (primary keys from table).
#[derive(strum_macros::Display)]
#[allow(dead_code)]
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same

@saibatizoku saibatizoku force-pushed the feat/purge-volatile-data-after-roll-forward branch from 447e1db to 85b532f Compare November 13, 2024 13:27
@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 298/298}$ | ${\color{red}Fail: 0/298}$ |

@saibatizoku saibatizoku force-pushed the feat/purge-volatile-data-after-roll-forward branch from 85b532f to 6e2ae43 Compare November 14, 2024 02:30
@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 298/298}$ | ${\color{red}Fail: 0/298}$ |

@saibatizoku saibatizoku force-pushed the feat/purge-volatile-data-after-roll-forward branch from 6e2ae43 to 6564784 Compare November 14, 2024 19:20
Mr-Leshiy
Mr-Leshiy previously approved these changes Nov 19, 2024
View reviewed changes
Copy link
Contributor

@Mr-Leshiy Mr-Leshiy left a comment

Choose a reason for hiding this comment

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

LGTM

@saibatizoku saibatizoku force-pushed the feat/purge-volatile-data-after-roll-forward branch from 6564784 to 86b837d Compare November 21, 2024 15:06
@saibatizoku saibatizoku force-pushed the feat/purge-volatile-data-after-roll-forward branch from e238a68 to c0056ff Compare December 2, 2024 13:41
@saibatizoku saibatizoku force-pushed the feat/purge-volatile-data-after-roll-forward branch from c0056ff to 84afb16 Compare December 4, 2024 20:14
@saibatizoku saibatizoku force-pushed the feat/purge-volatile-data-after-roll-forward branch from 10a1a00 to 7b170e0 Compare December 5, 2024 13:27
@saibatizoku saibatizoku added do not merge yet PR is not ready to be merged yet review me PR is ready for review labels Dec 10, 2024
@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 334/334}$ | ${\color{red}Fail: 0/334}$ |

Mr-Leshiy
Mr-Leshiy reviewed Dec 11, 2024
View reviewed changes
catalyst-gateway/bin/src/db/index/block/roll_forward.rs Outdated
};

/// Purge Data from Live Index
#[allow(unused_variables)]
Copy link
Contributor

Choose a reason for hiding this comment

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

seems its already redundant

Suggested change
#[allow(unused_variables)]

catalyst-gateway/bin/src/db/index/block/roll_forward.rs Outdated
}

/// Purge data from `txi_by_hash`.
#[allow(clippy::unused_async)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#[allow(clippy::unused_async)]

catalyst-gateway/bin/src/db/index/block/roll_forward.rs Outdated
}

/// Purge data from `unstaked_txo_ada`.
#[allow(clippy::unused_async)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#[allow(clippy::unused_async)]

Mr-Leshiy
Mr-Leshiy previously approved these changes Dec 11, 2024
View reviewed changes
Copy link
Contributor

@Mr-Leshiy Mr-Leshiy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Despite small comments LGTM

@cong-or
Copy link
Contributor

cong-or commented Dec 11, 2024

dont merge this yet

@stevenj stevenj marked this pull request as draft December 11, 2024 14:21
@stevenj
Copy link
Collaborator

stevenj commented Dec 11, 2024

Just converted to draft to prevent it merging pre-maturely.
Not because its not ready for final review.

@stevenj stevenj removed the draft Draft label Dec 11, 2024
@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 334/334}$ | ${\color{red}Fail: 0/334}$ |

@saibatizoku saibatizoku force-pushed the feat/purge-volatile-data-after-roll-forward branch from e45b4d7 to 3e0043b Compare December 19, 2024 04:39
@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 334/334}$ | ${\color{red}Fail: 0/334}$ |

@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 334/334}$ | ${\color{red}Fail: 0/334}$ |

@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 334/334}$ | ${\color{red}Fail: 0/334}$ |

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mr-Leshiy Mr-Leshiy Mr-Leshiy left review comments

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

@cong-or cong-or Awaiting requested review from cong-or cong-or is a code owner

@minikin minikin Awaiting requested review from minikin

@bkioshn bkioshn Awaiting requested review from bkioshn bkioshn will be requested when the pull request is marked ready for review bkioshn is a code owner

@stanislav-tkach stanislav-tkach Awaiting requested review from stanislav-tkach stanislav-tkach will be requested when the pull request is marked ready for review stanislav-tkach is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@saibatizoku saibatizoku

Labels
do not merge yet PR is not ready to be merged yet review me PR is ready for review
Projects
Catalyst
Status: 👀 In review
Milestone
M4: Voting & Delegation
4 participants
@saibatizoku @cong-or @stevenj @Mr-Leshiy