Repository navigation
[pull] main from hashicorp:main - #38
Merged
Merged
Conversation
…613) * feat: add CommitTrackingLogStore interface for commit index management * chore: remove non-idiomatic type assert func * feat(raft): add fast recovery mode for quicker log application - Introduced a `fastRecovery` flag in the Raft structure and configuration to enable fast recovery mode. - Updated `NewRaft` to initialize `fastRecovery` from the configuration. - Added `persistCommitIndex` function to store the commit index when fast recovery is enabled. - Modified `processLogs` to persist the commit index before updating `lastApplied`. - Documented the `FastRecovery` option in the config. * feat(raft): add recovery from committed logs during startup - Implemented `recoverFromCommitedLogs` function to recover the Raft node from committed logs. - If `fastRecovery` is enabled and the log store implements `CommitTrackingLogStore`, the commit index is read from the store, avoiding the need to replay logs. - Logs between the last applied and commit index are fed into the FSM for faster recovery. * refactor(store): rename ReadCommitIndex to GetCommitIndex for consistency - Refactor `ReadCommitIndex` to `GetCommitIndex` across `LogStore` and `InmemCommitTrackingStore`. - Introduce `InmemCommitTrackingStore` to track commit index in memory for testing purposes. - Add locking mechanism to safely read/write commit index in `InmemCommitTrackingStore`. * fix: also set inmem commit index when revocer log commit progress from persist store * perf: optimize startup recovery by skipping duplicated log replay * refactor(inmem-commit-tracking-store): store commit index in memory until the next StoreLogs call and then write it out on disk along with the log data. * chore: fix typo in recoverFromCommittedLogs function name * refactor(raft): update parameter name in persistCommitIndex function * refactor(raft): set commit index in memory before `StoreLogs` * refactor(raft): fix condition for skipping recovery in `recoverFromCommittedLogs` * feat(raft): add commit tracking logs and fast recovery tests - Implemented commit tracking logs in the Raft cluster configuration. - Added tests for restoring snapshots on startup with commit tracking logs. - Introduced a fast recovery test to ensure logs are applied correctly after restart. - Updated `MakeClusterOpts` to include `CommitTrackingLogs` option. * docs(config): update comments for FastRecovery mechanism * refactor(inmem-commit-tracking-store): simplify in-mem log tracking store implement * fix: rename persistCommitIndex to tryPersistCommitIndex Updated the function name from `persistCommitIndex` to `tryPersistCommitIndex` to better reflect its behavior. This function now updates the commit index in the persistent store only if fast recovery is enabled and if the log store implements `CommitTrackingLogStore`. Adjusted references to this function in `dispatchLogs` and `appendEntries`. * chore(raft): rename tryPersistCommitIndex to tryStageCommitIndex for clarity * refactor(log): introduce StagCommitIndex for optimized atomic persistence - Rename SetCommitIndex to StagCommitIndex in CommitTrackingLogStore interface - Add detailed documentation for StagCommitIndex method - Update raft.go to use StagCommitIndex instead of SetCommitIndex - Optimize commit index staging in appendEntries using min(lastNewIndex, leaderCommitIndex) This change ensures commit index updates are only persisted atomically with the following StoreLogs call, preventing inconsistencies between the commit index and log entries in case of crashes. It also optimizes the staged commit index to be as up-to-date as possible without exceeding the last new entry index, reducing commit index lag in the CommitTrackingStore. The contract for implementations is clarified, specifying that GetCommitIndex must never return a value higher than the last index in the log. * fix(raft): correct CommitTrackingLogStore implementation - Rename SetCommitIndex to StageCommitIndex in InmemCommitTrackingStore - Fix typo in CommitTrackingLogStore interface (StagCommitIndex to StageCommitIndex) - Update error message in tryStageCommitIndex for consistency - Ensure CommitTrackingLogStore extends LogStore interface * feat(raft): improve fast recovery error handling and commit index validation - Add error logging and panic for critical errors during recovery - Replace lastApplied check with lastIndex comparison - Ensure commitIndex does not exceed lastIndex - Improve code structure and readability * feat: add `CommitTrackingLogStore` interface check and adjust return type This commit adds a compile-time interface check for InmemCommitTrackingStore to ensure it implements the CommitTrackingLogStore interface. It also changes the return type of NewInmemCommitTrackingStore to be more specific. - Add interface check: var _ CommitTrackingLogStore = &InmemCommitTrackingStore{} - Change NewInmemCommitTrackingStore return type from CommitTrackingLogStore to *InmemCommitTrackingStore * refactor: improve type assertion for log store in TestRaft_FastRecovery This commit enhances the type assertion for CommitTrackingLogStore in the TestRaft_FastRecovery function, adding explicit error handling for better test reliability and clarity. - Add explicit type assertion check for CommitTrackingLogStore interface - Introduce new variable 'store' to hold the asserted CommitTrackingLogStore - Add error handling to fail the test if the type assertion is unsuccessful * feat: add warning log for unsupported fast recovery This commit adds a warning log message when fast recovery is enabled but the log store does not support the CommitTrackingLogStore interface. This provides better visibility into potential configuration issues. - Add warning log in recoverFromCommittedLogs when log store doesn't implement CommitTrackingLogStore - Include the type of the log store in the warning message for easier debugging * refactor: move commitIndex retrieve into `tryStageCommitIndex` * refactor: remove error from return field of recoverFromCommittedLogs * refactor: rename FastRecovery and revert the stageCommittedIdx change This commit renames the FastRecovery feature to RestoreCommittedLogs for better clarity and consistency. This is a breaking change as it modifies public API elements. - Rename Config.FastRecovery to Config.RestoreCommittedLogs - Rename Raft.fastRecovery to Raft.RestoreCommittedLogs - Update all references to fastRecovery in the codebase - Rename TestRaft_FastRecovery to TestRaft_RestoreCommittedLogs - Update comments to reflect the new terminology - Refactor tryStageCommitIndex to accept commitIndex as a parameter BREAKING CHANGE: Config.FastRecovery has been renamed to Config.RestoreCommittedLogs * docs: documented GetCommitIndex in CommitTrackingLogStore interface * docs: change fastRecovery flag to recoverCommittedLog in all documented comments * refactor: add a new ErrIncompatibleLogStore for recoverFromCommittedLogs * docs: clarify RestoreCommittedLogs configuration requirement - Add a notice in the Config struct documentation for RestoreCommittedLogs - Specify that Raft will fail to start with ErrIncompatibleLogStore if the requirement is not met * refactor: rename recoverFromCommittedLogs to restoreFromCommittedLogs * refactor!: update MakeCluster functions to return error - Update makeCluster to return (*cluster, error) - Modify MakeCluster, MakeClusterNoBootstrap, and MakeClusterCustom to return error - Update all test cases to handle potential errors from cluster creation - Replace t.Fatalf() calls with t.Logf() and error returns in makeCluster BREAKING CHANGE: MakeCluster, MakeClusterNoBootstrap, and MakeClusterCustom now return an additional error value, which needs to be handled in existing tests. * test: add test for RestoreCommittedLogs with incompatible log store * Revert "refactor!: update MakeCluster functions to return error" This reverts commit cfffcb5. * refactor: update makeCluster to return errors - Add PropagateError option to MakeClusterOpts - Update makeCluster to return (*cluster, error) - Modify MakeCluster, MakeClusterNo * Use wrapped err * docs: clarify GetCommitIndex behavior in CommitTrackingLogStore interface - Specify that GetCommitIndex should not return a value higher than the last index in the log - Clarify that if a higher value is returned, the last index in the log will be used instead - Add instruction to return (0, nil) when no commit index is found in the log store --------- Co-authored-by: peterxcli <peterxcli@gmail.com>
only error about "failed to take snapshot" when error isn't "nothing new to snapshot"
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )