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

Change: Replace loosen-follower-log-revert feature flag with Config::allow_log_reversion #1268

Conversation

drmingdrmer
Copy link
Member

@drmingdrmer drmingdrmer commented Nov 30, 2024

Changelog

Change: Replace loosen-follower-log-revert feature flag with Config::allow_log_reversion

The loosen-follower-log-revert feature flag is removed in favor of a
runtime-configurable option, Config::allow_log_reversion. This change
allows log reversion to be controlled dynamically via application
configuration rather than at compile time.

When Config::allow_log_reversion is enabled, log reversion on a
Follower is always permitted. Upon detecting a reversion, the Leader
will reset replication from the beginning instead of panicking.

Breaking Change:

Since this commit, compiling with --features loosen-follower-log-revert will now result in the following error:

error: The feature flag `loosen-follower-log-revert` is removed since version `0.10.0`.
       Use `Config::allow_log_reversion` instead.

Upgrade tip:

For applications relying on loosen-follower-log-revert:

  1. Remove this feature flag from Cargo.toml.
  2. Enable log reversion in your configuration as follows:
let config = Config {
    allow_log_reversion: Some(true),
    ..Default::default()
};

This change is Reviewable

@drmingdrmer drmingdrmer force-pushed the 167-remove-loosen-follower-log-revert branch 3 times, most recently from e11c65d to eed8af3 Compare November 30, 2024 07:33
@drmingdrmer drmingdrmer requested a review from schreter November 30, 2024 07:48
…g::allow_log_reversion`

The `loosen-follower-log-revert` feature flag is removed in favor of a
runtime-configurable option, `Config::allow_log_reversion`. This change
allows log reversion to be controlled dynamically via application
configuration rather than at compile time.

When `Config::allow_log_reversion` is enabled, log reversion on a
Follower is always permitted. Upon detecting a reversion, the Leader
will reset replication from the beginning instead of panicking.

### Breaking Change:

Since this commit, compiling with `--features
loosen-follower-log-revert` will now result in the following error:

```text
error: The feature flag `loosen-follower-log-revert` is removed since version `0.10.0`.
       Use `Config::allow_log_reversion` instead.
```

Upgrade tip:

For applications relying on `loosen-follower-log-revert`:

1. **Remove this feature flag** from `Cargo.toml`.
2. **Enable log reversion in your configuration** as follows:

```rust
let config = Config {
    allow_log_reversion: Some(true),
    ..Default::default()
};
```
Copy link
Collaborator

@schreter schreter left a comment

Choose a reason for hiding this comment

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

Reviewed 21 of 21 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @drmingdrmer)


openraft/src/progress/entry/update.rs line 35 at r1 (raw file):

    ///
    /// [`Config::allow_log_reversion`]: `crate::config::Config::allow_log_reversion`
    pub(crate) fn update_conflicting(&mut self, conflict: u64) {

I assume, this was just moved from above, so I didn't review.

Copy link
Member Author

@drmingdrmer drmingdrmer left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @drmingdrmer)


openraft/src/progress/entry/update.rs line 35 at r1 (raw file):

Previously, schreter wrote…

I assume, this was just moved from above, so I didn't review.

Yes. In order to reference EngineConfig, it is moved to a dedicate Updater without changes.

@drmingdrmer drmingdrmer merged commit 2d67471 into databendlabs:main Dec 2, 2024
31 checks passed
@drmingdrmer drmingdrmer deleted the 167-remove-loosen-follower-log-revert branch December 2, 2024 10:13
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