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

Add BackfillStatus and Improve StartingVersion Selection on (re)start #10

Merged
merged 10 commits into from
Oct 22, 2024

Conversation

dermanyang
Copy link
Contributor

@dermanyang dermanyang commented Oct 15, 2024

Purpose

Add new backfill_status Enum to BackfillProcessorStatus. The enum can either be InProgress or Complete. This will help the get_starting_version function decide whether or not to respect the latest checkpoint in the backfill processor status table or to just use config.starting_ver / genesis. Read more in the design

Technical Overview

Mostly the addition of logic into the starting_version.rs file. Otherwise, some convenience functions implementing ToSql and FromSql for the BackfillStatus enum.

New logic will mark BackfillStatus as Complete when lst_success_version >= backfill_end_version and as InProgress otherwise.

Testing

Added new dependency on the testing framework (!) which allows us to use a mock postgres instance so we can test the starting_version logic by manipulating the persisted state.

Added 4 test cases to lock the intended behavior:

  1. regular processor start with no checkpoint (empty DB)
  2. regular processor (re)start with checkpoint
  3. backfill processor start with a Complete checkpoint
  4. backfill processor with an InProgress checkpoint

Please note I tried to reduce the setup code repetition in the tests but couldn't fix the connection refused error within my time box 🤷‍♂️

Also tested the above scenarios manually. Works as expected.

@dermanyang dermanyang changed the title Add backfill status, improve starting ver selection. Add BackfillStatus and Improve StartingVersion Selection on (re)start Oct 17, 2024
@dermanyang dermanyang requested review from a team and rtso October 17, 2024 23:01
Copy link

@yuunlimm yuunlimm left a comment

Choose a reason for hiding this comment

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

approved with a minor comment

.await
.unwrap();

assert_eq!(starting_version, 11);
}

Choose a reason for hiding this comment

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

nit: should we consider adding a test case where the configuration has a starting_version explicitly set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added! thanks

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